bug 591437, r=brendan: can't reliably use pn_cookie for function args
authorDave Herman <dherman@mozilla.com>
Tue, 07 Sep 2010 16:27:52 -0700
changeset 53599 4f0f0d22e3c5c4235642786c850c51b88f22e0f3
parent 53598 537dd6f60dcce86217b453d238e0244eafec8ab4
child 53600 45f147e08faf61b0ab5aa7db2ad7428da9aaf575
push idunknown
push userunknown
push dateunknown
reviewersbrendan
bugs591437
milestone2.0b6pre
bug 591437, r=brendan: can't reliably use pn_cookie for function args
js/src/jsreflect.cpp
js/src/tests/js1_8_5/extensions/reflect-parse.js
--- a/js/src/jsreflect.cpp
+++ b/js/src/jsreflect.cpp
@@ -492,18 +492,19 @@ NodeBuilder::newNode(ASTType type, Token
     JS_ASSERT(type > AST_ERROR && type < AST_LIMIT);
 
     Value tv;
 
     JSObject *node = NewNonFunction<WithProto::Class>(cx, &js_ObjectClass, NULL, NULL);
     if (!node ||
         !setNodeLoc(node, pos) ||
         !atomValue(nodeTypeNames[type], &tv) ||
-        !setProperty(node, "type", tv))
+        !setProperty(node, "type", tv)) {
         return false;
+    }
 
     *dst = node;
     return true;
 }
 
 bool
 NodeBuilder::newArray(NodeVector &elts, Value *dst)
 {
@@ -1830,18 +1831,19 @@ ASTSerializer::statement(JSParseNode *pn
             VarDeclKind kind = VARDECL_LET;
             NodeVector dtors(cx);
             Value patt, init, dtor;
 
             if (!pattern(pnlet->pn_head, &kind, &patt) ||
                 !expression(prelude->pn_kid, &init) ||
                 !builder.variableDeclarator(patt, init, &pnlet->pn_pos, &dtor) ||
                 !dtors.append(dtor) ||
-                !builder.variableDeclaration(dtors, kind, &pnlet->pn_pos, &var))
+                !builder.variableDeclaration(dtors, kind, &pnlet->pn_pos, &var)) {
                 return false;
+            }
         }
 
         JSParseNode *head = loop->pn_left;
         JS_ASSERT(PN_TYPE(head) == TOK_IN);
 
         bool isForEach = loop->pn_iflags & JSITER_FOREACH;
 
         Value expr, stmt;
