Closed
Bug 471531
Opened 16 years ago
Closed 16 years ago
js1_7/decompilation/regress-379925.js | js1_8_1/decompilation/regress-371802.js FAIL
Categories
(Core :: JavaScript Engine, defect, P1)
Tracking
()
VERIFIED
FIXED
mozilla1.9.1b3
People
(Reporter: bc, Assigned: brendan)
References
Details
(Keywords: regression, testcase, verified1.9.1, Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
1.95 KB,
patch
|
jorendorff
:
review+
sayrer
:
approval1.9.1+
|
Details | Diff | Splinter Review |
bug 470374 regressed this on tracemonkey. js1_7/decompilation/regress-379925.js Expected value ' function ( ) { var [ x , x ] = [ , , ] ; for ( [ x , x ] in [ ] ) { } } ', Actual value ' function ( ) { x = var ; x = ; for ( [ x , x ] in [ ] ) { } } ' compile actual reason: Expected value 'No Error', Actual value 'SyntaxError: syntax error' js1_8_1/decompilation/regress-371802.js Expected value ' function ( a , b , n ) { for ( var [ i , j ] = [ a , b ] ; i < n ; [ i , j ] = [ a , b ] ) { print ( i ) ; } } ', Actual value ' function ( a , b , n ) { i = var a ; j = b ; for ( ; i < n ; i = a ; j = b ; a ) { print ( i ) ; } } ' Expected value 'No Error', Actual value 'SyntaxError: syntax error'
Flags: in-testsuite+
Flags: in-litmus-
Assignee | ||
Updated•16 years ago
|
Assignee: general → brendan
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.9.1b3
Assignee | ||
Comment 1•16 years ago
|
||
Urgh, thought I had run the js1_7 tests with and without that patch -- had the wrong patch pushed. Sorry, fixing today in tm. /be
Assignee | ||
Comment 2•16 years ago
|
||
Hope to land this in tm today. /be
Attachment #354903 -
Flags: review?(jorendorff)
Assignee | ||
Comment 3•16 years ago
|
||
Testsuite says this patch fixes these bugs: js1_5/Regress/regress-169559.js js1_7/decompilation/regress-379925.js js1_7/extensions/regress-346642-06.js js1_8_1/decompilation/regress-346642-01.js The first is the old "STATUS: Global vars should not be more than 2.5 times slower than local" chestnut, but I can reproducibly fail it without the patch for this bug, pass it with. Hrm. Second and third are of course this bug. Fourth is an assertbotch: Assertion failure: strcmp(rval, forelem_cookie) == 0, at ../jsopcode.cpp:3347 so also this bug. Bob, you concur? /be
Reporter | ||
Comment 4•16 years ago
|
||
re: js1_5/Regress/regress-169559.js, I've been having problems with this and some of the BigQ tests that I haven't resolved yet. I don't see this on my tracemonkey builds for the last couple of days however with or without the patch. testing on mac: js1_7/decompilation/regres-379925.js and js1_8_1/decompilation/regress-371802.js both pass with this patch and the assert in js1_7/extensions/regress-346642-06.js and js1_8_1/decompilation/regress-346642-01.js is also fixed by this patch. /me concurs.
Comment 5•16 years ago
|
||
Comment on attachment 354903 [details] [diff] [review] fix boundary condition on group assignment vs. faulting getlocal decomp > #if JS_HAS_DESTRUCTURING >- if (sn && SN_TYPE(sn) == SRC_GROUPASSIGN && len > JSOP_GETLOCAL_LENGTH) { >+ if (sn && >+ SN_TYPE(sn) == SRC_GROUPASSIGN && >+ (len > JSOP_GETLOCAL_LENGTH || pc > startpc)) { I don't see how `len > JSOP_GETLOCAL_LENGTH` can ever be true here. I think the change you want is just to change `len` to `endpc - startpc` on that line. Your patch works as well, but I think the `len` comparison is nonsense. Sorry I missed this in the previous review. There's something else confusing here: js> function f(x) {var [[a,b]]=[x];} js> f(null) typein:17: TypeError: var x is null Note the "var" in the error message. This is somehow caused by the source note. Can you fix that too?
Attachment #354903 -
Flags: review?(jorendorff)
Assignee | ||
Comment 6•16 years ago
|
||
Great review comments, endpc - pc is what I mistook len for, and that var regression in the last patch is fixed by this patch. /be
Attachment #354903 -
Attachment is obsolete: true
Attachment #354973 -
Flags: review?(jorendorff)
Comment 7•16 years ago
|
||
Comment on attachment 354973 [details] [diff] [review] better fix, thanks Much better!
Attachment #354973 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 8•16 years ago
|
||
Fixed in tm: http://hg.mozilla.org/tracemonkey/rev/34f4df9c4f9c This fix should go into m-c for sure, and into 1.9.1 too IMHO. /be
Flags: wanted1.9.1?
Whiteboard: fixed-in-tracemonkey
Updated•16 years ago
|
Flags: wanted1.9.1? → wanted1.9.1+
Updated•16 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•16 years ago
|
Attachment #354973 -
Flags: approval1.9.1?
Updated•16 years ago
|
Attachment #354973 -
Flags: approval1.9.1? → approval1.9.1+
Comment 9•15 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/dea8b86fe160
Keywords: fixed1.9.1
Reporter | ||
Comment 10•15 years ago
|
||
v 1.9.1, 1.9.2
Status: RESOLVED → VERIFIED
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•