Destructuring assignment has been proposed for inclusion in ES4/JS2, based on array destructuring implemented in Opera, and instead of a group assignment proposal meant to address the same use-cases. These cases include [key, value] pair return from iterators (see bug 326466), the proposed Date.nanotime() method which returns a pair of numbers, and permuting variables (e.g., [a, b] = [b, a]). More details soon, I want to get some syntax nailed down before copying proposal content here. /be

Destructuring assignment The idea is to allow for simple destructuring of array and object data using a syntax that resembles array and object literals. Proposed concrete syntax pattern ::= lhs lhs ::= "{" (field ("," field)*)? "}" | "[" ((lhs | lvalue)? ",")* "]" field ::= ident ":" (lhs | lvalue) lvalue ::= <any lvalue expression allowed in a normal assignment expression> A <pattern> can only appear on the left-hand-side of “=” in the following contexts: * in a plain assignment expression * in a variable initializer in a var, let or const definition * in a variable initializer in a let expression or let statement * in a variable initializer in a for statement If preceded by a var, let, or const the <pattern> must contain <lvalue>s that are all identifiers. If not, the compiler must throw a SyntaxError for the phrase. Note that object literals cannot appear in statement positions, so a plain object destructuring assignment statement { x } = y must be parenthesized either as ({ x } = y) or ({ x }) = y. Proposed semantics The meaning is given separately for the four contexts in which destructuring may appear. Assignments The meaning of the assignment expression P “=” E where P is a pattern and E is an expression is: 1. Evaluate E yielding a value V 2. Assign V to a fresh temporary T 3. If P is an array pattern then 1. For each lvalue N at the top level of P, with its ordinal position I in P 1. Perform N = T[I] 2. For each pattern P’ at the top level of P, with its ordinal position I in P 1. Destructure P’ and T[I] according to this algorithm (from step 2) 4. If P is an object pattern then 1. For each field F with name Q at the top level of P where the right-hand-side of the colon is an lvalue N 1. Perform N = T.Q 2. For each field F with name Q at the top level of P where the right-hand-side of the colon is a pattern P’ 1. Destructure P’ and T.Q according to this algorithm (from step 2) 5. Return T As can be seen, the destructuring assignment is simple syntactic sugar. Variable definitions The meaning of <defining-keyword> <pattern> = expr; where the <defining-keyword> can be var, let, or const and all <lvalue>s in <pattern> are simple identifiers, the set of which is i1, i2, ..., is <defining-keyword> i1, i2, ...; <pattern> = expr ''let'' statements and ''let'' expressions The meaning of let (b0, <pattern> = expr, b1, ...) { ... } where all <lvalue>s in <pattern> are simple identifiers i1, i2, ..., is let (b0 ..., tmp = expr, i1, i2, ... b1, ...) { <pattern> = tmp; ... } where tmp is a fresh temporary variable. Similarly, the meaning of let (b0, <pattern> = expr, b1, ...) E where all <lvalue>s in <pattern> are simple identifiers i1, i2, ..., is let (b0 ..., tmp = expr, i1, i2, ... b1, ...) ( <pattern> = tmp, E ) where tmp is a fresh temporary variable. For statements Below a <var-keyword> is var or let. If a <var-keyword> is present in the for loops, then all the <lvalue>s in the <pattern> must be simple identifiers. Plain ''for'' statement The meaning of for ( <var-keyword>? b0, ..., <pattern> = expr, b1, ... ; e1 ; e2 ) { ... } is that the b0 ... are evaluated, then expr is evaluated and destructured into the <lvalue>s of <lhs>, then the b1 .... are evaluated, and then the loop proceeds by normal rules. For-in statements The meaning of for ( <var-keyword>? <pattern> = expr in e0 ) { ... } where <pattern> has the form “[” i1 “,” i2 “]” where both i1 and i2 may be omitted, is that expr is evalulated and destructured into i1 and i2 before the loop begins; then before each loop iteration i1 receives the property name and i2 receives the property value extracted from the object expression e0. If <pattern> has any other form then the compiler must throw a SyntaxError. For-each statements The meaning of for each ( <var-keyword>? <pattern> = expr in e0 ) { ... } where <lvalue>s in <pattern> are identifiers i1, i2, ... is that expr is evaluated and destructured into i1, i2, ... before the loop begins. Then before at each iteration the value extracted from the object e0 is destructured into the variables i1, i2 .... Examples Multiple-value returns: function f() { return [1,2] } var a, b; [a,b] = f(); Multiple-value returns, some values are not interesting: function f() { return [1,2,3] } var [a,,b] = f(); Going deeper into the array: [a,,[b,,[c]]] = f(); Object destructuring: var { op: a, lhs: b, rhs: c } = getASTNode() Digging deeper into an object: var { op: a, lhs: { op: b }, rhs: c } = getASTNode() Looping across an object: for ( let [name, value] in obj ) print("Name: " + name + ", Value: " + value); Looping across values in an object: for each ( let { name: n, family: { father: f } } in obj ) print("Name: " + n + ", Father: " + f); [This is Lars T. Hansen of Opera's work, iterated with feedback from ECMA TG1 members. Array destructuring is actually implemented in Opera. It's my belief that this proposal will be adopted more or less as-is. /be]

