From 164c7b48ec239fcf58957da93d117c2facfc51ee Mon Sep 17 00:00:00 2001 From: "nboyd%atg.com" Date: Wed, 15 Aug 2007 12:00:05 +0000 Subject: [PATCH] Implement correct semantics of storage class (global, var, let) for destructuring assignment. git-svn-id: svn://10.0.0.236/trunk@232109 18797224-902f-48f8-a5cc-f745e15eee43 --- .../src/org/mozilla/javascript/IRFactory.java | 238 +++++++++--------- .../src/org/mozilla/javascript/Node.java | 5 +- .../mozilla/javascript/NodeTransformer.java | 26 +- .../src/org/mozilla/javascript/Parser.java | 89 ++++--- .../javascript/resources/Messages.properties | 7 + .../js/tests/js1_7/geniter/regress-366941.js | 4 +- 6 files changed, 205 insertions(+), 164 deletions(-) diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/IRFactory.java b/mozilla/js/rhino/src/org/mozilla/javascript/IRFactory.java index 61f3dac53eb..c717efac546 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/IRFactory.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/IRFactory.java @@ -517,48 +517,18 @@ final class IRFactory return loop; } - - /* - * Return true iff "n" is an array literal of length 2. This is - * surprisingly hard to determine due to the "skip_indexes" property. - */ - private boolean isArrayLitOfLengthTwo(Node n) { - int[] skipIndexes = (int[])n.getProp(Node.SKIP_INDEXES_PROP); - if (n.getFirstChild() != null && n.getLastChild() != null && - n.getFirstChild().getNext() == n.getLastChild() && - skipIndexes == null) - { - // two elements in tree, no skipped indexes - return true; - } - if (n.getFirstChild() != null && n.getFirstChild() == n.getLastChild() && - skipIndexes != null && skipIndexes.length == 1 && - skipIndexes[0] < 2) - { - // one element in tree, one skipped index - return true; - } - if (n.getFirstChild() == null && - skipIndexes != null && skipIndexes.length == 2 && - skipIndexes[0] == 0 && skipIndexes[1] == 1) - { - // no elements in tree, two skipped indices - return true; - } - return false; - } /** * For .. In * */ - Node createForIn(Node loop, Node lhs, Node obj, Node body, + Node createForIn(int declType, Node loop, Node lhs, Node obj, Node body, boolean isForEach) { - int type = lhs.getType(); - + int destructuring = -1; + int destructuringLen = 0; Node lvalue; - boolean destructuring = false; + int type = lhs.getType(); if (type == Token.VAR || type == Token.LET) { /* * check that there was only one variable given. @@ -570,18 +540,20 @@ final class IRFactory if (lhs.getFirstChild() != lastChild) { parser.reportError("msg.mult.index"); } - lvalue = Node.newString(Token.NAME, lastChild.getString()); - } else if (type == Token.ARRAYLIT || type == Token.OBJECTLIT) { - // destructuring assignment is only allowed in for each or - // with an array type of length 2 (to hold key and value) - if (!isForEach && (type == Token.OBJECTLIT || - !isArrayLitOfLengthTwo(lhs))) + if (lastChild.getType() == Token.ARRAYLIT || + lastChild.getType() == Token.OBJECTLIT) { - parser.reportError("msg.bad.for.in.lhs"); - return obj; + type = destructuring = lastChild.getType(); + lvalue = lastChild; + destructuringLen = lastChild.getIntProp( + Node.DESTRUCTURING_ARRAY_LENGTH, 0); + } else { + lvalue = Node.newString(Token.NAME, lastChild.getString()); } - lvalue = lhs; - destructuring = true; + } else if (type == Token.ARRAYLIT || type == Token.OBJECTLIT) { + destructuring = type; + lvalue = lhs; + destructuringLen = lhs.getIntProp(Node.DESTRUCTURING_ARRAY_LENGTH, 0); } else { lvalue = makeReference(lhs); if (lvalue == null) { @@ -591,9 +563,9 @@ final class IRFactory } Node localBlock = new Node(Token.LOCAL_BLOCK); - int initType = (isForEach) ? Token.ENUM_INIT_VALUES : - (destructuring) ? Token.ENUM_INIT_ARRAY : - Token.ENUM_INIT_KEYS; + int initType = (isForEach) ? Token.ENUM_INIT_VALUES : + (destructuring != -1) ? Token.ENUM_INIT_ARRAY : + Token.ENUM_INIT_KEYS; Node init = new Node(initType, obj); init.putProp(Node.LOCAL_BLOCK_PROP, localBlock); Node cond = new Node(Token.ENUM_NEXT); @@ -603,8 +575,16 @@ final class IRFactory Node newBody = new Node(Token.BLOCK); Node assign; - if (destructuring) { - assign = createAssignment(Token.ASSIGN, lvalue, id); + if (destructuring != -1) { + assign = createDestructuringAssignment(declType, lvalue, id); + int len = assign.getIntProp(Node.DESTRUCTURING_ARRAY_LENGTH, 0); + if (!isForEach && (destructuring == Token.OBJECTLIT || + destructuringLen!= 2)) + { + // destructuring assignment is only allowed in for..each or + // with an array type of length 2 (to hold key and value) + parser.reportError("msg.bad.for.in.destruct"); + } } else { assign = simpleAssignment(lvalue, id); } @@ -832,7 +812,7 @@ final class IRFactory return result; } - Node createArrayLiteral(ObjArray elems, int skipCount) + Node createArrayLiteral(ObjArray elems, int skipCount, int destructuringLen) { int length = elems.size(); int[] skipIndexes = null; @@ -852,6 +832,7 @@ final class IRFactory if (skipCount != 0) { array.putProp(Node.SKIP_INDEXES_PROP, skipIndexes); } + array.putIntProp(Node.DESTRUCTURING_ARRAY_LENGTH, destructuringLen); return array; } @@ -1333,7 +1314,7 @@ final class IRFactory parser.reportError("msg.bad.destruct.op"); return right; } - return createDestructuringAssignment(left, right); + return createDestructuringAssignment(-1, left, right); } parser.reportError("msg.bad.assign.left"); return right; @@ -1395,23 +1376,25 @@ final class IRFactory * as an array or object literal and a right hand side expression, * rewrite as a series of assignments to the variables defined in * left from property accesses to the expression on the right. + * @param type declaration type: Token.VAR or Token.LET or -1 * @param left array or object literal containing NAME nodes for * variables to assign * @param right expression to assign from * @return expression that performs a series of assignments to * the variables defined in left */ - Node createDestructuringAssignment(Node left, Node right) + Node createDestructuringAssignment(int type, Node left, Node right) { String tempName = parser.currentScriptOrFn.getNextTempName(); - Node result = destructuringAssignmentHelper(left, right, tempName); + Node result = destructuringAssignmentHelper(type, left, right, + tempName); Node comma = result.getLastChild(); comma.addChildToBack(createName(tempName)); return result; } - private Node destructuringAssignmentHelper(Node left, Node right, - String tempName) + private Node destructuringAssignmentHelper(int variableType, Node left, + Node right, String tempName) { Node result = createScopeNode(Token.LETEXPR, parser.getCurrentLineNumber()); @@ -1420,75 +1403,88 @@ final class IRFactory try { parser.pushScope(result); parser.defineSymbol(Token.LET, tempName); - Node comma = new Node(Token.COMMA); - result.addChildToBack(comma); - boolean empty = true; - int type = left.getType(); - if (type == Token.ARRAYLIT) { - int index = 0; - int[] skipIndices = (int[])left.getProp(Node.SKIP_INDEXES_PROP); - int skip = 0; - for (Node n = left.getFirstChild(); n != null; n = n.getNext()) - { - if (skipIndices != null) { - while (skip < skipIndices.length && - skipIndices[skip] == index) { - skip++; - index++; - } - } - Node rightElem = new Node(Token.GETELEM, - createName(tempName), - createNumber(index)); - if (n.getType() == Token.NAME) { - comma.addChildToBack(new Node(Token.SETNAME, - createName(Token.BINDNAME, n.getString(), null), - rightElem)); - } else { - comma.addChildToBack( - destructuringAssignmentHelper(n, rightElem, - parser.currentScriptOrFn.getNextTempName())); - } - index++; - empty = false; - } - } else if (type == Token.OBJECTLIT) { - int index = 0; - Object[] propertyIds = (Object[]) - left.getProp(Node.OBJECT_IDS_PROP); - for (Node n = left.getFirstChild(); n != null; n = n.getNext()) - { - Object id = propertyIds[index]; - Node rightElem = id instanceof String - ? new Node(Token.GETPROP, - createName(tempName), - createString((String)id)) - : new Node(Token.GETELEM, - createName(tempName), - createNumber(((Number)id).intValue())); - if (n.getType() == Token.NAME) { - comma.addChildToBack(new Node(Token.SETNAME, - createName(Token.BINDNAME, n.getString(), null), - rightElem)); - } else { - comma.addChildToBack( - destructuringAssignmentHelper(n, rightElem, - parser.currentScriptOrFn.getNextTempName())); - } - index++; - empty = false; - } - } else { - parser.reportError("msg.bad.assign.left"); - } - if (empty) { - // Don't want a COMMA node with no children. Just add a zero. - comma.addChildToBack(createNumber(0)); - } - return result; } finally { parser.popScope(); } + Node comma = new Node(Token.COMMA); + result.addChildToBack(comma); + boolean empty = true; + int type = left.getType(); + if (type == Token.ARRAYLIT) { + int index = 0; + int[] skipIndices = (int[])left.getProp(Node.SKIP_INDEXES_PROP); + int skip = 0; + Node n = left.getFirstChild(); + for (;;) { + if (skipIndices != null) { + while (skip < skipIndices.length && + skipIndices[skip] == index) { + skip++; + index++; + } + } + if (n == null) + break; + Node rightElem = new Node(Token.GETELEM, + createName(tempName), + createNumber(index)); + if (n.getType() == Token.NAME) { + String name = n.getString(); + comma.addChildToBack(new Node(Token.SETNAME, + createName(Token.BINDNAME, name, null), + rightElem)); + if (variableType != -1) { + parser.defineSymbol(variableType, name); + } + } else { + comma.addChildToBack( + destructuringAssignmentHelper(variableType, n, + rightElem, + parser.currentScriptOrFn.getNextTempName())); + } + index++; + empty = false; + n = n.getNext(); + } + } else if (type == Token.OBJECTLIT) { + int index = 0; + Object[] propertyIds = (Object[]) + left.getProp(Node.OBJECT_IDS_PROP); + for (Node n = left.getFirstChild(); n != null; n = n.getNext()) + { + Object id = propertyIds[index]; + Node rightElem = id instanceof String + ? new Node(Token.GETPROP, + createName(tempName), + createString((String)id)) + : new Node(Token.GETELEM, + createName(tempName), + createNumber(((Number)id).intValue())); + if (n.getType() == Token.NAME) { + String name = n.getString(); + comma.addChildToBack(new Node(Token.SETNAME, + createName(Token.BINDNAME, name, null), + rightElem)); + if (variableType != -1) { + parser.defineSymbol(variableType, name); + } + } else { + comma.addChildToBack( + destructuringAssignmentHelper(variableType, n, + rightElem, + parser.currentScriptOrFn.getNextTempName())); + } + index++; + empty = false; + } + } else { + parser.reportError("msg.bad.assign.left"); + } + if (empty) { + // Don't want a COMMA node with no children. Just add a zero. + comma.addChildToBack(createNumber(0)); + } + return result; } Node createUseLocal(Node localBlock) diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/Node.java b/mozilla/js/rhino/src/org/mozilla/javascript/Node.java index 4f05e3c2f5f..ec3994c6e33 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/Node.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/Node.java @@ -86,7 +86,8 @@ public class Node CONTROL_BLOCK_PROP = 18, // flags a control block that can drop off PARENTHESIZED_PROP = 19, // expression is parenthesized GENERATOR_END_PROP = 20, - LAST_PROP = 20; + DESTRUCTURING_ARRAY_LENGTH = 21, + LAST_PROP = 21; // values of ISNUMBER_PROP to specify // which of the children are Number types @@ -534,6 +535,8 @@ public class Node case CONTROL_BLOCK_PROP: return "control_block_prop"; case PARENTHESIZED_PROP: return "parenthesized_prop"; case GENERATOR_END_PROP: return "generator_end"; + case DESTRUCTURING_ARRAY_LENGTH: + return "destructuring_array_length"; default: Kit.codeBug(); } diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/NodeTransformer.java b/mozilla/js/rhino/src/org/mozilla/javascript/NodeTransformer.java index 9fa917bce84..27efa50c74f 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/NodeTransformer.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/NodeTransformer.java @@ -305,17 +305,23 @@ public class NodeTransformer // Move cursor to next before createAssignment gets chance // to change n.next Node n = cursor; - if (n.getType() != Token.NAME) Kit.codeBug(); cursor = cursor.getNext(); - if (!n.hasChildren()) - continue; - Node init = n.getFirstChild(); - n.removeChild(init); - n.setType(Token.BINDNAME); - n = new Node(type == Token.CONST ? - Token.SETCONST : - Token.SETNAME, - n, init); + if (n.getType() == Token.NAME) { + if (!n.hasChildren()) + continue; + Node init = n.getFirstChild(); + n.removeChild(init); + n.setType(Token.BINDNAME); + n = new Node(type == Token.CONST ? + Token.SETCONST : + Token.SETNAME, + n, init); + } else { + // May be a destructuring assignment already transformed + // to a LETEXPR + if (n.getType() != Token.LETEXPR) + throw Kit.codeBug(); + } Node pop = new Node(Token.EXPR_VOID, n, node.getLineno()); result.addChildToBack(pop); } diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/Parser.java b/mozilla/js/rhino/src/org/mozilla/javascript/Parser.java index ca7d7db97f6..3e6086b7b6b 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/Parser.java +++ b/mozilla/js/rhino/src/org/mozilla/javascript/Parser.java @@ -873,6 +873,7 @@ public class Parser Node cond; // Node cond is also object in 'foo in object' Node incr = null; Node body; + int declType = -1; // See if this is a for each () instead of just a for () if (matchToken(Token.NAME)) { @@ -895,6 +896,7 @@ public class Parser consumeToken(); // consume the token decompiler.addToken(tt); init = variables(true, true, tt); + declType = tt; } else { init = expr(true); @@ -933,7 +935,8 @@ public class Parser if (incr == null) { // cond could be null if 'in obj' got eaten // by the init node. - pn = nf.createForIn(loop, init, cond, body, isForEach); + pn = nf.createForIn(declType, loop, init, cond, body, + isForEach); } else { pn = nf.createFor(loop, init, cond, incr, body); } @@ -1367,50 +1370,67 @@ public class Parser { Node result = nf.createVariables(inStatement ? declType : Token.COMMA, ts.getLineno()); - + Node destructuringInit = null; boolean first = true; for (;;) { + Node destructuring = null; + String s = null; int tt = peekToken(); if (tt == Token.LB || tt == Token.LC) { // Destructuring assignment, e.g., var [a,b] = ... - Node n = expr(inFor); - return inStatement && !inFor - ? new Node(Token.EXPR_VOID, n, ts.getLineno()) - : n; + destructuring = primaryExpr(); + } else { + // Simple variable name + mustMatchToken(Token.NAME, "msg.bad.var"); + s = ts.getString(); + + if (!first) + decompiler.addToken(Token.COMMA); + first = false; + + decompiler.addName(s); + defineSymbol(declType, s); } - mustMatchToken(Token.NAME, "msg.bad.var"); - String s = ts.getString(); - - if (!first) - decompiler.addToken(Token.COMMA); - first = false; - - decompiler.addName(s); - defineSymbol(declType, s); - + Node init = null; if (matchToken(Token.ASSIGN)) { decompiler.addToken(Token.ASSIGN); init = assignExpr(inFor); } - - if (inStatement) { - Node name = nf.createName(s); - if (init != null) - nf.addChildToBack(name, init); - nf.addChildToBack(result, name); - } else if (init != null) { - Node string = nf.createString(s); - string.setScope(currentScope); - nf.addChildToBack(result, - nf.createBinary(Token.SETVAR, string, init)); + + if (destructuring != null) { + if (init == null) { + if (!inFor) + reportError("msg.destruct.assign.no.init"); + nf.addChildToBack(result, destructuring); + } else { + if (destructuringInit == null) { + destructuringInit = new Node(Token.COMMA); + } + nf.addChildToBack(result, + nf.createDestructuringAssignment(declType, + destructuring, init)); + } + } else { + if (inStatement) { + Node name = nf.createName(s); + if (init != null) + nf.addChildToBack(name, init); + nf.addChildToBack(result, name); + } else if (init != null) { + Node string = nf.createString(s); + string.setScope(currentScope); + nf.addChildToBack(result, + nf.createBinary(Token.SETVAR, string, init)); + } } - + if (!matchToken(Token.COMMA)) break; } return result; } + private Node let(boolean isStatement) throws IOException, ParserException @@ -2203,7 +2223,8 @@ public class Parser Node loop = enterLoop(null, true); try { - return nf.createForIn(loop, init, iterator, body, isForEach); + return nf.createForIn(Token.LET, loop, init, iterator, body, + isForEach); } finally { exitLoop(false); } @@ -2225,6 +2246,7 @@ public class Parser case Token.LB: { ObjArray elems = new ObjArray(); int skipCount = 0; + int destructuringLen = 0; decompiler.addToken(Token.LB); boolean after_lb_or_comma = true; for (;;) { @@ -2242,6 +2264,13 @@ public class Parser } else if (tt == Token.RB) { consumeToken(); decompiler.addToken(Token.RB); + // for ([a,] in obj) is legal, but for ([a] in obj) is + // not since we have both key and value supplied. The + // trick is that [a,] and [a] are equivalent in other + // array literal contexts. So we calculate a special + // length value just for destructuring assignment. + destructuringLen = elems.size() + + (after_lb_or_comma ? 1 : 0); break; } else if (skipCount == 0 && elems.size() == 1 && tt == Token.FOR) @@ -2276,7 +2305,7 @@ public class Parser after_lb_or_comma = false; } } - return nf.createArrayLiteral(elems, skipCount); + return nf.createArrayLiteral(elems, skipCount, destructuringLen); } case Token.LC: { diff --git a/mozilla/js/rhino/src/org/mozilla/javascript/resources/Messages.properties b/mozilla/js/rhino/src/org/mozilla/javascript/resources/Messages.properties index 881a478b0fb..7aea19bedeb 100644 --- a/mozilla/js/rhino/src/org/mozilla/javascript/resources/Messages.properties +++ b/mozilla/js/rhino/src/org/mozilla/javascript/resources/Messages.properties @@ -114,6 +114,10 @@ msg.bad.for.in.lhs =\ msg.mult.index =\ Only one variable allowed in for..in loop. +msg.bad.for.in.destruct =\ + Left hand side of for..in loop must be an array of length 2 to accept \ + key/value pair. + msg.cant.convert =\ Can''t convert to type "{0}". @@ -482,6 +486,9 @@ msg.equal.as.assign =\ msg.var.hides.arg =\ Variable {0} hides argument +msg.destruct.assign.no.init =\ + Missing = in destructuring declaration + # ScriptRuntime msg.no.properties =\ {0} has no properties. diff --git a/mozilla/js/tests/js1_7/geniter/regress-366941.js b/mozilla/js/tests/js1_7/geniter/regress-366941.js index a280b9cc924..9e743e755a1 100755 --- a/mozilla/js/tests/js1_7/geniter/regress-366941.js +++ b/mozilla/js/tests/js1_7/geniter/regress-366941.js @@ -93,7 +93,7 @@ function test() // syntax error in js17 var iter2 = gen(list2); - expect = 'SyntaxError: invalid for/in left-hand side'; + expect = /SyntaxError: (invalid for.in left-hand side|Left hand side of for..in loop must be an array of length 2 to accept key.value pair.)/; actual = ''; try @@ -108,7 +108,7 @@ function test() actual = ex + ''; } - reportCompare(expect, actual, summary + ': 4'); + reportMatch(expect, actual, summary + ': 4'); exitFunc ('test'); }