Bug 410649: function statement and destructuring parameter name clash now favours the function. r,a=brendan
authorigor@mir2.org
Sun, 20 Jan 2008 02:34:06 -0800
changeset 10466 b556ff5b01a85619f3ec85f6f1d8b17fdf760aa6
parent 10465 07dd35e6c3ffc3c821d7bc580154eee660668c32
child 10467 a61fcf408f66d87e144725d7f3d7fc9f7974847f
push id1
push userbsmedberg@mozilla.com
push dateThu, 20 Mar 2008 16:49:24 +0000
treeherderautoland@61007906a1f8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbrendan
bugs410649
milestone1.9b3pre
Bug 410649: function statement and destructuring parameter name clash now favours the function. r,a=brendan
js/src/jsemit.c
js/src/jsinterp.c
js/src/jsparse.c
js/src/jsparse.h
js/src/jsxdrapi.h
--- a/js/src/jsemit.c
+++ b/js/src/jsemit.c
@@ -3923,16 +3923,24 @@ GettableNoteForNextOp(JSCodeGenerator *c
         if (offset == target && SN_IS_GETTABLE(sn))
             return JS_TRUE;
         offset += SN_DELTA(sn);
     }
     return JS_FALSE;
 }
 #endif
 
+/* Top-level named functions need a nop for decompilation. */
+static JSBool
+EmitFunctionDefNop(JSContext *cx, JSCodeGenerator *cg, uintN index)
+{
+    return js_NewSrcNote2(cx, cg, SRC_FUNCDEF, (ptrdiff_t)index) >= 0 &&
+           js_Emit1(cx, cg, JSOP_NOP) >= 0;
+}
+
 JSBool
 js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
 {
     JSBool ok, useful, wantval;
     JSStmtInfo *stmt, stmtInfo;
     ptrdiff_t top, off, tmp, beq, jmp;
     JSParseNode *pn2, *pn3;
     JSAtom *atom;
@@ -3957,42 +3965,56 @@ js_EmitTree(JSContext *cx, JSCodeGenerat
     pn->pn_offset = top = CG_OFFSET(cg);
 
     /* Emit notes to tell the current bytecode's source line number. */
     UPDATE_LINE_NUMBER_NOTES(cx, cg, pn);
 
     switch (pn->pn_type) {
       case TOK_FUNCTION:
       {
+        JSFunction *fun;
         void *cg2mark;
         JSCodeGenerator *cg2;
-        JSFunction *fun;
         uintN slot;
 
 #if JS_HAS_XML_SUPPORT
         if (pn->pn_arity == PN_NULLARY) {
             if (js_Emit1(cx, cg, JSOP_GETFUNNS) < 0)
                 return JS_FALSE;
             break;
         }
 #endif
 
+        fun = GET_FUNCTION_PRIVATE(cx, pn->pn_funpob->object);
+        if (fun->u.i.script) {
+            /*
+             * This second pass is needed to emit JSOP_NOP with a source note
+             * for the already-emitted function. See comments in the TOK_LC
+             * case.
+             */
+            JS_ASSERT(pn->pn_op == JSOP_NOP);
+            JS_ASSERT(cg->treeContext.flags & TCF_IN_FUNCTION);
+            JS_ASSERT(pn->pn_index != (uint32) -1);
+            if (!EmitFunctionDefNop(cx, cg, pn->pn_index))
+                return JS_FALSE;
+            break;
+        }
+
         /* Generate code for the function's body. */
         cg2mark = JS_ARENA_MARK(&cx->tempPool);
         JS_ARENA_ALLOCATE_TYPE(cg2, JSCodeGenerator, &cx->tempPool);
         if (!cg2) {
             js_ReportOutOfScriptQuota(cx);
             return JS_FALSE;
         }
         js_InitCodeGenerator(cx, cg2, cg->treeContext.parseContext,
                              cg->codePool, cg->notePool,
                              pn->pn_pos.begin.lineno);
         cg2->treeContext.flags = (uint16) (pn->pn_flags | TCF_IN_FUNCTION);
         cg2->treeContext.maxScopeDepth = pn->pn_sclen;
-        fun = GET_FUNCTION_PRIVATE(cx, pn->pn_funpob->object);
         cg2->treeContext.fun = fun;
         cg2->parent = cg;
         if (!js_EmitFunctionScript(cx, cg2, pn->pn_body)) {
             pn = NULL;
         } else {
             /*
              * We need an activation object if an inner peeks out, or if such
              * inner-peeking caused one of our inners to become heavyweight.
@@ -4018,62 +4040,51 @@ js_EmitTree(JSContext *cx, JSCodeGenerat
             if ((pn->pn_flags & TCF_GENEXP_LAMBDA) &&
                 js_NewSrcNote(cx, cg, SRC_GENEXP) < 0) {
                 return JS_FALSE;
             }
             EMIT_INDEX_OP(PN_OP(pn), index);
             break;
         }
 
-        /* Top-level named functions need a nop for decompilation. */
-        noteIndex = js_NewSrcNote2(cx, cg, SRC_FUNCDEF, (ptrdiff_t)index);
-        if (noteIndex < 0 ||
-            js_Emit1(cx, cg, JSOP_NOP) < 0) {
-            return JS_FALSE;
-        }
-
         /*
-         * Top-levels also need a prolog op to predefine their names in the
-         * variable object, or if local, to fill their stack slots.
+         * For a script we emit the code as we parse. Thus the bytecode for
+         * top-level functions should go in the prolog to predefine their
+         * names in the variable object before the already-generated main code
+         * is executed. This extra work for top-level scripts is not necessary
+         * when we emit the code for a function. It is fully parsed prior to
+         * invocation of the emitter and calls to js_EmitTree for function
+         * definitions can be scheduled before generating the rest of code.
          */
-        CG_SWITCH_TO_PROLOG(cg);
-
         if (!(cg->treeContext.flags & TCF_IN_FUNCTION)) {
+            CG_SWITCH_TO_PROLOG(cg);
+
             /*
-             * Emit JSOP_CLOSURE for eval code to do less checks when
+             * Emit JSOP_CLOSURE for eval code to do fewer checks when
              * instantiating top-level functions in the non-eval case.
              */
             JS_ASSERT(!cg->treeContext.topStmt);
             op = (cx->fp->flags & JSFRAME_EVAL) ? JSOP_CLOSURE : JSOP_DEFFUN;
             EMIT_INDEX_OP(op, index);
+            CG_SWITCH_TO_MAIN(cg);
+
+            /* Emit NOP for the decompiler. */
+            if (!EmitFunctionDefNop(cx, cg, index))
+                return JS_FALSE;
         } else {
 #ifdef DEBUG
             JSLocalKind localKind =
 #endif
                 js_LookupLocal(cx, cg->treeContext.fun, fun->atom, &slot);
             JS_ASSERT(localKind == JSLOCAL_VAR || localKind == JSLOCAL_CONST);
-
-            /*
-             * If this local function is declared in a body block induced
-             * by let declarations, reparent fun->object to the compiler-
-             * created body block object, so that JSOP_DEFLOCALFUN clones
-             * that block into the runtime scope chain.
-             */
-            stmt = cg->treeContext.topStmt;
-            if (stmt && stmt->type == STMT_BLOCK &&
-                stmt->down && stmt->down->type == STMT_BLOCK &&
-                (stmt->down->flags & SIF_SCOPE)) {
-                JS_ASSERT(STOBJ_GET_CLASS(stmt->down->u.blockObj) ==
-                          &js_BlockClass);
-                OBJ_SET_PARENT(cx, fun->object, stmt->down->u.blockObj);
-            }
+            JS_ASSERT(pn->pn_index == (uint32) -1);
+            pn->pn_index = index;
             if (!EmitSlotIndexOp(cx, JSOP_DEFLOCALFUN, slot, index, cg))
                 return JS_FALSE;
         }
-        CG_SWITCH_TO_MAIN(cg);
         break;
       }
 
 #if JS_HAS_EXPORT_IMPORT
       case TOK_EXPORT:
         pn2 = pn->pn_head;
         if (pn2->pn_type == TOK_STAR) {
             /*
@@ -5153,16 +5164,37 @@ js_EmitTree(JSContext *cx, JSCodeGenerat
         tmp = CG_OFFSET(cg);
         if (pn->pn_extra & PNX_NEEDBRACES) {
             noteIndex = js_NewSrcNote2(cx, cg, SRC_BRACE, 0);
             if (noteIndex < 0 || js_Emit1(cx, cg, JSOP_NOP) < 0)
                 return JS_FALSE;
         }
 
         js_PushStatement(&cg->treeContext, &stmtInfo, STMT_BLOCK, top);
+        if (pn->pn_extra & PNX_FUNCDEFS) {
+            /*
+             * This block contains top-level function definitions. To ensure
+             * that we emit the bytecode defining them prior the rest of code
+             * in the block we use a separate pass over functions. During the
+             * main pass later the emitter will add JSOP_NOP with source notes
+             * for the function to preserve the original functions position
+             * when decompiling.
+             *
+             * Currently this is used only for functions, as compile-as-we go
+             * mode for scripts does not allow separate emitter passes.
+             */
+            JS_ASSERT(cg->treeContext.flags & TCF_IN_FUNCTION);
+            for (pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next) {
+                if (pn2->pn_type == TOK_FUNCTION) {
+                    JS_ASSERT(pn2->pn_op == JSOP_NOP);
+                    if (!js_EmitTree(cx, cg, pn2))
+                        return JS_FALSE;
+                }
+            }
+        }
         for (pn2 = pn->pn_head; pn2; pn2 = pn2->pn_next) {
             if (!js_EmitTree(cx, cg, pn2))
                 return JS_FALSE;
         }
 
         if (noteIndex >= 0 &&
             !js_SetSrcNoteOffset(cx, cg, (uintN)noteIndex, 0,
                                  CG_OFFSET(cg) - tmp)) {
--- a/js/src/jsinterp.c
+++ b/js/src/jsinterp.c
@@ -4712,61 +4712,27 @@ interrupt:
              * Define a local function (i.e., one nested at the top level of
              * another function), parented by the current scope chain, and
              * stored in a local variable slot that the compiler allocated.
              * This is an optimization over JSOP_DEFFUN that avoids requiring
              * a call object for the outer function's activation.
              */
             slot = GET_VARNO(pc);
 
-            JS_ASSERT(!fp->blockChain);
-            if (!(fp->flags & JSFRAME_POP_BLOCKS)) {
-                /*
-                 * If the compiler-created function object (obj) is scoped by a
-                 * let-induced body block, temporarily update fp->blockChain so
-                 * that js_GetScopeChain will clone the block into the runtime
-                 * scope needed to parent the function object's clone.
-                 */
-                parent = OBJ_GET_PARENT(cx, obj);
-                if (parent && OBJ_GET_CLASS(cx, parent) == &js_BlockClass)
-                    fp->blockChain = parent;
-                parent = js_GetScopeChain(cx, fp);
-            } else {
-                /*
-                 * We have already emulated JSOP_ENTERBLOCK for the enclosing
-                 * body block, for a prior JSOP_DEFLOCALFUN in the prolog,  so
-                 * we just load fp->scopeChain into parent.
-                 *
-                 * In typical execution scenarios, the prolog bytecodes that
-                 * include this JSOP_DEFLOCALFUN run, then come main bytecodes
-                 * including JSOP_ENTERBLOCK for the outermost (body) block.
-                 * JSOP_ENTERBLOCK will detect that it need not do anything if
-                 * the body block was entered above due to a local function.
-                 * Finally the matching JSOP_LEAVEBLOCK runs.
-                 *
-                 * If the matching JSOP_LEAVEBLOCK for the body block does not
-                 * run for some reason, the body block will be properly "put"
-                 * (via js_PutBlockObject) by the PutBlockObjects call at the
-                 * bottom of js_Interpret.
-                 */
-                parent = fp->scopeChain;
-                JS_ASSERT(OBJ_GET_CLASS(cx, parent) == &js_BlockClass);
-                JS_ASSERT(OBJ_GET_PROTO(cx, parent) == OBJ_GET_PARENT(cx, obj));
-                JS_ASSERT(OBJ_GET_CLASS(cx, OBJ_GET_PARENT(cx, parent))
-                          == &js_CallClass);
+            parent = js_GetScopeChain(cx, fp);
+            if (!parent) {
+                ok = JS_FALSE;
+                goto out;
             }
 
-            /* If re-parenting, store a clone of the function object. */
-            if (OBJ_GET_PARENT(cx, obj) != parent) {
-                SAVE_SP_AND_PC(fp);
-                obj = js_CloneFunctionObject(cx, obj, parent);
-                if (!obj) {
-                    ok = JS_FALSE;
-                    goto out;
-                }
+            SAVE_SP_AND_PC(fp);
+            obj = js_CloneFunctionObject(cx, obj, parent);
+            if (!obj) {
+                ok = JS_FALSE;
+                goto out;
             }
 
             fp->vars[slot] = OBJECT_TO_JSVAL(obj);
           END_CASE(JSOP_DEFLOCALFUN)
 
           BEGIN_CASE(JSOP_ANONFUNOBJ)
             /* Load the specified function object literal. */
             LOAD_FUNCTION(0);
@@ -5542,33 +5508,22 @@ interrupt:
              * runtime any longer, because an outer block that parents obj has
              * been cloned onto the scope chain.  To avoid re-cloning such a
              * parent and accumulating redundant clones via js_GetScopeChain,
              * we must clone each block eagerly on entry, and push it on the
              * scope chain, until this frame pops.
              */
             if (fp->flags & JSFRAME_POP_BLOCKS) {
                 JS_ASSERT(!fp->blockChain);
-
-                /*
-                 * Check whether JSOP_DEFLOCALFUN emulated JSOP_ENTERBLOCK for
-                 * the body block in order to correctly scope the local cloned
-                 * function object it creates.
-                 */
-                parent = fp->scopeChain;
-                if (OBJ_GET_PROTO(cx, parent) == obj) {
-                    JS_ASSERT(OBJ_GET_CLASS(cx, parent) == &js_BlockClass);
-                } else {
-                    obj = js_CloneBlockObject(cx, obj, parent, fp);
-                    if (!obj) {
-                        ok = JS_FALSE;
-                        goto out;
-                    }
-                    fp->scopeChain = obj;
+                obj = js_CloneBlockObject(cx, obj, fp->scopeChain, fp);
+                if (!obj) {
+                    ok = JS_FALSE;
+                    goto out;
                 }
+                fp->scopeChain = obj;
             } else {
                 JS_ASSERT(!fp->blockChain ||
                           OBJ_GET_PARENT(cx, obj) == fp->blockChain);
                 fp->blockChain = obj;
             }
           END_CASE(JSOP_ENTERBLOCK)
 
           BEGIN_CASE(JSOP_LEAVEBLOCKEXPR)
--- a/js/src/jsparse.c
+++ b/js/src/jsparse.c
@@ -80,16 +80,24 @@
 #include "jsxml.h"
 #endif
 
 #if JS_HAS_DESTRUCTURING
 #include "jsdhash.h"
 #endif
 
 /*
+ * Asserts to verify assumptions behind pn_ macros.
+ */
+JS_STATIC_ASSERT(offsetof(JSParseNode, pn_u.name.atom) ==
+                 offsetof(JSParseNode, pn_u.apair.atom));
+JS_STATIC_ASSERT(offsetof(JSParseNode, pn_u.name.slot) ==
+                 offsetof(JSParseNode, pn_u.lexical.slot));
+
+/*
  * JS parsers, from lowest to highest precedence.
  *
  * Each parser takes a context, a token stream, and a tree context struct.
  * Each returns a parse node tree or null on error.
  */
 
 typedef JSParseNode *
 JSParser(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc);
@@ -1101,16 +1109,19 @@ FunctionDef(JSContext *cx, JSTokenStream
 
     /* Make a TOK_FUNCTION node. */
 #if JS_HAS_GETTER_SETTER
     op = CURRENT_TOKEN(ts).t_op;
 #endif
     pn = NewParseNode(cx, ts, PN_FUNC, tc);
     if (!pn)
         return NULL;
+#ifdef DEBUG
+    pn->pn_index = (uint32) -1;
+#endif
 
     /* Scan the optional function name into funAtom. */
     ts->flags |= TSF_KEYWORD_IS_NAME;
     tt = js_GetToken(cx, ts);
     ts->flags &= ~TSF_KEYWORD_IS_NAME;
     if (tt == TOK_NAME) {
         funAtom = CURRENT_TOKEN(ts).t_atom;
     } else {
@@ -1437,56 +1448,67 @@ FunctionExpr(JSContext *cx, JSTokenStrea
 static JSParseNode *
 Statements(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc)
 {
     JSParseNode *pn, *pn2, *saveBlock;
     JSTokenType tt;
 
     CHECK_RECURSION();
 
+    JS_ASSERT(CURRENT_TOKEN(ts).type == TOK_LC);
     pn = NewParseNode(cx, ts, PN_LIST, tc);
     if (!pn)
         return NULL;
     saveBlock = tc->blockNode;
     tc->blockNode = pn;
     PN_INIT_LIST(pn);
 
     for (;;) {
         ts->flags |= TSF_OPERAND;
         tt = js_PeekToken(cx, ts);
         ts->flags &= ~TSF_OPERAND;
-        if (tt <= TOK_EOF || tt == TOK_RC)
+        if (tt <= TOK_EOF || tt == TOK_RC) {
+            if (tt == TOK_ERROR)
+                return NULL;
             break;
+        }
         pn2 = Statement(cx, ts, tc);
         if (!pn2) {
             if (ts->flags & TSF_EOF)
                 ts->flags |= TSF_UNEXPECTED_EOF;
             return NULL;
         }
 
-        /* Detect a function statement for the TOK_LC case in Statement. */
-        if (pn2->pn_type == TOK_FUNCTION && !AT_TOP_LEVEL(tc))
-            tc->flags |= TCF_HAS_FUNCTION_STMT;
-
+        if (pn2->pn_type == TOK_FUNCTION) {
+            /*
+             * PNX_FUNCDEFS notifies the emitter that the block contains top-
+             * level function definitions that should be processed before the
+             * rest of nodes.
+             *
+             * TCF_HAS_FUNCTION_STMT is for the TOK_LC case in Statement. It
+             * is relevant only for function definitions not at top-level,
+             * which we call function statements.
+             */
+            if (AT_TOP_LEVEL(tc))
+                pn->pn_extra |= PNX_FUNCDEFS;
+            else
+                tc->flags |= TCF_HAS_FUNCTION_STMT;
+        }
         PN_APPEND(pn, pn2);
     }
 
     /*
      * Handle the case where there was a let declaration under this block.  If
      * it replaced tc->blockNode with a new block node then we must refresh pn
      * and then restore tc->blockNode.
      */
     if (tc->blockNode != pn)
         pn = tc->blockNode;
     tc->blockNode = saveBlock;
 
-    ts->flags &= ~TSF_OPERAND;
-    if (tt == TOK_ERROR)
-        return NULL;
-
     pn->pn_pos.end = CURRENT_TOKEN(ts).pos.end;
     return pn;
 }
 
 static JSParseNode *
 Condition(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc)
 {
     JSParseNode *pn;
--- a/js/src/jsparse.h
+++ b/js/src/jsparse.h
@@ -277,16 +277,17 @@ struct JSParseNode {
     JSTokenPos          pn_pos;
     ptrdiff_t           pn_offset;      /* first generated bytecode offset */
     union {
         struct {                        /* TOK_FUNCTION node */
             JSParsedObjectBox *funpob;  /* function object */
             JSParseNode *body;          /* TOK_LC list of statements */
             uint16      flags;          /* accumulated tree context flags */
             uint16      sclen;          /* maximum scope chain length */
+            uint32      index;          /* emitter's index */
         } func;
         struct {                        /* list of next-linked nodes */
             JSParseNode *head;          /* first node in list */
             JSParseNode **tail;         /* ptr to ptr to last node in list */
             uint32      count;          /* number of nodes in list */
             uint32      extra;          /* extra flags, see below */
         } list;
         struct {                        /* ternary: if, for(;;), ?: */
@@ -326,16 +327,17 @@ struct JSParseNode {
     } pn_u;
     JSParseNode         *pn_next;       /* to align dval and pn_u on RISCs */
 };
 
 #define pn_funpob       pn_u.func.funpob
 #define pn_body         pn_u.func.body
 #define pn_flags        pn_u.func.flags
 #define pn_sclen        pn_u.func.sclen
+#define pn_index        pn_u.func.index
 #define pn_head         pn_u.list.head
 #define pn_tail         pn_u.list.tail
 #define pn_count        pn_u.list.count
 #define pn_extra        pn_u.list.extra
 #define pn_kid1         pn_u.ternary.kid1
 #define pn_kid2         pn_u.ternary.kid2
 #define pn_kid3         pn_u.ternary.kid3
 #define pn_left         pn_u.binary.left
@@ -357,17 +359,18 @@ struct JSParseNode {
 #define PNX_CANTFOLD    0x02            /* TOK_PLUS list has unfoldable term */
 #define PNX_POPVAR      0x04            /* TOK_VAR last result needs popping */
 #define PNX_FORINVAR    0x08            /* TOK_VAR is left kid of TOK_IN node,
                                            which is left kid of TOK_FOR */
 #define PNX_ENDCOMMA    0x10            /* array literal has comma at end */
 #define PNX_XMLROOT     0x20            /* top-most node in XML literal tree */
 #define PNX_GROUPINIT   0x40            /* var [a, b] = [c, d]; unit list */
 #define PNX_NEEDBRACES  0x80            /* braces necessary due to closure */
-
+#define PNX_FUNCDEFS   0x100            /* contains top-level function
+                                           statements */
 /*
  * Move pn2 into pn, preserving pn->pn_pos and pn->pn_offset and handing off
  * any kids in pn2->pn_u, by clearing pn2.
  */
 #define PN_MOVE_NODE(pn, pn2)                                                 \
     JS_BEGIN_MACRO                                                            \
         (pn)->pn_type = (pn2)->pn_type;                                       \
         (pn)->pn_op = (pn2)->pn_op;                                           \
--- a/js/src/jsxdrapi.h
+++ b/js/src/jsxdrapi.h
@@ -197,17 +197,17 @@ JS_XDRFindClassById(JSXDRState *xdr, uin
  * Bytecode version number.  Decrement the second term whenever JS bytecode
  * changes incompatibly.
  *
  * This version number should be XDR'ed once near the front of any file or
  * larger storage unit containing XDR'ed bytecode and other data, and checked
  * before deserialization of bytecode.  If the saved version does not match
  * the current version, abort deserialization and invalidate the file.
  */
-#define JSXDR_BYTECODE_VERSION      (0xb973c0de - 17)
+#define JSXDR_BYTECODE_VERSION      (0xb973c0de - 18)
 
 /*
  * Library-private functions.
  */
 extern JSBool
 js_XDRAtom(JSXDRState *xdr, JSAtom **atomp);
 
 extern JSBool