({a:{c:x}, b:x}) = ({b:3}) TypeError on line 1: undefined has no properties [[x]] = 6; TypeError on line 1: undefined has no properties I think these could use more specific error messages.

These are caused by lack of decompilation support. Resummarizing, and taking to fix for js1.7. /be

This is a critical bug, because at the limit Function.prototype.toString will not produce valid output (an ECMA violation), and may crash. /be

javascript:function f() { [z] = 3 }; alert(uneval(f)); does indeed crash today's Mac trunk nightly.

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

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

Jesse, please test this hard. Thanks, /be

With the patch, js> function () { [g] = [3]; } Assertion failure: strcmp(rval, forelem_cookie) == 0, at jsopcode.c:2315

This patch does strange things to decompilation of "let" expressions: js> function() { for(; let (x=3,y=4) null;) { } } function () { for (; const x = 3, y = 4; null;) { } } js> function() { let (x=3,y=4) { } x = 5; } function () { var x = 3, y = 4; x = 5; }

After this patch, function () { for ([11].x;;) break; } no longer compiles. It used to work, but now it says "SyntaxError: missing ; after for-loop initializer".

With this patch, the following crashes [@ js_PopStatement]. new Function("for([0 for each (x in");

(In reply to comment #7) > With the patch, > > js> function () { [g] = [3]; } > Assertion failure: strcmp(rval, forelem_cookie) == 0, at jsopcode.c:2315 Group assignment is optimized separately, and doesn't use SRC_IMAGE. Fixing. /be

Comment on attachment 238547 [details] [diff] [review] fix, plus robustification of decompiler code > case SRC_DECL: >+ cond = js_GetSrcNoteOffset(sn, 0); >+#if JS_HAS_DESTRUCTURING >+ if (cond <= SRC_DECL_LET) { >+ /* This pop follows a destructuring declaration. */ As Jesse's found, you can't do this. The distance could very well be 0 or 1, which would incorrectly trigger this code. Is there some other bytecode/srcnote sequence you can use for this?

This is "plan C", source recovery. I haven't fixed group assignment (comment ) yet, because even though plan C uses brute force and is not dependent on bytecode generation details, it is incomplete. It fails to address the requirement of js_DecompileValueGenerator, which wants to decompile any left-hand-side expression (including one in a destructuring expression). The other bugs Jesse is seeing to do with js_ReportIsNotFunction arise from a bug in js_DecompileValueGenerator that's increasingly acute: it does not model the full stack for the decompiler, it expects to be able to decompile a slice of the bytecode for a script with only the top few slots of the stack pushed and (by the end of the decompilation) mostly popped. This obviously can't work if there are JSOP_GETLOCAL and the like references lower down the stack. "Plan A" is detailed decompilation based on code generation for destructuring. "Plan B" is pretty-printing the AST to get a canonical source image, and only for destructuring expressions (to avoid imposing costs on all object and array initialiser expressions). Note how this patch, "plan C", imposes source saving costs on all initialisers. I'm posting it here for posterity as much as for review. To fix js_DecompileValueGenerator, we need plan A. I'm thinking about how to do plan A cleanly. /be

With this patch, js> function() { return [({ x: y }) = p for (z in 5)] } Assertion failure: atom, at jsatom.c:928

With this patch, this function's decompilation doesn't compile: js> function() { new ({ a: b } = c) } function () { new { a: b } = c; } js> function () { new { a: b } = c; SyntaxError: invalid assignment left-hand side: I don't know whether the bug is in the decompiler omitting parens, or in the compiler rejecting the version without parens.

(In reply to comment #15) > I don't know whether the bug is in the decompiler omitting parens, or in the > compiler rejecting the version without parens. Parens are required, you can't have new operator expression on the left-hand side of assignment. /be

This is for testing, mainly. mrbkap is welcome to start reviewing, however! /be

js> (let (x) 3)() typein:1: TypeError: 3 is not a function js> (let (x) 3).foo() typein:2: TypeError: (let (x) 3).foo is not a function It would be nice if the former would also decompile the expression (instead of uneval'ing the thing that isn't a function).

This patch causes a regression: js> try { throw 1; } catch(e1 if 0) { } catch(e2 if (new NaN)) { } Assertion failure: pcdepth >= 0, at jsopcode.c:3889

And another: js> z = [1]; 1 js> let (x = (undefined ? 3 : z)) { x.t.g } Assertion failure: top < ss->printer->script->depth, at jsopcode.c:792

This cures all regressions and lesser issues reported by Jesse so far. /be

Better version of the assertion-triggering code in comment 20 (e.g. for using as a regression test) : z = [1]; new Function("let (x = (undefined ? 3 : z)) { x.t.g }")()

Or even z = [1]; (function() { let (x = (undefined ? 3 : z)) { x.t.g } })()

This fixes the ?: expression stack modeling problem in js_DecompileValueGenerator. /be

js> with({x: (new (b = 1))}) (2).x Assertion failure: top < ss->printer->script->depth, at jsopcode.c:792

js> (function(){try { } catch(f) { return; } finally { this.zzz.zzz }})() Assertion failure: pcdepth >= 0, at jsopcode.c:3929

js> (new Function("if(\n({y:5, p: (print).r})) { p={}; (p.z = <x\n><y/></x> ? 3 : 4)(5) }"))() Assertion failure: op == JSOP_GOTO || op == JSOP_GOTOX, at jsopcode.c:3853

This should cure known ills. /be

That last patch was bad medicine! Try this, it's better. /be

Thanks to Jesse for the IRC test-buddying. /be

Small fix to last patch. /be

This isn't a regression from the patch, but it would be nice if the patch would fix it. js> switch(x) { case 3: case (new ([3].map)): } const x; Assertion failure: top != 0, at jsopcode.c:816

This is a regression from the patch: js> let (x=3) ((++x)()) Crash [@ strlen] deep within js_DecompileValueGenerator.

I got a conflict in jsscan.c when I did "cvs update" after applying this patch (due to the checkin for bug 348836?).

Btw, if I use opt jsshell (on trunk, not with this patch), comment 33 gives me a null deref in strcmp.

Ok, this fixes all reported problems. It paves the way for destructuring plan A, which is next. /be

With the patch, I'm getting incorrect error messages with "import" statements: js> x = {}; [object Object] js> import x.y; typein:3: ReferenceError: import x.y; is not defined

Jesse, this should defend against destructuring decompilation crashes. It doesn't produce correct results for destructuring decompilation, but it should not crash. It fixes the minor import regression, too. Can you verify? Thanks, /be

Jesse, please try this instead. If nothing better happens today, we should land this for 1.8.1. /be

Comment on attachment 238896 [details] [diff] [review] prep patch for plan A, v7a Oops, wrong upload. /be

Blake, FindMaybeScopeStatement was just getting in the way, so I inlined it to its single call site. This should fix any bug where a let declaration is inside a with inside a block (where the with body is not a block, that is). Same goes for switch instead of block, since switch and block are both maybe-scopes. /be

This is with not only FindMaybeScopeStatement inlined, but more comments and assertions to help prove soundness. Previously the code begged questions about things like "if the downScope link is null, must this be a maybe-scope?" and (the bug being fixed) "the maybe-scope is always above any statement info record that is already on the tc->topScopeStmt stack." /be

This is what I would like to land for 1.8.1. It fixes the known safety bugs, but leaves destructuring decompiling broken but crash-free (Jesse please confirm). /be

Comment on attachment 238939 [details] [diff] [review] prep patch for plan A, v8a Where are my reviewers? May have to call in shaver :-P. /be

Comment on attachment 238941 [details] [diff] [review] prep patch for plan A, v8b >+#if JS_HAS_DESTRUCTURING >+ if (!params[i]) { >+ JS_ASSERT(*pc == JSOP_GETARG); >+ pc += JSOP_GETARG_LENGTH; >+ JS_ASSERT(*pc == JSOP_DUP); >+ /* XYZZY */ XXX? FIXME? I'm confused. ;-) >- if (!stmt) { >+ if (!stmt && (tc->flags & TCF_COMPILING)) { I don't think you can do this. Isn't there code below this if statement that relies on stmt being non-null? >- data.op = CURRENT_TOKEN(ts).t_op; >+ data.op = let ? JSOP_NOP : CURRENT_TOKEN(ts).t_op; > data.binder = let ? BindLet : BindVarOrConst; > pn = NewParseNode(cx, ts, PN_LIST, tc); > if (!pn) > return NULL; >- pn->pn_op = let ? JSOP_NOP : data.op; >+ JS_ASSERT(!let || data.op == JSOP_NOP); I find this assertion somewhat vacuous given the context, but I suppose it's fine. Other than that, things look good, r=mrbkap.

(In reply to comment #48) > (From update of attachment 238941 [details] [diff] [review] [edit]) > >+#if JS_HAS_DESTRUCTURING > >+ if (!params[i]) { > >+ JS_ASSERT(*pc == JSOP_GETARG); > >+ pc += JSOP_GETARG_LENGTH; > >+ JS_ASSERT(*pc == JSOP_DUP); > >+ /* XYZZY */ > > XXX? FIXME? I'm confused. ;-) I was too :-P -- working on this... > > >- if (!stmt) { > >+ if (!stmt && (tc->flags & TCF_COMPILING)) { > > I don't think you can do this. Isn't there code below this if statement that > relies on stmt being non-null? Thanks, fixed. > >- data.op = CURRENT_TOKEN(ts).t_op; > >+ data.op = let ? JSOP_NOP : CURRENT_TOKEN(ts).t_op; > > data.binder = let ? BindLet : BindVarOrConst; > > pn = NewParseNode(cx, ts, PN_LIST, tc); > > if (!pn) > > return NULL; > >- pn->pn_op = let ? JSOP_NOP : data.op; > >+ JS_ASSERT(!let || data.op == JSOP_NOP); > > I find this assertion somewhat vacuous given the context, but I suppose it's > fine. No, it's pointless -- it was due to too little context. Thanks. New patch to checkpoint pre-plan-A state soon. /be

Bugzilla interdiff should work. This still is a prep patch, it does not cure all crashes due to destructuring, specifically in for-in loops: function() { for (var [a, b] in []) for ([c, d] in []) { } } /be

Comment on attachment 238970 [details] [diff] [review] prep patch for plan A, v9 >+ * If the stacked offset is -1, return 0 to index the NUL padding at the start >+ * of ss->sprinter.base. If this happens, it means there is a decompiler bug >+ * to fix, but it won't violate memory safety. Want to add a debug-only assertion here so that we can track these down? >+ if ((pc == fp->pc || (pc == startpc && cs->nuses != 0))&& Nit: space before &&.

I'll let mrbkap mark this if he's still up. The interdiff is self-explanatory. Next is the real patch, destructuring decompilation. Finally we have a sound framework on which to build. /be

This fixes a regression in v9 and v9a: GetLocal must not call GetOff, because local names are found when not already stacked by brute-force script atom map search. This also puts in an assertion as mrbkap suggested, but only if he, crowder or I is compiling. No point in flicting what may be a frequently bogus assertion on anyone else. /be

Presuming r+mrbkap, landing this on trunk and 1.8 branch shortly. /be

Rerunning the js testsuite on branch vs. patched trunk disclosed a change here that I'd made at Jesse's prompting, but only in a more evolved patch than what I'm checking in. We need to avoid uneval(this) when complaining about this() for an uncallable this object, e.g. This is what I'm checking in. /be

Brendan, can we get this checked in by EOD today and marked fixed? Thanks.

(In reply to comment #56) > Brendan, can we get this checked in by EOD today and marked fixed? Thanks. Helping take care of sick family at home, but sure: I'll do it or die trying :-/. /be

I'm this close to having group assignment working, but need to sleep (cold still dogging me). This is for testing. /be

js> f = function () { ({x: a(b)}) = window; } function () { ({x: a(b)[]} = window); } js> f = function () { ({x: *}) = window; } function () { ({x: *[]} = window); }

js> function() { let ([a]=x) { } } function () { let (let [a] = x) { } }

js> f = function( ){ new ([x] = k) } function () { new [x] = k; } js> eval(""+f) typein:7: SyntaxError: invalid assignment left-hand side: typein:7: new [x] = k; typein:7: ............^

js> function() { [a] = [b] = c } Hangs, using an infinite amount of memory.

Fixes the problems Jesse reported. Still to decompile: * group assignment (throws an "unimplemented" error still); * destructuring catch variables; * destructuring formal parameters. /be

This patch handles destructuring catch variables and formal parameters. Group assignment next. Attached for testing and easier incremental reviewing. /be

Fixes a regression in restricting for (let [k, v] in o) and similar loops to allow only an array of length 2, and the regression Jesse found in bug 353214 comment 2. Still no group assignment decompilation. /be

Eliminate code in DecompileDestructuring (always a good thing) that was trying to decompile a special case that can't be attempted without arbitrary lookahead in the bytecode; the general case is handled by the Decompile(ss, pc, -(stackdepth)) mode. Also fixed that general code to auto-parenthesize the left-most decompiled result if necessary. /be

This has one dead code elimination (in DecompileDestructuring) and three spot fixes based on fast interaction with Jesse over IRC. /be

This is a checkpoint before group assignment fixing. Mainly, it subroutines DecompileDestructuringLHS from the inline left-hand side element or property destructuring code in DecompileDestructuring. It fixes a bug Jesse found in non-int numeric property name handling. And it allows empty destructuring initialisers, per the ECMA Edition 4 proposal. /be

Decompilation fails for the new "empty destructuring assignment" expressions: js> "" + function() { [] = 3 }; print(3); js> js> function() { ({}) = 3 } js>

js> function () { while( {} = e ) ; } out of memory js> function () { while( {} = (a)(b) ) ; } function () { a(b)}

Generate minimal distinct bytecode sequences for empty destructuring and group assignment, and decompile them. This patch supports empty group assignment in all contexts: js> function f(){[] = [a,b,c]} js> f function f() { [] = [a, b, c]; } js> function f(){for([] = [a,b,c];;);} js> f function f() { for ([] = [a, b, c];;) { } } js> function f(){for(;;[] = [a,b,c]);} js> f function f() { for (;; [] = [a, b, c]) { } } js> function f(){for(let [] = [a,b,c];;);} js> f function f() { for (let [] = [a, b, c];;) { } } The next patch, v5, will support non-empty group assignment. /be

This implements group assignment decompilation. /be

"let [] = []" in a for initial-expression disappears entirely. This changes the meaning of any "let" declarations inside the for loop, because for loops currently(?) establish new scope iff the initial-expression is a let declaration. js> z = 6 6 js> f = function (){for(let [] = []; false;) let z; return z} function () { for (; false;) let z; return z; } js> f() 6 js> eval(""+f)() js>

js> "" + function () { return [2 for ([] in p)] }; print(3); js>

(In reply to comment #73) > "let [] = []" in a for initial-expression disappears entirely. This changes > the meaning of any "let" declarations inside the for loop, because for loops > currently(?) establish new scope iff the initial-expression is a let > declaration. Yes, no (?) about it -- "just like C++" ;-). New patch in a minute. /be

More mysterious disappearing: js> function() { for (;; [x] = [1]) { } } function () { for (;; ) { } }

This fixes all reported problems (reported recently in the bug -- I can't get on irc.mozilla.org for some reason atm). /be

See the interdiff -- just a comment tweak and clarifying note. /be

js> function() { [g['//']] = h } function () { [g.//] = h; }

js> "" + function () { for(;; [[a]] = [5]) { } }; print(3); js>

It fails to decompile an object with a setter (or a function with such an object literal), if the setter has a destructuring formal parameter and is a generator. js> f = function () { return { set x([a]) { yield; } } } js> obj = f() [object Object] js> uneval(obj); print(3); js>

Fix those three hard cases (good finds! your fuzzer fu is the best). /be

js> f = (function() { for ( let [a,b]=[c,d] in [3]) { } }) function () { let [a, b] = [c, d]; for (let [a, b] in [3]) { } }

Btw, the fuzzer isn't very good at noticing when decompilation changes the scope of a "let" declaration (e.g. comment 73 and comment 83). So when I find bugs like that, it's often through ad-hoc testing.

Another case where stuff just disappears: js> function () { while(1) [a] = [b]; } function () { while (1) { } }

js> function () { for(var [x, y] = r in p) { } } function () { for (var [x, y] in p) { } var [x, y] = r} js>

js> "" + function () { [y([a]=b)] = z }; print(3); js>

js> "" + function () { for(;; ([[,]] = p)) { } }; print(3) js>

js> function() { for([x] = [];;) { } } js>

Patch commentary coming as a separate attachment. /be

When compiling opt: jsopcode.c: In function 'DecompileDestructuringLHS': jsopcode.c:1168: warning: 'todo' may be used uninitialized in this function

Brain fart, should have grepped for warnings. /be

Fix this case: function () { let ([y] = delete [1]) { } } to parenthesize the rewritten ([1], true) on the right of =. /be

I ran the fuzzer for 2.3 million iterations with v5f, and didn't find any regressions or remaining destructuring-decompilation problems :)

Patch to commit. Notes on the patch under way, to be attached. /be

js> print(1); for(x in (function ([y]) { })() ) { } print(3); 1 In a debug build, I get "Assertion failure: atom, at jsopcode.c:3375". In an opt build, it just stops.

The expressions in comment 0 still give the not-very-helpful message "undefined has no properties". Should I file a separate bug / enhancement request for that?

This is a regression: js> function () { delete 4..x } function () { delete 4.x; }

I will fix the easy-to-fix problems identified in comment 97 and comment 98, but I'm not going to fix the "(null|undefined) has no properties" error messages for 1.8.1. So please file that follow-up bug; thanks. /be

Patch to commit, take two. /be

1.2 million iterations with v6a, looks good.

Split out bug 354921, "Better error messages for destructuring-assignment failures", for the issues in comment 0.

In the interest of getting this baking, I landed the v6a patch on the trunk: Checking in jsemit.c; /cvsroot/mozilla/js/src/jsemit.c,v <-- jsemit.c new revision: 3.216; previous revision: 3.215 done Checking in jsemit.h; /cvsroot/mozilla/js/src/jsemit.h,v <-- jsemit.h new revision: 3.68; previous revision: 3.67 done Checking in jsinterp.c; /cvsroot/mozilla/js/src/jsinterp.c,v <-- jsinterp.c new revision: 3.293; previous revision: 3.292 done Checking in jsopcode.c; /cvsroot/mozilla/js/src/jsopcode.c,v <-- jsopcode.c new revision: 3.186; previous revision: 3.185 done Checking in jsopcode.tbl; /cvsroot/mozilla/js/src/jsopcode.tbl,v <-- jsopcode.tbl new revision: 3.79; previous revision: 3.78 done Checking in jsparse.c; /cvsroot/mozilla/js/src/jsparse.c,v <-- jsparse.c new revision: 3.249; previous revision: 3.248 done Checking in jsscan.h; /cvsroot/mozilla/js/src/jsscan.h,v <-- jsscan.h new revision: 3.47; previous revision: 3.46 done Checking in jsxdrapi.h; /cvsroot/mozilla/js/src/jsxdrapi.h,v <-- jsxdrapi.h new revision: 1.23; previous revision: 1.22 done Tired now, will work on notes for tomorrow. Hoping for mrbkap to appear soon. /be

Checking in regress-346642-01.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-01.js,v <-- regress-346642-01.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-02.js,v done Checking in regress-346642-02.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-02.js,v <-- regress-346642-02.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-03.js,v done Checking in regress-346642-03.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-03.js,v <-- regress-346642-03.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-04.js,v done Checking in regress-346642-04.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-04.js,v <-- regress-346642-04.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-05.js,v done Checking in regress-346642-05.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-05.js,v <-- regress-346642-05.js initial revision: 1.1 done RCS file: /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-06.js,v done Checking in regress-346642-06.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-06.js,v <-- regress-346642-06.js initial revision: 1.1 done there may be problems with the tests or with the patch.

These are the results for the tests with my debug shell. Let me know if the tests are incorrect.

Comment on attachment 240721 [details] shell results Testcase js1_7/lexical/regress-346642-01.js failed Bug Number 346642 [ Top of Page ] STATUS: decompilation of destructuring assignment Failure messages were: FAILED!: [reported from test()] decompilation of destructuring assignment: 13: compare source FAILED!: [reported from test()] Expected value ' function ( ) { ( { } ) = 3 ; } ', Actual value ' function ( ) { [ ] = 3 ; } ' This is the one true decompilation of an empty destructuring initialiser, whether object or array on input. The effect is the same (really, we could just decompile the right hand side, since nothing is destructured). FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 14: compare source FAILED!: [reported from test()] Expected value ' function ( ) { while ( { } = e ) ; } ', Actual value ' function ( ) { while ( ( [ ] = e ) ) { } } ' Ditto. FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 15: compare source FAILED!: [reported from test()] Expected value ' function ( ) { while ( { } = ( a ) ( b ) ) { } } ', Actual value ' function ( ) { while ( ( [ ] = a ( b ) ) ) { } } ' Ditto. FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 22: compare source FAILED!: [reported from test()] Expected value ' function ( ) { for ( let [ a , b ] = [ c , d ] in [ 3 ] ) { } } ', Actual value ' function ( ) { [ c , d ] ; for ( let [ a , b ] in [ 3 ] ) { } } ' The decompiler hoists the initialiser since it may have side effects. The initializer [c, d] is never destructured into the let-bound a and b variables, because they are scoped by the loop body and (if the loop iterates at all) are destructured from the key and value of the next enumerable property in [3]. FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 24: compare source FAILED!: [reported from test()] Expected value ' function ( ) { for ( var [ x , y ] = r in p ) { } } ', Actual value ' function ( ) { var [ x , y ] = r ; for ( [ x , y ] in p ) { } } ' Here, not only the initialiser but the var is hoisted, since var is not scoped to the for loop body. So we must not only evaluate the initialiser for effect, but also destructure it once before the loop starts. FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 27: compare source FAILED!: [reported from test()] Expected value ' function ( ) { delete 4 . . x ; } ', Actual value ' function ( ) { delete ( 4 ) . x ; } ' As elsewhere, (4).x is the standard decompilation of 4..x, ((4.).x), etc. FAILED!: [reported from test()] Testcase js1_7/lexical/regress-346642-03.js failed Bug Number 346642 [ Top of Page ] STATUS: decompilation of destructuring assignment Failure messages were: FAILED!: [reported from test()] decompilation of destructuring assignment: 2 FAILED!: [reported from test()] Expected value 'TypeError: x.t has no properties', Actual value 'TypeError: e1.t has no properties' FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 8 FAILED!: [reported from test()] Expected value 'TypeError: ++x is not a function', Actual value 'TypeError: ++e1 is not a function' FAILED!: [reported from test()] This test passes for me now -- sorry if I'm slow, did you change it, or did I fix something in a recent (v6a vs. v6 perhaps) patch to satisfy it? Testcase js1_7/lexical/regress-346642-06.js failed Bug Number 346642 [ Top of Page ] Expected exit code 0, got 3 Testcase terminated with signal 0 Complete testcase output was: BUGNUMBER: 346642 STATUS: decompilation of destructuring assignment PASSED! decompilation of destructuring assignment: 1 PASSED! decompilation of destructuring assignment: 2 PASSED! decompilation of destructuring assignment: 3 PASSED! decompilation of destructuring assignment: 4 PASSED! decompilation of destructuring assignment: 5 PASSED! decompilation of destructuring assignment: 6 ./js1_7/lexical/regress-346642-06.js:92: TypeError: arguments[0] has no properties This passes too. /be

Small followup fix to jsparse.c next. /be

I'm checking this into the trunk, r=self: Checking in jsparse.c; /cvsroot/mozilla/js/src/jsparse.c,v <-- jsparse.c new revision: 3.250; previous revision: 3.249 done /be

Comment on attachment 240703 [details] [diff] [review] destructuring decompilation, v6a This landed on the trunk, so not obsoleting. Combined 1.8 branch patch next. /be

Comment on attachment 240775 [details] [diff] [review] patch to land on the 1.8 branch Approved for RC2.

Fixed on the 1.8 branch: Checking in jsemit.c; /cvsroot/mozilla/js/src/jsemit.c,v <-- jsemit.c new revision: 3.128.2.53; previous revision: 3.128.2.52 done Checking in jsemit.h; /cvsroot/mozilla/js/src/jsemit.h,v <-- jsemit.h new revision: 3.38.20.17; previous revision: 3.38.20.16 done Checking in jsinterp.c; /cvsroot/mozilla/js/src/jsinterp.c,v <-- jsinterp.c new revision: 3.181.2.66; previous revision: 3.181.2.65 done Checking in jsopcode.c; /cvsroot/mozilla/js/src/jsopcode.c,v <-- jsopcode.c new revision: 3.89.2.62; previous revision: 3.89.2.61 done Checking in jsopcode.tbl; /cvsroot/mozilla/js/src/jsopcode.tbl,v <-- jsopcode.tbl new revision: 3.43.2.19; previous revision: 3.43.2.18 done Checking in jsparse.c; /cvsroot/mozilla/js/src/jsparse.c,v <-- jsparse.c new revision: 3.142.2.62; previous revision: 3.142.2.61 done Checking in jsscan.h; /cvsroot/mozilla/js/src/jsscan.h,v <-- jsscan.h new revision: 3.36.2.5; previous revision: 3.36.2.4 done Checking in jsxdrapi.h; /cvsroot/mozilla/js/src/jsxdrapi.h,v <-- jsxdrapi.h new revision: 1.14.58.9; previous revision: 1.14.58.8 done /be

Checking in regress-346642-01.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-01.js,v <-- regress-346642-01.js new revision: 1.2; previous revision: 1.1 done

I still fail on 1.8.1_20061002|1.9a1_20061002 windows/linux with these. I also fail on macppc 1.8.1 and don't know for macppc 1.9 or mactel 1.8.1/1.9 due to problems with mac builds and mac machines. js1_7/lexical/regress-346642-03.js : 2 expected: TypeError: x.t has no properties actual: TypeError: e1.t has no properties : 8 expected: TypeError: ++x is not a function actual: TypeError: ++e1 is not a function js1_7/lexical/regress-346642-06.js error reason: arguments[0] has no properties

(In reply to comment #114) > I still fail on 1.8.1_20061002|1.9a1_20061002 windows/linux with these. I also > fail on macppc 1.8.1 and don't know for macppc 1.9 or mactel 1.8.1/1.9 due to > problems with mac builds and mac machines. > > js1_7/lexical/regress-346642-03.js > > : 2 expected: TypeError: x.t has no properties actual: TypeError: e1.t has no > properties > : 8 expected: TypeError: ++x is not a function actual: TypeError: ++e1 is not a > function Separate bug, please file it as "Error decompiler uses name of first local allocated to a given stack slot." Thanks, sorry I didn't spot this sooner. > js1_7/lexical/regress-346642-06.js > > error reason: arguments[0] has no properties The function call expression in question is for(x in (function ([y]) { })() ) { } which passes undefined (no arguments), which is arguments[0], which is then destructured via [y], resulting in var y = undefined[0]. Bug in test. /be

filed Bug 355202 Checking in regress-346642-04.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-04.js,v <-- regress-346642-04.js new revision: 1.2; previous revision: 1.1 done Removing regress-346642-05.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-05.js,v <-- regress-346642-05.js new revision: delete; previous revision: 1.1 done Checking in regress-346642-06.js; /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-06.js,v <-- regress-346642-06.js new revision: 1.2; previous revision: 1.1 done hat tip to Jesse for catching my foobar in 04/05.

verified fixed 20061003 1.8 windows/mac*/linux 1.9 windows/linux modulo js1_7/lexical/regress-346642-02.js:2,:8 filed as Bug 355202

14, 15 changed on the trunk between 2007-02-27, 2007-02-28 FAILED!: [reported from test()] decompilation of destructuring assignment: 14: compare source FAILED!: [reported from test()] Expected value ' function ( ) { while ( ( [ ] = e ) ) { } } ', Actual value ' function ( ) { while ( [ ] = e ) { } } ' FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 15: compare source FAILED!: [reported from test()] Expected value ' function ( ) { while ( ( [ ] = a ( b ) ) ) { } } ', Actual value ' function ( ) { while ( [ ] = a ( b ) FAILED!: [reported from test()] out of <http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fjs%2Fsrc&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-02-27+00%3A00%3A00&maxdate=2007-02-28+04%3A00%3A00&cvsroot=%2Fcvsroot> maybe bug 368213? Do we care or should I adjust to the ever changing decompilation again?

(In reply to comment #118) that was with js1_7/lexical/regress-346642-01.js.

Was this line cut off? FAILED!: [reported from test()] Expected value ' function ( ) { while ( ( [ ] = a ( b ) ) ) { } } ', Actual value ' function ( ) { while ( [ ] = a ( b ) If so, then the only difference is that the expected value parenthesizes the loop condition, but the decompiler does not. That's ok, although I recall someone in a bug recently wanting over-parenthesized assignment expressions as conditions. Anyone have the ref? /be

See also bug 351503 for this actual value cut-off problem, which seems like a real regression indeed. /be

(In reply to comment #120) > Was this line cut off? > yes, sorry. was grepping FAIL and forgot to copy/paste the full output. FAILED!: [reported from test()] decompilation of destructuring assignment: 14: compare source FAILED!: [reported from test()] Expected value ' function ( ) { while ( ( [ ] = e ) ) { } } ', Actual value ' function ( ) { while ( [ ] = e ) { } } ' FAILED!: [reported from test()] FAILED!: [reported from test()] decompilation of destructuring assignment: 15: compare source FAILED!: [reported from test()] Expected value ' function ( ) { while ( ( [ ] = a ( b ) ) ) { } } ', Actual value ' function ( ) { while ( [ ] = a ( b ) ) { } } ' FAILED!: [reported from test()] > If so, then the only difference is that the expected value parenthesizes the > loop condition, but the decompiler does not. That's ok, although I recall > someone in a bug recently wanting over-parenthesized assignment expressions as > conditions. Anyone have the ref? bug 371692

Filed bug 379860 for js1_7/lexical/regress-346642-03.js Assertion failure: OBJ_BLOCK_DEPTH(cx, obj) == pcdepth updated tests 14,15 in regress-346642-01.js to reflect changes in decompilation. /cvsroot/mozilla/js/tests/js1_7/lexical/regress-346642-01.js,v <-- regress-346642-01.js new revision: 1.3; previous revision: 1.2