Closed
Bug 630232
Opened 12 years ago
Closed 11 years ago
Reflect.parse("[,,,]") should make an ArrayExpression with .elements.length 3
Categories
(Core :: JavaScript Engine, defect, P5)
Tracking
()
RESOLVED
FIXED
mozilla13
People
(Reporter: jorendorff, Assigned: jorendorff)
Details
Attachments
(1 file)
4.92 KB,
patch
|
dherman
:
review+
|
Details | Diff | Splinter Review |
Reflect.parse("[]") and Reflect.parse("[,,,]") return identical ASTs, even though those are two different programs. js> eval("[]").length 0 js> eval("[,,,]").length 3 js> assertEq(Reflect.parse("[,,,]").body[0].expression.elements.length, 3) typein:11: Error: Assertion failed: got 0, expected 3
Assignee | ||
Comment 1•12 years ago
|
||
Taking, since I have a patch lying around. This bug would have been detected by the tests, but the testing mechanism had a minor bug. Fixing that bug caused the tests to detect bug 638577 (i.e. fail because of it)! So I had to comment out a few lines of tests.
Assignee: dherman → jorendorff
Attachment #516703 -
Flags: review?(dherman)
Comment 2•11 years ago
|
||
Comment on attachment 516703 [details] [diff] [review] v1 >- JSObject *array = NewDenseEmptyArray(cx); >+ const size_t len = elts.length(); >+ JSObject *array = NewDenseAllocatedArray(cx, uint(len)); Neat -- didn't know about NewDenseAllocatedArray. > if (!array) > return false; > >- const size_t len = elts.length(); > for (size_t i = 0; i < len; i++) { > Value val = elts[i]; > > JS_ASSERT_IF(val.isMagic(), val.whyMagic() == JS_SERIALIZE_NO_NODE); > > /* Represent "no node" as an array hole by not adding the value. */ > if (val.isMagic(JS_SERIALIZE_NO_NODE)) > continue; Aaaaargh, fencepost bug! I haz a sad. >-assertExpr("[,,,1,2,3,]", arrExpr([,,,lit(1),lit(2),lit(3)])); >-assertExpr("[,,,1,2,3,,]", arrExpr([,,,lit(1),lit(2),lit(3),])); >-assertExpr("[,,,1,2,3,,,]", arrExpr([,,,lit(1),lit(2),lit(3),,])); >-assertExpr("[,,,,,]", arrExpr([,,,,])); >+assertExpr("[,,,1,2,3,]", arrExpr([,,,lit(1),lit(2),lit(3),])); >+assertExpr("[,,,1,2,3,,]", arrExpr([,,,lit(1),lit(2),lit(3),,])); >+assertExpr("[,,,1,2,3,,,]", arrExpr([,,,lit(1),lit(2),lit(3),,,])); >+assertExpr("[,,,,,]", arrExpr([,,,,,])); Bogus test case is bogus. Mea culpa. Looks good! r=me
Attachment #516703 -
Flags: review?(dherman) → review+
Assignee | ||
Comment 3•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/21f05e099a36
Comment 4•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/21f05e099a36
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in
before you can comment on or make changes to this bug.
Description
•