Closed
Bug 696109
Opened 10 years ago
Closed 10 years ago
"Assertion failure: !cx->isExceptionPending()," with Reflect.parse
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla10
People
(Reporter: gkw, Unassigned)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: js-triage-needed)
Attachments
(2 files)
2.88 KB,
text/plain
|
Details | |
2.14 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
Reflect.parse("with({c})p") asserts js debug shell on m-c changeset 311fdb9b38b7 without any CLI flags at Assertion failure: !cx->isExceptionPending(), autoBisect is now running.
Comment 1•10 years ago
|
||
Might be a regression from bug 695238.
![]() |
Reporter | |
Comment 2•10 years ago
|
||
(In reply to Jesse Ruderman from comment #1) > Might be a regression from bug 695238. That's correct. autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 78876:feeee0906588 user: Dave Herman date: Mon Oct 17 21:09:56 2011 -0700 summary: Bug 695238 - Reflect.parse should throw when an object literal is missing a property RHS. r=jorendorff
Comment 3•10 years ago
|
||
Found it. Turns out that patch surfaced a bug that had been sitting around for a while. ASTSerializer::statement has a precedence bug in the TOK_WITH | TOK_WHILE case (missing parentheses around the ? : expression). Patch forthcoming. Thanks, Dave
Comment 4•10 years ago
|
||
Updated•10 years ago
|
Attachment #568523 -
Flags: review?(jorendorff)
Comment 5•10 years ago
|
||
Comment on attachment 568523 [details] [diff] [review] C++ precedence bug Please make a separate 6-line test rather than adding to reflect-parse.js. reflect-parse.js is awesome, and I only wish our whole test suite were that thorough -- but because it's so long, once it fails I still don't know what I broke. I have to make a copy of it and manually reduce. So let's not make it longer. r=me with that.
Attachment #568523 -
Flags: review?(jorendorff) → review+
Comment 6•10 years ago
|
||
> Please make a separate 6-line test rather than adding to reflect-parse.js. Good call. > reflect-parse.js is awesome, and I only wish our whole test suite were that > thorough -- but because it's so long, once it fails I still don't know what > I broke. I have to make a copy of it and manually reduce. So let's not make > it longer. Is it worth separating out chunks of reflect-parse.js? Separate directory for it, divided into a few files by theme? Am I really asking you to ask me to do more work? Dave
Comment 7•10 years ago
|
||
(In reply to Dave Herman [:dherman] from comment #6) > Is it worth separating out chunks of reflect-parse.js? Separate directory > for it, divided into a few files by theme? If you want to, rs=me. There's no need to hold this up over that, though.
Comment 9•10 years ago
|
||
Backed out for reftest failure: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=d5cc34a9351d { REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/jsreftest/tests/jsreftest.html?test=js1_8_5/extensions/regress-696109.js | Unknown file:///home/cltbld/talos-slave/test/build/jsreftest/tests/js1_8_5/extensions/regress-696109.js:10: Reflect is not defined item 1 } https://hg.mozilla.org/integration/mozilla-inbound/rev/5b8e29b39c7f philor/mak says likely fix is to add |skip-if(!xulRuntime.shell)| to the test in jstests.list.
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/f944c4c67b78
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
Comment 12•8 years ago
|
||
A testcase for this bug was automatically identified at js/src/tests/js1_8_5/extensions/regress-696109.js.
Flags: in-testsuite+
![]() |
Reporter | |
Comment 13•8 years ago
|
||
Testcases have been landed by virtue of being marked in-testsuite+ -> VERIFIED as well.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•