313567 - string.length should not be generic

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Igor Bukanov]

From ECMA-262:

--------------------------------------------
15.5.5    Properties of String Instances
...
The [[Value]] property is the string value represented by this String object.
 
15.5.5.1    length

The number of characters in the String value represented by this String object. Once a String object is created, this property is unchanging. It has the attributes { DontEnum, DontDelete, ReadOnly }.
--------------------------------------------

AFAICS this text implies that "length" property is the length of the string passed to "String" constructor and in particular for any string instance "s" s.valueOf().length == s.length unless valueOf is overwritten.

On the other hand in SpiderMonkey "length" property is generic and is defined to be the length of the string resulted from conversion of the object to string. The latter can be overwritten,for example, with custom toString method.

Here is a test case that demonstrates the difference between SpiderMonkey and ECMA required behavior:

var s = new String("1");
s.toString = function() {
    return "22";
}
var ecma_expected_length = 1;
var actual = s.length;
print("ecma_expected_length="+ecma_expected_length+" actual="+actual);

Currentl in js shell it prints 
ecma_expected_length=1 actual=2

where actual=2 comes from "22" returned by custom toString method.

Comment 1

[Brendan Eich [:brendan]]

Attachment: possible fix

The jsobj.c change fixes a bug that was supposed to have been fixed already (old unreported bug, see rev 3.167 of jsobj.c).

The effect of the jsstr.c changes is to allow this to work:

var o = {__proto__: new String("Hello"), toString: function(){return "Bye"}};

print(o[0], o[1]);  // H e
print(o.length);    // 3
print(o);           // Bye

Of course, uneval(o) throws TypeError: String.prototype.toSource called on incompatible Object.

I'm trying to balance backward compatibility with following the spec.  Always a ton of fun.  Comments?

/be

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 200617 [details] [diff] [review]
possible fix

The JSVAL_IS_VOID(v) tests on the private data value are not necessary.

/be

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 200617 [details] [diff] [review]
possible fix

sr=shaver

Comment 4

[Brendan Eich [:brendan]]

Attachment: possible fix, v2

Comment 5

[Brendan Eich [:brendan]]

Fixed on trunk.  I'm trying not to fix this for 1.8/fx1.5/js1.6 -- if someone thinks it should be fixed on that branch, please nominate.

/be

Comment 6

[Bob Clary [:bc] (inactive)]

Checking in regress-313567.js;
/cvsroot/mozilla/js/tests/ecma_3/String/regress-313567.js,v  <--  regress-313567.js
initial revision: 1.1

Comment 7

[Bob Clary [:bc] (inactive)]

verified fixed cvs debug trunk builds windows/linux

Comment 8

[Brendan Eich [:brendan]]

*** Bug 340940 has been marked as a duplicate of this bug. ***

Comment 9

[Brendan Eich [:brendan]]

*** Bug 340534 has been marked as a duplicate of this bug. ***

Comment 10

[Bob Clary [:bc] (inactive)]

fixed by Bug 336373 on the 1.8.1 branch. 
verified fixed 1.8.1 with windows/macppc/linux 20060707