Closed
Bug 57043
Opened 24 years ago
Closed 24 years ago
Negative integers as object properties: strange behavior!
Categories
(Core :: JavaScript Engine, defect, P3)
Core
JavaScript Engine
Tracking
()
VERIFIED
FIXED
People
(Reporter: david, Assigned: brendan)
Details
(Keywords: js1.5, Whiteboard: [rtm-] fix in trunk, ECMA-262 standard conformance)
Attachments
(3 files)
1.69 KB,
patch
|
Details | Diff | Splinter Review | |
2.52 KB,
patch
|
Details | Diff | Splinter Review | |
1.90 KB,
patch
|
Details | Diff | Splinter Review |
I get weird behavior when I use a negative integer as an object property.
js 1.5 rc2 treats the property name differently as a string than it does
as an integer. The following js shell session illustrates. Note that
this behavior doesn not occur when I use non-negative integers, or when
I use a positive or negative floating-point value as the array index
js> var o = new Object();
js> o[-1] = "number"; // is this even legal syntax?
number
js> o[-1]
number
js> o["-1"]
js> o["-1"] = "string";
string
js> o[-1]
number
js> o["-1"]
string
js> for(i in o) print(i + ": " + typeof i + ": " + o[i]);
-1: string: string
-1: string: string
js> o[-1]
number
Comment 1•24 years ago
|
||
Looking into this - somehow I remember that only string literals
may be used to define object property names. In the meantime, here
is an experiment in the JS Shell using the number 1 instead of -1 :
js> var o = {1 : "one - as Number"}
js> o[1]
one - as Number
js> o["1"]
one - as Number
js> var o = {"1" : "one - as String"}
js> o[1]
one - as String
js> o["1"]
one - as String
js> var o = {1 : "one - as Number", "1" : "one - as String"}
js> o[1]
one - as String
js> o["1"]
one - as String
Comment 2•24 years ago
|
||
I get an error if I attempt the same experiment with -1 :
js> var o = {-1 : "minus one - as Number", "-1" : "minus one - as String"}
110: invalid property id:
110: var o = {-1 : "minus one - as Number", "-1" : "minus one - as String"}
110: .........^
"invalid property id" is the error JSMSG_BAD_PROP_ID in js.msg.
It occurs in a switch case default in jsparse.c:
default:
js_ReportCompileErrorNumber(cx, ts, JSREPORT_ERROR, JSMSG_BAD_PROP_ID);
Comment 3•24 years ago
|
||
From the ECMA3 document:
11.2.1 Property Accessors
Properties are accessed by name, using either the dot notation:
MemberExpression . Identifier
CallExpression . Identifier
or the bracket notation:
MemberExpression [ Expression ]
CallExpression [ Expression ]
The dot notation is explained by the following syntactic conversion:
MemberExpression . Identifier
is identical in its behaviour to
MemberExpression [ <identifier-string> ]
and similarly
CallExpression . Identifier
is identical in its behaviour to
CallExpression [ <identifier-string> ]
where <identifier-string> is a string literal containing the same sequence
of characters as the Identifier.
The production MemberExpression : MemberExpression [ Expression ]
is evaluated as follows:
1. Evaluate MemberExpression.
2. Call GetValue(Result(1)).
3. Evaluate Expression.
4. Call GetValue(Result(3)).
5. Call ToObject(Result(2)).
6. Call ToString(Result(4)).
7. Return a value of type Reference whose base object is Result(5)
and whose property name is Result(6).
The production CallExpression : CallExpression [ Expression ]
is evaluated in exactly the same manner, except that the contained
CallExpression is evaluated in step 1.
Comment 4•24 years ago
|
||
Now trying an experiment closer to David's, and comparing 1 vs. -1
There is a clear difference in the behavior:
js> var o = new Object();
js> o[1] = "one - as Number"
one - as Number
js> o["1"] = "one - as String"
one - as String
js> for(i in o) print(i + ": " + typeof i + ": " + o[i]);
1: string: one - as String
js> o[-1] = "minus one - as Number"
minus one - as Number
js> o["-1"] = "minus one - as String"
minus one - as String
js> for(i in o) print(i + ": " + typeof i + ": " + o[i]);
1: string: one - as String
-1: string: minus one - as String
-1: string: minus one - as String
Assignee | ||
Updated•24 years ago
|
Assignee | ||
Comment 5•24 years ago
|
||
CHECK_FOR_FUNNY_INDEX got some ECMA love from mccabe, but it doesn't handle
ToString(-1) or other strings containing negative literals that fit in a jsval.
Mike, can you field this one fast for js1.5 (and maybe, hope against hope, for
Netscape 6 rtm)? Please reassign to me if not.
/be
Comment 6•24 years ago
|
||
Reassigning to Brendan (sorry) because Mike is away -
Assignee: mccabe → brendan
Assignee | ||
Comment 7•24 years ago
|
||
Patch coming up. Will try for [rtm+] on account of standards purity.
/be
Status: NEW → ASSIGNED
Assignee | ||
Comment 8•24 years ago
|
||
Assignee | ||
Comment 9•24 years ago
|
||
jsints are symmetric around 0, so we don't need to test for a different "max"
value when accumulating digits. The greatest jsint that fits in a jsval is
(1<<30) - 1 or 1073741823. The least is 1 - (1<<30) or -1073741823. The only
thing we need to do is remember whether the property id string began with a '-',
and if so, use the two's complement of the accumulated _index.
Looking for r= and sr= this weekend.
/be
Severity: normal → major
OS: Linux → All
Hardware: PC → All
Assignee | ||
Comment 10•24 years ago
|
||
Assignee | ||
Comment 11•24 years ago
|
||
Comment 12•24 years ago
|
||
Fix makes sense. Have you run the ECMA suite?
Also, there are some tab characters visible, even in lines that you added. Fix
the tab damage and verify with the ECMA suite, and r=shaver shall be your
reward.
Assignee | ||
Comment 13•24 years ago
|
||
shaver: my editor never adds tabs, but unless I shift or otherwise reformat a
whole line, it leaves any existing tabs alone.
I did run the JS testsuite, which needs to cover these cases! Same results as
before my change, all passed (once you suppress warnings, which now are on by
default in the js shell, but which confuse the test driver).
/be
Comment 14•24 years ago
|
||
sr=jband I'll by this.
Assignee | ||
Comment 15•24 years ago
|
||
Checked into trunk. Among the bugs david@oreilly.com has reported (which I
learned of only yesterday, and which show holes in our ECMA testsuite), this is
one of only a few that I think netscape.com should take for Netscape 6 RTM.
The change allows a leading '-' sign, and remembers (for a short section of
code) that it saw that character in order to pass a length check that weeds out
numeric-literal string identifiers too big to fit in a jsval (a signed machine
word used to hold JS identifiers and values), and finally to negate the
converted integer before tagging it as a jsval.
The consequences of not fixing this bug are that:
(a) We fail to conform to the ECMA-262 Edition 3 spec for the crucial rule that
property identifiers are strings, and indexing a property by an expression looks
up the identifier given by evaluating the expression and converting it to a
string.
(b) JS that depends on index-converted-to-string equivalence for negative
indexes expressed as strings and as numbers will fail (I don't know of web
examples, or of a good way to search for any using string matching).
(c) David Flanagan, the author of the O'Reilly "Rhino" JS book -- the top JS
book on the market, and a bestseller for O'Reilly -- will have to update that
book for Netscape 6 to document this bug.
PDT: If you have any questions, please feel free to ask here, via e-mail, on IRC
#mozilla, or by phone. Thanks,
/be
Whiteboard: [rtm+] fix in trunk, ECMA-262 standard conformance
Comment 17•24 years ago
|
||
FYI only -
I get 'unary minus operator applied to unsigned type, result still unsigned'
warnings from VC++.
Assignee | ||
Comment 18•24 years ago
|
||
That warning is silly, it's easily avoided via 0 - _index instead of -_index.
Fixed in trunk.
/be
Assignee | ||
Comment 19•24 years ago
|
||
PDT: I hope it's clear from the patch itself, and from my earlier comment:
>The change allows a leading '-' sign, and remembers (for a short section of
>code) that it saw that character in order to pass a length check that weeds out
>numeric-literal string identifiers too big to fit in a jsval (a signed machine
>word used to hold JS identifiers and values), and finally to negate the
>converted integer before tagging it as a jsval.
plus from the testing David has done, that *should this fix have a bug*, it will
affect only negative indexes expressed as strings, which were already broken.
It won't run off the zero-terminated end of a string, any more than the code
could before. It won't crash on a bad address, any more than the code could
before.
The fix does not alter behavior for unsigned numeric literals, it pellucidly
works for "-1", and it can be demonstrated to work for "-1073741823" (the least
integer that fits in a jsval, converted to a string) and "-1073741824" (which
can't fit, and so remains a string.
/be
Comment 20•24 years ago
|
||
r=mccabe.
+ JSBool _negative = (*_cp == '-');
+ if (_negative) _cp++;
Would it be a micro-erg better to say _cp += _negative, and avoid the branch?
Or is this too fine an optimization?
Similarly, but less clearly,
> if (_negative) _index = -_index;
Assignee | ||
Comment 21•24 years ago
|
||
An unconditional add instead of a predicted-taken branch around the then
statement might be better, yeah. I'll play around on popular architectures,
look at code, pipeline it through the superscalar simulator that MicroUnity left
in my brain.
Already did the _index = 0 - _index to abate an inconsistent Windows warning.
PDT: this fussing doesn't matter for the branch, although the warning fix is
free if you want it.
/be
Comment 22•24 years ago
|
||
This seems release-notable to us. [rtm-]
Whiteboard: [rtm+] fix in trunk, ECMA-262 standard conformance → [rtm-] fix in trunk, ECMA-262 standard conformance
Assignee | ||
Comment 23•24 years ago
|
||
Fix in trunk, and not wanted in N6 branch, so closing.
/be
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Comment 24•24 years ago
|
||
Verified on Linux, Mac and WinNT via this testcase added to JS test suite:
js/tests/js1_5/Regress/regress-57043.js
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•