One point about which I'm seeking a change: the semantics proposed require two traversals of each level of destructuring initialiser, one to process lvalues, the second to recursively deal with nested destructuring initialisers. Unless there is a good reason for this, a single left-to-right traversal (a DFS of the entire destructuring left-hand side, in other words) seems better. /be

Reviewers, start your engines. /be

This may be too much off topic but what about pattern matching in general? It would be really nice to clean this up and allow patterns to be used in switch-case statements and mabe even in ifs and other places. I remember with fondness my early days using pattern matching in Haskell.

We have some thoughts on destructuring typecase (type switch), but this ain't no static lazy functional language here! Seriously, destructuring is not matching in the OCaml or Haskell sense. /be

Comment on attachment 223133 [details] [diff] [review] updated patch against updated-to-trunk JS_1_7_ALPHA_BRANCH I landed this on the JS_1_7_ALPHA_BRANCH to aid wider testing. /be

I've noticed since last night's checkins that in the JS shell, you can no longer type: js> function f() { /* function body } js> Now when you hit enter after the first line, you get a syntax error. Is this intentional or a glitch introduced by these checkins?

Also, is it intentional that this works (it does, but I'm curious if it's on purpose): [,,,] = f()

(In reply to comment #9) > I've noticed since last night's checkins which were to the JS_1_7_ALPHA_BRANCH, not the trunk -- just checking/noting. > that in the JS shell, you can no > longer type: > > js> function f() { > /* function body > } > js> > > Now when you hit enter after the first line, you get a syntax error. Is this > intentional or a glitch introduced by these checkins? Bug, fixed just now in jsparse.c 3.170.2.5. (In reply to comment #10) > Also, is it intentional that this works (it does, but I'm curious if it's on > purpose): > > [,,,] = f() Allowed by the proposed Edition 4 spec grammar. But the proposed spec disallows empty object initialisers. We could add a semantic check to avoid complicating the BNF unduly. I'll raise the issue. /be

[]=[] segfaults

Empty initialisers must be prohibited, and are now -- update JS_1_7_ALPHA_BRANCH to get this fix, along with a fix to error reporting for non-lvalue expressions in the "value" positions of destructuring initialisers, and some group assignment optimizations. /be

Will destructuring assignment be supported in an array comprehension and an arguments list? e.g. [key + " = " + value for ([key, value] in obj)]; [[1,2],[3,4],[5,6]].map(function ([a, b]) { return a + b; });

(In reply to comment #14) > Will destructuring assignment be supported in an array comprehension and an > arguments list? > e.g. > [key + " = " + value for ([key, value] in obj)]; > [[1,2],[3,4],[5,6]].map(function ([a, b]) { return a + b; }); > Yes, and also in catch block head -- but not immediately. I want to do this for js1.7, but it won't make Firefox 2 beta 1. /be

I would like to land this patch and then patch catch blocks to use block scope, and *then* patch destructuring catch variable binding. /be

Comment on attachment 232635 [details] [diff] [review] destructuring formal parameters This patch looks bigger than it is due to renaming and some refactoring. I thought I might minimize the patch size with a sed script to unrename, but it doesn't help that much -- the refactoring, often just movement of code, dwarfs the name change froth. The BindVarArgs struct is now BindData, and local variables of that type or pointing to that type are now named "data", not "args", to avoid confusion. BumpFormalCount, which checks for overflow and increments fun->nargs, is factored. BindLocalVariable is factored too, for use from BindVariable and the new BindDestructuringArg. The change to FunctionDef follows the pattern of previous JS_HAS_DESTRUCTURING changes: instead of matching a TOK_NAME only, either a TOK_LB, TOK_LC, or TOK_NAME is allowed. The first two lead to destructuring "lvalue" parsing via PrimaryExpr, followed by CheckDestructuring and code specific to the destructuring form. In this case, the formal destructuring parameters must be turned into a comma expression of, one operand per param, that gets the actual positional parameter and destructures it according to each lvalue in turn. This is done by prepending a synthesized statement-expression to the function body. The BIND_DATA_REPORT_ARGS macro is a bad, bad boy: it expands to two actual parameters to js_ReportCompileErrorNumber, so that when destructuring, the error's source coordinate comes from the already-parsed real lvalue JSParseNode * without the larger destructuring "lvalue"; but when parsing a simple variable binding form, the current token in ts is used. To avoid confusion, all instances and pointers-to-instances of the FindPropertyValue parameter block type, FindPropValData, were renamed from "data" to "fpvd". SetupLexicalBlock was renamed to PushLexicalScope and made more efficient by returning a JSParseNode *, as all parser functions in jsparse.c do. Its unnecessary objp out param was eliminated. The following #if JS_HAS_BLOCK_SCOPE was simplified: >- } else { > #if JS_HAS_BLOCK_SCOPE >- if (tt == TOK_LET) { >- (void) js_GetToken(cx, ts); >- if (!SetupLexicalBlock(cx, ts, tc, &block, &pnlet, &obj)) >- return NULL; >- pn1 = Variables(cx, ts, tc); >- } else >+ } else if (tt == TOK_LET) { >+ (void) js_GetToken(cx, ts); >+ pnlet = PushLexicalScope(cx, ts, tc, &blockInfo); >+ if (!pnlet) >+ return NULL; >+ pn1 = Variables(cx, ts, tc); > #endif >- pn1 = Expr(cx, ts, tc); >+ } else { >+ pn1 = Expr(cx, ts, tc); > } Catch and finally parsing was cleaned up to avoid peeking and then getting. An overweight if/else (where the then clause is significantly larger than the else clause) was prettified by inverting if condition and transposing then and else clauses. /be

With the patch the following test case prints function test([a, [b0, b1]], {prop: c, prop2: [d]}, e) { print("a="+a+" b0="+b0+" b1="+b1+" c="+c+" d="+d+" e="+e); } function test2([a, [b0, b1]], {prop: c, prop2: [d]}, e) { print(Array.prototype.toSource.call(arguments)); } test([0, [1, 2]], {prop: 3, prop2: [4]}, 5); test2([0, [1, 2]], {prop: 3, prop2: [4]}, 5); a=0 b0=1 b1=2 c=3 d=4 e=5 [[0, [1, 2]], {prop:3, prop2:[4]}, 5]

The patch does not detect duplicated parameter names and asserts later: js> function f([a, a]) { print (a); } js> f() Assertion failure: atom, at jsopcode.c:2335 Trace/breakpoint trap Note that duplicated names in let bindings currently are detected only partially: js> { let [a, a] = [1, 2]; print(a); } typein:10: TypeError: redeclaration of variable a: typein:10: { let [a, a] = [1, 2]; print(a); } typein:10: ..........^ while js> let [c, c] = [1, 2]; print(c) 2

Note that the following does not detect duplicated names but at least it does not assert: js> function f([a], [a]) { print(a); } js> f([1], [2]) 2

(In reply to comment #19) > The patch does not detect duplicated parameter names Duplicate parameter names are required to be supported by ECMA-262, for no good reason that I can recall (MS's rep wanted them). They are a botch and a blight. We give a strict warning, in all cases including destructuring, but you need to use -sw with the shell, or otherwise set the prefs or options.strict, etc. properties (in a prior script tag). > and asserts later: > > js> function f([a, a]) { print (a); } > js> f() > Assertion failure: atom, at jsopcode.c:2335 > Trace/breakpoint trap This is in the course of throwing a type error due to dereferencing undefined. It's known that the decompiler is lagging, see bug 346642. > Note that duplicated names in let bindings currently are detected only > partially: > > js> { let [a, a] = [1, 2]; print(a); } > typein:10: TypeError: redeclaration of variable a: > typein:10: { let [a, a] = [1, 2]; print(a); } > typein:10: ..........^ > > while > js> let [c, c] = [1, 2]; print(c) > 2 That's a known bug too, but probably not one on file yet (top-level let is turned into var). Blake, can you file this and link it to the js1.7 tracking bug? Thanks, /be

(In reply to comment #20) > Note that the following does not detect duplicated names but at least it does > not assert: > > js> function f([a], [a]) { print(a); } > js> f([1], [2]) > 2 This works in the same way that function f(a, a) {print(a)} works per ECMA-262, all editions. I don't intend to deviate without an explicit change for Edition 4, and may defer that at this rate (I'll poke the destructuring proposer in ECMA TG1 about this). /be

(In reply to comment #21) > That's a known bug too, but probably not one on file yet (top-level let is > turned into var). This is already filed as bug 346749.

Comment on attachment 232635 [details] [diff] [review] destructuring formal parameters Igor, will you reconsider in light of separate decompiler bug? This patch works for non-error cases, and I'm trying to avoid making a giant patch of all the work, risking conflicts and failing to land partial progress. /be

Comment on attachment 232635 [details] [diff] [review] destructuring formal parameters >+static JSBool >+BindDestructuringArg(JSContext *cx, BindData *data, JSAtom *atom, >+ JSTreeContext *tc) >+{ >+ JSAtomListElement *ale; >+ JSFunction *fun; >+ JSObject *obj, *pobj; >+ JSProperty *prop; >+ JSScopeProperty *sprop; This was set but never used, I had already eliminated it from my tree yesterday. /be

With useless sprop local removed from BindDestructuringArg, and with a fix to FunctionDef to pass &funtc, not tc, to CheckDestructuring (this requires init'ing funtc early, but otherwise has the desired effect of capturing TCF_FUN_HEAVYWEIGHT in funtc.flags for cases such as function ([arguments, b]){print(arguments, b)}, where the arguments built-in local variable name is being overridden). /be

Comment on attachment 232803 [details] [diff] [review] destructuring formal parameters, v2 Assuming that duplicated parameter name crash would be resolved in bug 346642.

Comment on attachment 232803 [details] [diff] [review] destructuring formal parameters, v2 I landed this patch on the trunk. /be

Trying to make small patches here. Next comes a bigger patch that will move catch block code generation out of line, into a TOK_CATCH case in js_EmitTree, so that TOK_LEXICALSCOPE can be composed to get lexical catch variable scope. /be

Comment on attachment 233196 [details] [diff] [review] rename vars, shrink JSStmtInfo, preparing way for catch block scope This patch is checked into the trunk now. /be

As documented in jsparse.[ch], the old tree structure singly-linked ternary catch nodes via pn_kid2, and pointed from the ternary TOK_TRY node to the first catch node via pn_kid2 as well; for try-finally, pn_kid2 was null. The new AST uses the same no-catch case (pn_kid2 null), but if there are catch clauses, we burn a list node to hold them, for greater regularity (code sharing) and to allow O(1) access to the last catch clause. This patch also renameds JSStmtInfo.label to JSStmtInfo.atom, and eliminates JSStmtInfo.blockObj by saving the atomized block object's atom in stmt->atom. A drive-by style infraction cleanup on jspubtd.h tags along here. A diff -w version of this patch is next, for easier reviewing. /be

Comment on attachment 233293 [details] [diff] [review] next step: change try-catch-finally AST >Index: jsparse.c >+ if (js_PeekToken(cx, ts) == TOK_IF) { >+ (void)js_GetToken(cx, ts); /* eat 'if' */ Nit: Replace this with js_MatchToken.

I landed this on the trunk. /be

- Switch catch clauses to use lexical scope (JSOP_ENTER/LEAVEBLOCK) instead of new Object followed by JSOP_ENTER/LEAVEWITH. JSOP_INITCATCHVAR is now a specialized version of JSOP_SETLOCAL. SRC_CATCH changes incompatibly. This patch does not include a XUL_FASTLOAD_FILE_VERSION change, but I will make that change before landing. - LexicalLookup changed to use SCOPE_GET_PROPERTY, so is infallible, removing LL_NOT_FOUND as the static whose address was returned for no-such-variable. This change caused js_InCatchBlock, which was used only by the parser along with js_InWithStatement, to become specialized into js_IsGlobalReference, avoiding two or three walks up the statement stack. - Fix latent bug in js_DefineCompileTimeConstant where the current frame/cg is skipped if a lexical binding is found -- this would make global constants "shine through" local let variables! Fixed by returning true forthwith. - Moved the JSOpLength enum (generated via #include "jsopcode.tbl") from jsinterp.c to jsopcode.c. - Fixed JSOP_SETLOCAL not to have JOF_ASSIGNING or JOF_DETECTING -- these are obviously not needed given the lexical nature of all local ops. - Changed XXX to FIXME: https://bugzilla.mozilla.org/show_bug.cgi?id=346749. /be

Comment on attachment 233658 [details] [diff] [review] lexical scope for catch clauses > JSBool >-js_InCatchBlock(JSTreeContext *tc, JSAtom *atom) >+js_IsGlobalReference(JSTreeContext *tc, JSAtom *atom, JSBool *loopyp) > { > JSStmtInfo *stmt; >+ JSObject *obj; >+ JSScope *scope; > >+ *loopyp = JS_FALSE; > for (stmt = tc->topStmt; stmt; stmt = stmt->down) { >- if (stmt->type == STMT_CATCH && stmt->atom == atom) >- return JS_TRUE; >+ if (stmt->type == STMT_WITH) >+ return JS_FALSE; Talk me through this: can't we have global references within with statements? I'm working through the catch-related codegen changes in another window, but I figured I'd start asking questions early.

(In reply to comment #37) > Talk me through this: can't we have global references within with statements? We can, of course -- this predicate is just answering whether a reference is known to be global (or else a reference error). If with lies along the statement stack then we just don't know. /be

Comment on attachment 233658 [details] [diff] [review] lexical scope for catch clauses Looks good.

Orange tinderboxen on second start after fastload file regeneration => latent bug in block object XDR code, exposed by this change (which results in many more block objects at compile time, one per catch clause, than before). I'm backing out the js/src/* changes and debugging harder. Sorry for the flames. /be

Of course, I have to change the XUL_FASTLOAD_FILE_VERSION again, to avoid bad file bits pre-backout mixing with good code, post-backout. /be

(In reply to comment #41) > Orange tinderboxen on second start after fastload file regeneration => latent > bug in block object XDR code, exposed by this change Filed at bug 348696 and patched. /be

This is the same patch that I landed, but with these changes: - XUL_FASTLOAD_FILE_VERSION changed yet again - Beefy assertion in JSOP_LEAVEBLOCK to catch any non-blocks on fp->blockChain. - GUARDJUMPS renamed GUARDJUMP -- it's not a backpatch chain cursor, it's the index of a single jump needing targeting. /be

Oops, didn't mean to revert the fix for bug 348685. /be

For reviewing ease. /be

I landed "fix, take 3" on the trunk. All should be well now, since bug 348696 has already been fixed (just a little while ago). /be

Argh, latent bug #2 bites: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x00000000 0x230658cd in js_XDRObject (xdr=0x1b746f00, objp=0xbfffdccc) at jsobj.c:4598 4598 clasp = OBJ_GET_CLASS(cx, proto); (gdb) p cx.globalObject $8 = (JSObject *) 0x18dc028 (gdb) p *(JSClass*)($.slots[2]-1) $9 = { name = 0x2da7bc1c "nsXULPrototypeScript compilation scope", flags = 9, addProperty = 0x2300a09e <JS_PropertyStub>, delProperty = 0x2300a09e <JS_PropertyStub>, getProperty = 0x2300a09e <JS_PropertyStub>, setProperty = 0x2300a09e <JS_PropertyStub>, enumerate = 0x2300a0a8 <JS_EnumerateStub>, resolve = 0x2d816208 <nsXULPDGlobalObject_resolve(JSContext*, JSObject*, long)>, convert = 0x2300a0bc <JS_ConvertStub>, finalize = 0x2d8184e0 <nsXULPDGlobalObject_finalize(JSContext*, JSObject*)>, getObjectOps = 0, checkAccess = 0, call = 0, construct = 0, xdrObject = 0, hasInstance = 0, mark = 0, reserveSlots = 0 } This is bug 346144. I'm going to back out yet again, and mark the dependency. /be

(In reply to comment #48) > Argh, latent bug #2 bites: ... > This is bug 346144. Wrong bug, I meant (or mean, now that we've diagnosed bug 346144 as INVALID) bug 348786. Fixing that momentarily. /be

(In reply to comment #49) > Wrong bug, I meant (or mean, now that we've diagnosed bug 346144 as INVALID) > bug 348786. Fixing that momentarily. With that bug fixed, the patch for this bug works for me on any number of restarts (which use the FastLoad file, deserializing block objects created for catch clause lexical scope). /be

What I'm about to land. Same as last time except the jsint count = -1 in the TOK_TRY js_EmitTree case uses 0 as the initializer now, and the assertion that it's non-negative now asserts that it's positive. Empty blocks should never be generated. /be

Comment on attachment 233939 [details] [diff] [review] fix, take 3a This has been checked into the trunk (btw, in spite of the warning from bugzilla, it bugzilla-interdiffs against the "take 3" patch). /be

After this, some much deferred decompiler love, and then rest and recovery for me. /be

It's looking likely that this patch broke the feed parser. I used MOZ_CO_DATE to narrow the regression to between 15 Aug 2006 19:08 PDT and 15 Aug 2006 22:11 PDT. http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-08-15+19%3A08%3A00&maxdate=2006-08-15+22%3A11%3A00&cvsroot=%2Fcvsroot I don't know what exactly is broken. I will try to find a smaller test case than 1000 lines of SAX callbacks.

(In reply to comment #54) > It's looking likely that this patch broke the feed parser. OK, patch 3a definitely did it. I checked with patch -R. I will reduce this. Test results in XPCShell: Before tests: 156 ran: 156 passed: 156 After tests: 156 ran: 156 passed: 58

Expected Output: ---------------------------------------- The Bag: [xpconnect wrapped nsIPropertyBag2 @ 0x525dd0 (native @ 0x5185a0)] fields to obj -- open outer -- number1 -- foo,bar,baz ---- open inner ---- Exception: container.fields has no properties ---- prop: null ---- close inner ---- open inner ---- Exception: container.fields has no properties ---- prop: null ---- close inner ---- open inner ---- Exception: container.fields has no properties ---- prop: null ---- close inner -- close outer -- open outer -- number2 -- qux,quux,quuux ---- open inner ---- Exception: container.fields has no properties ---- prop: null ---- close inner ---- open inner ---- Exception: container.fields has no properties ---- prop: null ---- close inner ---- open inner ---- Exception: container.fields has no properties ---- prop: null ---- close inner -- close outer end fields to obj Output After Patch ---------------------------------------- The Bag: [xpconnect wrapped nsIPropertyBag2 @ 0x53a560 (native @ 0x539fb0)] fields to obj -- open outer -- number1 -- foo,bar,baz ---- open inner /Users/sayrer/bonecho/mozilla/fb-debug/dist/bin/loopcatch.js:22: TypeError: e has no properties

this one should be easier to follow

This testcase shows the essence of the problem.

While we do know the slot at parse-time, we need to adjust it at compile-time to account for the block depth, just like BindNameToSlot via LexicalLookup does.

Thanks to mrbkap, he says r=him. /be

(In reply to comment #58) > Created an attachment (id=234068) [edit] > 2nd pass: replace variables left over from my app > > this one should be easier to follow Thanks, and sorry for the regression. /be

This also renames JSOP_INITCATCHVAR to JSOP_SETLOCALPOP (same functionality, stack arity, etc.). It turns out we want popping setters for destructuring at least, and we would be faster and more ECMA-compliant in general if we had such opcodes. /be

Pre-existing code in LetBlock was not combining consequents under one if test, making a gratuitous else (gratuitous jumps). /be

What I'm checking in later tonight (have to run now). /be

This has been baking for too long. It's required for js1.7, and needed as a prerequisite other approved patches. Sorry I didn't pull it together till now. /be

Comment on attachment 237431 [details] [diff] [review] 1.8 branch patch of lexical/destructuring catch a=schrep for drivers.

Fixed by the final patches for bug 346642. /be