@@ -1999,18 +2001,17 @@ ASTSerializer::comprehension(JSParseNode
 {
     LOCAL_ASSERT(PN_TYPE(pn) == TOK_FOR);
 
     NodeVector blocks(cx);
 
     JSParseNode *next = pn;
     while (PN_TYPE(next) == TOK_FOR) {
         Value block;
-        if (!comprehensionBlock(next, &block) ||
-            !blocks.append(block))
+        if (!comprehensionBlock(next, &block) || !blocks.append(block))
             return false;
         next = next->pn_right;
     }
 
     Value filter = MagicValue(JS_SERIALIZE_NO_NODE);
 
     if (PN_TYPE(next) == TOK_IF) {
         if (!optExpression(next->pn_kid1, &filter))
@@ -2035,18 +2036,17 @@ ASTSerializer::generatorExpression(JSPar
 {
     LOCAL_ASSERT(PN_TYPE(pn) == TOK_FOR);
 
     NodeVector blocks(cx);
 
     JSParseNode *next = pn;
     while (PN_TYPE(next) == TOK_FOR) {
         Value block;
-        if (!comprehensionBlock(next, &block) ||
-            !blocks.append(block))
+        if (!comprehensionBlock(next, &block) || !blocks.append(block))
             return false;
         next = next->pn_right;
     }
 
     Value filter = MagicValue(JS_SERIALIZE_NO_NODE);
 
     if (PN_TYPE(next) == TOK_IF) {
         if (!optExpression(next->pn_kid1, &filter))
@@ -2556,18 +2556,19 @@ ASTSerializer::objectPattern(JSParseNode
         return false;
 
     for (JSParseNode *next = pn->pn_head; next; next = next->pn_next) {
         LOCAL_ASSERT(PN_OP(next) == JSOP_INITPROP);
 
         Value key, patt, prop;
         if (!propertyName(next->pn_left, &key) ||
             !pattern(next->pn_right, pkind, &patt) ||
-            !builder.propertyPattern(key, patt, &next->pn_pos, &prop))
+            !builder.propertyPattern(key, patt, &next->pn_pos, &prop)) {
             return false;
+        }
 
         JS_ALWAYS_TRUE(elts.append(prop)); /* space check above */
     }
 
     return builder.objectPattern(elts, &pn->pn_pos, dst);
 }
 
 bool
@@ -2710,26 +2711,35 @@ ASTSerializer::functionArgs(JSParseNode 
      * Arguments are found in potentially two different places: 1) the
      * argsbody sequence (which ends with the body node), or 2) a
      * destructuring initialization at the beginning of the body. Loop
      * |arg| through the argsbody and |destruct| through the initial
      * destructuring assignments, stopping only when we've exhausted
      * both.
      */
     while ((arg && arg != pnbody) || destruct) {
-        if (arg && arg != pnbody && arg->frameSlot() == i) {
-            if (!identifier(arg, &node) ||
-                !args.append(node))
+        if (destruct && destruct->pn_right->frameSlot() == i) {
+            if (!pattern(destruct->pn_left, NULL, &node) || !args.append(node))
+                return false;
+            destruct = destruct->pn_next;
+        } else if (arg && arg != pnbody) {
+            /*
+             * We don't check that arg->frameSlot() == i since we
+             * can't call that method if the arg def has been turned
+             * into a use, e.g.:
+             *
+             *     function(a) { function a() { } }
+             *
+             * There's no other way to ask a non-destructuring arg its
+             * index in the formals list, so we rely on the ability to
+             * ask destructuring args their index above.
+             */
+            if (!identifier(arg, &node) || !args.append(node))
                 return false;
             arg = arg->pn_next;
-        } else if (destruct && destruct->pn_right->frameSlot() == i) {
-            if (!pattern(destruct->pn_left, NULL, &node) ||
-                !args.append(node))
-                return false;
-            destruct = destruct->pn_next;
         } else {
             LOCAL_NOT_REACHED("missing function argument");
         }
         ++i;
     }
 
     return true;
 }
@@ -2737,18 +2747,17 @@ ASTSerializer::functionArgs(JSParseNode 
 bool
 ASTSerializer::functionBody(JSParseNode *pn, TokenPos *pos, Value *dst)
 {
     NodeVector elts(cx);
 
     /* We aren't sure how many elements there are up front, so we'll check each append. */
     for (JSParseNode *next = pn; next; next = next->pn_next) {
         Value child;
-        if (!sourceElement(next, &child) ||
-            !elts.append(child))
+        if (!sourceElement(next, &child) || !elts.append(child))
             return false;
     }
 
     return builder.blockStatement(elts, pos, dst);
 }
 
 } /* namespace js */
 
@@ -2830,16 +2839,17 @@ static JSFunctionSpec static_methods[] =
 JSObject *
 js_InitReflectClass(JSContext *cx, JSObject *obj)
 {
     JSObject *Reflect = NewNonFunction<WithProto::Class>(cx, &js_ReflectClass, NULL, obj);
     if (!Reflect)
         return NULL;
 
     if (!JS_DefineProperty(cx, obj, js_Reflect_str, OBJECT_TO_JSVAL(Reflect),
-                           JS_PropertyStub, JS_PropertyStub, 0))
+                           JS_PropertyStub, JS_PropertyStub, 0)) {
         return NULL;
+    }
 
     if (!JS_DefineFunctions(cx, Reflect, static_methods))
         return NULL;
 
     return Reflect;
 }
--- a/js/src/tests/js1_8_5/extensions/reflect-parse.js
+++ b/js/src/tests/js1_8_5/extensions/reflect-parse.js
@@ -190,16 +190,33 @@ assertDecl("var x, y, z",
                     { id: ident("y"), init: null },
                     { id: ident("z"), init: null }]));
 assertDecl("function foo() { }",
            funDecl(ident("foo"), [], blockStmt([])));
 assertDecl("function foo() { return 42 }",
            funDecl(ident("foo"), [], blockStmt([returnStmt(lit(42))])));
 
 
+// Bug 591437: rebound args have their defs turned into uses
+assertDecl("function f(a) { function a() { } }",
+           funDecl(ident("f"), [ident("a")], blockStmt([funDecl(ident("a"), [], blockStmt([]))])));
+assertDecl("function f(a,b,c) { function b() { } }",
+           funDecl(ident("f"), [ident("a"),ident("b"),ident("c")], blockStmt([funDecl(ident("b"), [], blockStmt([]))])));
+assertDecl("function f(a,[x,y]) { function a() { } }",
+           funDecl(ident("f"),
+                   [ident("a"), arrPatt([ident("x"), ident("y")])],
+                   blockStmt([funDecl(ident("a"), [], blockStmt([]))])));
+
+// Bug 591450: this test currently crashes because of a bug in jsparse
+// assertDecl("function f(a,[x,y],b,[w,z],c) { function b() { } }",
+//            funDecl(ident("f"),
+//                    [ident("a"), arrPatt([ident("x"), ident("y")]), ident("b"), arrPatt([ident("w"), ident("z")]), ident("c")],
+//                    blockStmt([funDecl(ident("b"), [], blockStmt([]))])));
+
+
 // expressions
 
 assertExpr("true", lit(true));
 assertExpr("false", lit(false));
 assertExpr("42", lit(42));
 assertExpr("(/asdf/)", lit(/asdf/));
 assertExpr("this", thisExpr);
 assertExpr("foo", ident("foo"));