Bug 542002 - Optimize to flat closures even if some upvars can't be copied (r=jorendorff).

js/src/imacros.c.out

@@ -917,6 +917,7 @@ uint8 js_opcode2extra[JSOP_LIMIT] = {
     0,  /* JSOP_SETMETHOD */
     0,  /* JSOP_INITMETHOD */
     0,  /* JSOP_UNBRAND */
+    0,  /* JSOP_UNBRANDTHIS */
     0,  /* JSOP_SHARPINIT */
 };
 #define JSOP_IS_IMACOP(x) (0 \

js/src/jsemit.cpp

@@ -2025,6 +2025,8 @@ BindNameToSlot(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
         dn = pn->pn_lexdef;
         JS_ASSERT(dn->pn_defn);
         pn->pn_dflags |= (dn->pn_dflags & PND_CONST);
+        if (pn->isDeoptimized())
+            return JS_TRUE;
     } else {
         if (!pn->pn_defn)
             return JS_TRUE;
@@ -2616,6 +2618,7 @@ EmitNameOp(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn,
             op = JSOP_CALLNAME;
             break;
           case JSOP_GETGVAR:
+            JS_ASSERT(!cg->funbox);
             op = JSOP_CALLGVAR;
             break;
           case JSOP_GETARG:
@@ -3582,13 +3585,7 @@ js_EmitFunctionScript(JSContext *cx, JSCodeGenerator *cg, JSParseNode *body)
     }
 
     if (cg->flags & TCF_FUN_UNBRAND_THIS) {
-        if (js_Emit1(cx, cg, JSOP_THIS) < 0)
+        if (js_Emit1(cx, cg, JSOP_UNBRANDTHIS) < 0)
-            return false;
-        if (js_Emit1(cx, cg, JSOP_UNBRAND) < 0)
-            return false;
-        if (js_NewSrcNote(cx, cg, SRC_HIDDEN) < 0)
-            return false;
-        if (js_Emit1(cx, cg, JSOP_POP) < 0)
             return false;
     }
 
@@ -5741,7 +5738,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
          */
         pn2 = pn->pn_left;
         atomIndex = (jsatomid) -1;              /* quell GCC overwarning */
-        switch (pn2->pn_type) {
+        switch (PN_TYPE(pn2)) {
           case TOK_NAME:
             if (!BindNameToSlot(cx, cg, pn2))
                 return JS_FALSE;
@@ -5830,10 +5827,9 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
                         return JS_FALSE;
                     EMIT_INDEX_OP(JSOP_GETXPROP, atomIndex);
                 } else {
+                    JS_ASSERT(PN_OP(pn2) != JSOP_GETUPVAR);
                     EMIT_UINT16_IMM_OP((PN_OP(pn2) == JSOP_SETGVAR)
                                        ? JSOP_GETGVAR
-                                       : (PN_OP(pn2) == JSOP_GETUPVAR)
-                                       ? JSOP_GETUPVAR
                                        : (PN_OP(pn2) == JSOP_SETARG)
                                        ? JSOP_GETARG
                                        : JSOP_GETLOCAL,
@@ -6333,7 +6329,6 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
       case TOK_LP:
       {
         bool callop = (PN_TYPE(pn) == TOK_LP);
-        uintN oldflags;
 
         /*
          * Emit callable invocation or operator new (constructor call) code.
@@ -6398,7 +6393,7 @@ js_EmitTree(JSContext *cx, JSCodeGenerator *cg, JSParseNode *pn)
          * JSOP_NEW bytecode with a two-byte immediate telling how many args
          * were pushed on the operand stack.
          */
-        oldflags = cg->flags;
+        uintN oldflags = cg->flags;
         cg->flags &= ~TCF_IN_FOR_INIT;
         for (pn3 = pn2->pn_next; pn3; pn3 = pn3->pn_next) {
             if (!js_EmitTree(cx, cg, pn3))

js/src/jsemit.h

@@ -305,6 +305,12 @@ struct JSTreeContext {              /* tree context for semantic checks */
  */
 #define TCF_FUN_UNBRAND_THIS   0x100000
 
+/*
+ * "Module pattern", i.e., a lambda that is immediately applied and the whole
+ * of an expression statement.
+ */
+#define TCF_FUN_MODULE_PATTERN 0x200000
+
 /*
  * Flags to check for return; vs. return expr; in a function.
  */

js/src/jsfun.h

@@ -128,7 +128,8 @@ typedef union JSLocalNames {
                               JS_ASSERT((fun)->flags & JSFUN_TRCINFO),        \
                               fun->u.n.trcinfo)
 
-struct JSFunction : public JSObject {
+struct JSFunction : public JSObject
+{
     uint16          nargs;        /* maximum number of specified arguments,
                                      reflected as f.length/f.arity */
     uint16          flags;        /* flags, see JSFUN_* below and in jsapi.h */
@@ -192,6 +193,10 @@ struct JSFunction : public JSObject {
     JSAtom *findDuplicateFormal() const;
 
     uint32 countInterpretedReservedSlots() const;
+
+    bool mightEscape() const {
+        return FUN_INTERPRETED(this) && (FUN_FLAT_CLOSURE(this) || u.i.nupvars == 0);
+    }
 };
 
 /*

js/src/jsopcode.cpp

@@ -2528,13 +2528,8 @@ Decompile(SprintStack *ss, jsbytecode *pc, intN nb, JSOp nextop)
                     break;
 
                   case SRC_HIDDEN:
-                    /*
+                    /* Hide this pop, it's from a goto in a with or for/in. */
-                     * Hide this pop. Don't adjust our stack depth model if
-                     * it's from a goto in a with or for/in.
-                     */
                     todo = -2;
-                    if (lastop == JSOP_UNBRAND)
-                        (void) POP_STR();
                     break;
 
                   case SRC_DECL:
@@ -5586,14 +5581,9 @@ ReconstructPCStack(JSContext *cx, JSScript *script, jsbytecode *target,
             }
         }
 
-        /*
+        /* Ignore early-exit code, which is annotated SRC_HIDDEN. */
-         * Ignore early-exit code, which is SRC_HIDDEN, but do not ignore the
+        if (sn && SN_TYPE(sn) == SRC_HIDDEN)
-         * hidden POP that sometimes appears after an UNBRAND. See bug 543565.
-         */
-        if (sn && SN_TYPE(sn) == SRC_HIDDEN &&
-            (op != JSOP_POP || js_GetOpcode(cx, script, pc - 1) != JSOP_UNBRAND)) {
             continue;
-        }
 
         if (SimulateOp(cx, script, op, cs, pc, pcstack, pcdepth) < 0)
             return -1;

js/src/jsopcode.tbl

@@ -602,5 +602,6 @@ OPDEF(JSOP_CONCATN,       234,"concatn",       NULL,  3, -1,  1, 13,  JOF_UINT16
 OPDEF(JSOP_SETMETHOD,     235,"setmethod",     NULL,  3,  2,  1,  3,  JOF_ATOM|JOF_PROP|JOF_SET|JOF_DETECTING)
 OPDEF(JSOP_INITMETHOD,    236,"initmethod",    NULL,  3,  2,  1,  3,  JOF_ATOM|JOF_PROP|JOF_SET|JOF_DETECTING)
 OPDEF(JSOP_UNBRAND,       237,"unbrand",       NULL,  1,  1,  1,  0,  JOF_BYTE)
+OPDEF(JSOP_UNBRANDTHIS,   238,"unbrandthis",   NULL,  1,  0,  0,  0,  JOF_BYTE)
 
-OPDEF(JSOP_SHARPINIT,     238,"sharpinit",     NULL,  3,  0,  0,  0,  JOF_UINT16|JOF_SHARPSLOT)
+OPDEF(JSOP_SHARPINIT,     239,"sharpinit",     NULL,  3,  0,  0,  0,  JOF_UINT16|JOF_SHARPSLOT)

js/src/jsops.cpp

@@ -1468,6 +1468,12 @@ BEGIN_CASE(JSOP_THIS)
     PUSH_OPND(OBJECT_TO_JSVAL(obj));
 END_CASE(JSOP_THIS)
 
+BEGIN_CASE(JSOP_UNBRANDTHIS)
+    COMPUTE_THIS(cx, fp, obj);
+    if (!obj->unbrand(cx))
+        goto error;
+END_CASE(JSOP_UNBRANDTHIS)
+
 BEGIN_CASE(JSOP_GETTHISPROP)
     i = 0;
     COMPUTE_THIS(cx, fp, obj);

js/src/jsparse.cpp

@@ -326,7 +326,7 @@ JSFunctionBox::shouldUnbrand(uintN methods, uintN slowMethods) const
 {
     if (slowMethods != 0) {
         for (const JSFunctionBox *funbox = this; funbox; funbox = funbox->parent) {
-            if (!(funbox->node->pn_dflags & PND_MODULEPAT))
+            if (!(funbox->tcflags & TCF_FUN_MODULE_PATTERN))
                 return true;
             if (funbox->inLoop)
                 return true;
@@ -2051,6 +2051,188 @@ OneBlockId(JSParseNode *fn, uint32 id)
     return true;
 }
 
+static inline bool
+CanFlattenUpvar(JSDefinition *dn, JSFunctionBox *funbox, uint32 tcflags)
+{
+    /*
+     * Consider the current function (the lambda, innermost below) using a var
+     * x defined two static levels up:
+     *
+     *  function f() {
+     *      // z = g();
+     *      var x = 42;
+     *      function g() {
+     *          return function () { return x; };
+     *      }
+     *      return g();
+     *  }
+     *
+     * So long as (1) the initialization in 'var x = 42' dominates all uses of
+     * g and (2) x is not reassigned, it is safe to optimize the lambda to a
+     * flat closure. Uncommenting the early call to g makes this optimization
+     * unsafe (z could name a global setter that calls its argument).
+     */
+    JSFunctionBox *afunbox = funbox;
+    uintN dnLevel = dn->frameLevel();
+
+    JS_ASSERT(dnLevel <= funbox->level);
+    while (afunbox->level != dnLevel) {
+        afunbox = afunbox->parent;
+
+        /*
+         * NB: afunbox can't be null because we are sure to find a function box
+         * whose level == dnLevel before we would try to walk above the root of
+         * the funbox tree. See bug 493260 comments 16-18.
+         *
+         * Assert but check anyway, to protect future changes that bind eval
+         * upvars in the parser.
+         */
+        JS_ASSERT(afunbox);
+
+        /*
+         * If this function is reaching up across an enclosing funarg, then we
+         * cannot copy dn's value into a flat closure slot (the display stops
+         * working once the funarg escapes).
+         */
+        if (!afunbox || afunbox->node->isFunArg())
+            return false;
+    }
+
+    /*
+     * If afunbox's function (which is at the same level as dn) is in a loop,
+     * pessimistically assume the variable initializer may be in the same loop.
+     * A flat closure would then be unsafe, as the captured variable could be
+     * assigned after the closure is created. See bug 493232.
+     */
+    if (afunbox->inLoop)
+        return false;
+
+    /*
+     * |with| and eval used as an operator defeat lexical scoping: they can be
+     * used to assign to any in-scope variable. Therefore they must disable
+     * flat closures that use such upvars.  The parser detects these as special
+     * forms and marks the function heavyweight.
+     */
+    if ((afunbox->parent ? afunbox->parent->tcflags : tcflags) & TCF_FUN_HEAVYWEIGHT)
+        return false;
+
+    /*
+     * If afunbox's function is not a lambda, it will be hoisted, so it could
+     * capture the undefined value that by default initializes var, let, and
+     * const bindings. And if dn is a function that comes at (meaning a
+     * function refers to its own name) or strictly after afunbox, we also
+     * defeat the flat closure optimization for this dn.
+     */
+    JSFunction *afun = (JSFunction *) afunbox->object;
+    if (!(afun->flags & JSFUN_LAMBDA)) {
+        if (dn->isBindingForm() || dn->pn_pos >= afunbox->node->pn_pos)
+            return false;
+    }
+
+    if (!dn->isInitialized())
+        return false;
+
+    JSDefinition::Kind dnKind = dn->kind();
+    if (dnKind != JSDefinition::CONST) {
+        if (dn->isAssigned())
+            return false;
+
+        /*
+         * Any formal could be mutated behind our back via the arguments
+         * object, so deoptimize if the outer function uses arguments.
+         *
+         * In a Function constructor call where the final argument -- the body
+         * source for the function to create -- contains a nested function
+         * definition or expression, afunbox->parent will be null. The body
+         * source might use |arguments| outside of any nested functions it may
+         * contain, so we have to check the tcflags parameter that was passed
+         * in from JSCompiler::compileFunctionBody.
+         */
+        if (dnKind == JSDefinition::ARG &&
+            ((afunbox->parent ? afunbox->parent->tcflags : tcflags) & TCF_FUN_USES_ARGUMENTS)) {
+            return false;
+        }
+    }
+
+    /*
+     * Check quick-and-dirty dominance relation. Function definitions dominate
+     * their uses thanks to hoisting.  Other binding forms hoist as undefined,
+     * of course, so check forward-reference and blockid relations.
+     */
+    if (dnKind != JSDefinition::FUNCTION) {
+        /*
+         * Watch out for code such as
+         *
+         *   (function () {
+         *   ...
+         *   var jQuery = ... = function (...) {
+         *       return new jQuery.foo.bar(baz);
+         *   }
+         *   ...
+         *   })();
+         *
+         * where the jQuery variable is not reassigned, but of course is not
+         * initialized at the time that the would-be-flat closure containing
+         * the jQuery upvar is formed.
+         */
+        if (dn->pn_pos.end >= afunbox->node->pn_pos.end ||
+            (dn->isTopLevel()
+             ? !MinBlockId(afunbox->node, dn->pn_blockid)
+             : !dn->isBlockChild() ||
+               !afunbox->node->isBlockChild() ||
+               !OneBlockId(afunbox->node, dn->pn_blockid))) {
+            return false;
+        }
+    }
+    return true;
+}
+
+static void
+FlagHeavyweights(JSDefinition *dn, JSFunctionBox *funbox, uint32& tcflags)
+{
+    JSFunctionBox *afunbox = funbox->parent;
+    uintN dnLevel = dn->frameLevel();
+
+    while (afunbox) {
+        /*
+         * Notice that afunbox->level is the static level of the definition or
+         * expression of the function parsed into afunbox, not the static level
+         * of its body. Therefore we must add 1 to match dn's level to find the
+         * afunbox whose body contains the dn definition.
+         */
+        if (afunbox->level + 1U == dnLevel || (dnLevel == 0 && dn->isLet())) {
+            afunbox->tcflags |= TCF_FUN_HEAVYWEIGHT;
+            break;
+        }
+        afunbox = afunbox->parent;
+    }
+    if (!afunbox && (tcflags & TCF_IN_FUNCTION))
+        tcflags |= TCF_FUN_HEAVYWEIGHT;
+}
+
+static void
+DeoptimizeUsesWithin(JSDefinition *dn, JSFunctionBox *funbox, uint32& tcflags)
+{
+    JSParseNode **pnup = &dn->dn_uses;
+    uintN ndeoptimized = 0;
+
+    while (JSParseNode *pnu = *pnup) {
+        JS_ASSERT(pnu->pn_used);
+        JS_ASSERT(!pnu->pn_defn);
+        const JSTokenPos &pos = funbox->node->pn_body->pn_pos;
+        if (pnu->pn_pos.begin >= pos.begin && pnu->pn_pos.end < pos.end) {
+            pnu->pn_dflags |= PND_DEOPTIMIZED;
+            *pnup = pnu->pn_link;
+            ++ndeoptimized;
+            continue;
+        }
+        pnup = &pnu->pn_link;
+    }
+
+    if (ndeoptimized != 0)
+        FlagHeavyweights(dn, funbox, tcflags);
+}
+
 void
 JSCompiler::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags)
 {
@@ -2181,8 +2363,9 @@ JSCompiler::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags)
                 } else if (!mutation && !(funbox->tcflags & TCF_FUN_IS_GENERATOR)) {
                     /*
                      * Algol-like functions can read upvars using the dynamic
-                     * link (cx->fp/fp->down). They do not need to entrain and
+                     * link (cx->fp/fp->down), optimized using the cx->display
-                     * search their environment.
+                     * lookup table indexed by static level. They do not need
+                     * to entrain and search their environment objects.
                      */
                     FUN_METER(display);
                     FUN_SET_KIND(fun, JSFUN_NULL_CLOSURE);
@@ -2191,7 +2374,7 @@ JSCompiler::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags)
                         FUN_METER(setupvar);
                 }
             } else {
-                uintN nupvars = 0;
+                uintN nupvars = 0, nflattened = 0;
 
                 /*
                  * For each lexical dependency from this closure to an outer
@@ -2203,165 +2386,18 @@ JSCompiler::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags)
 
                     if (!lexdep->isFreeVar()) {
                         ++nupvars;
-
+                        if (CanFlattenUpvar(lexdep, funbox, tcflags)) {
-                        /*
+                            ++nflattened;
-                         * Consider the current function (the lambda, innermost
+                            continue;
-                         * below) using a var x defined two static levels up:
-                         *
-                         *  function f() {
-                         *      // z = g();
-                         *      var x = 42;
-                         *      function g() {
-                         *          return function () { return x; };
-                         *      }
-                         *      return g();
-                         *  }
-                         *
-                         * So long as (1) the initialization in 'var x = 42'
-                         * dominates all uses of g and (2) x is not reassigned,
-                         * it is safe to optimize the lambda to a flat closure.
-                         * Uncommenting the early call to g makes it unsafe to
-                         * so optimize (z could name a global setter that calls
-                         * its argument).
-                         */
-                        JSFunctionBox *afunbox = funbox;
-                        uintN lexdepLevel = lexdep->frameLevel();
-
-                        JS_ASSERT(lexdepLevel <= funbox->level);
-                        while (afunbox->level != lexdepLevel) {
-                            afunbox = afunbox->parent;
-
-                            /*
-                             * afunbox can't be null because we are sure
-                             * to find a function box whose level == lexdepLevel
-                             * before walking off the top of the funbox tree.
-                             * See bug 493260 comments 16-18.
-                             *
-                             * Assert but check anyway, to check future changes
-                             * that bind eval upvars in the parser.
-                             */
-                            JS_ASSERT(afunbox);
-
-                            /*
-                             * If this function is reaching up across an
-                             * enclosing funarg, we cannot make a flat
-                             * closure. The display stops working once the
-                             * funarg escapes.
-                             */
-                            if (!afunbox || afunbox->node->isFunArg())
-                                goto break2;
                         }
-
+                        DeoptimizeUsesWithin(lexdep, funbox, tcflags);
-                        /*
-                         * If afunbox's function (which is at the same level as
-                         * lexdep) is in a loop, pessimistically assume the
-                         * variable initializer may be in the same loop. A flat
-                         * closure would then be unsafe, as the captured
-                         * variable could be assigned after the closure is
-                         * created. See bug 493232.
-                         */
-                        if (afunbox->inLoop)
-                            break;
-
-                        /*
-                         * with and eval defeat lexical scoping; eval anywhere
-                         * in a variable's scope can assign to it. Both defeat
-                         * the flat closure optimization. The parser detects
-                         * these cases and flags the function heavyweight.
-                         */
-                        if ((afunbox->parent ? afunbox->parent->tcflags : tcflags)
-                            & TCF_FUN_HEAVYWEIGHT) {
-                            break;
-                        }
-
-                        /*
-                         * If afunbox's function is not a lambda, it will be
-                         * hoisted, so it could capture the undefined value
-                         * that by default initializes var/let/const
-                         * bindings. And if lexdep is a function that comes at
-                         * (meaning a function refers to its own name) or
-                         * strictly after afunbox, we also break to defeat the
-                         * flat closure optimization.
-                         */
-                        JSFunction *afun = (JSFunction *) afunbox->object;
-                        if (!(afun->flags & JSFUN_LAMBDA)) {
-                            if (lexdep->isBindingForm())
-                                break;
-                            if (lexdep->pn_pos >= afunbox->node->pn_pos)
-                                break;
-                        }
-
-                        if (!lexdep->isInitialized())
-                            break;
-
-                        JSDefinition::Kind lexdepKind = lexdep->kind();
-                        if (lexdepKind != JSDefinition::CONST) {
-                            if (lexdep->isAssigned())
-                                break;
-
-                            /*
-                             * Any formal could be mutated behind our back via
-                             * the arguments object, so deoptimize if the outer
-                             * function uses arguments.
-                             *
-                             * In a Function constructor call where the final
-                             * argument -- the body source for the function to
-                             * create -- contains a nested function definition
-                             * or expression, afunbox->parent will be null. The
-                             * body source might use |arguments| outside of any
-                             * nested functions it may contain, so we have to
-                             * check the tcflags parameter that was passed in
-                             * from JSCompiler::compileFunctionBody.
-                             */
-                            if (lexdepKind == JSDefinition::ARG &&
-                                ((afunbox->parent ? afunbox->parent->tcflags : tcflags) &
-                                 TCF_FUN_USES_ARGUMENTS)) {
-                                break;
                     }
                 }
 
-                        /*
-                         * Check quick-and-dirty dominance relation. Function
-                         * definitions dominate their uses thanks to hoisting.
-                         * Other binding forms hoist as undefined, of course,
-                         * so check forward-reference and blockid relations.
-                         */
-                        if (lexdepKind != JSDefinition::FUNCTION) {
-                            /*
-                             * Watch out for code such as
-                             *
-                             *   (function () {
-                             *   ...
-                             *   var jQuery = ... = function (...) {
-                             *       return new jQuery.foo.bar(baz);
-                             *   }
-                             *   ...
-                             *   })();
-                             *
-                             * where the jQuery var is not reassigned, but of
-                             * course is not initialized at the time that the
-                             * would-be-flat closure containing the jQuery
-                             * upvar is formed.
-                             */
-                            if (lexdep->pn_pos.end >= afunbox->node->pn_pos.end)
-                                break;
-
-                            if (lexdep->isTopLevel()
-                                ? !MinBlockId(afunbox->node, lexdep->pn_blockid)
-                                : !lexdep->isBlockChild() ||
-                                  !afunbox->node->isBlockChild() ||
-                                  !OneBlockId(afunbox->node, lexdep->pn_blockid)) {
-                                break;
-                            }
-                        }
-                    }
-                }
-
-              break2:
                 if (nupvars == 0) {
                     FUN_METER(onlyfreevar);
                     FUN_SET_KIND(fun, JSFUN_NULL_CLOSURE);
-                } else if (!ale) {
+                } else if (nflattened != 0) {
                     /*
                      * We made it all the way through the upvar loop, so it's
                      * safe to optimize to a flat closure.
@@ -2406,30 +2442,8 @@ JSCompiler::setFunctionKinds(JSFunctionBox *funbox, uint32& tcflags)
 
             while ((ale = iter()) != NULL) {
                 JSDefinition *lexdep = ALE_DEFN(ale)->resolve();
-
+                if (!lexdep->isFreeVar())
-                if (!lexdep->isFreeVar()) {
+                    FlagHeavyweights(lexdep, funbox, tcflags);
-                    JSFunctionBox *afunbox = funbox->parent;
-                    uintN lexdepLevel = lexdep->frameLevel();
-
-                    while (afunbox) {
-                        /*
-                         * NB: afunbox->level is the static level of
-                         * the definition or expression of the function
-                         * parsed into afunbox, not the static level of
-                         * its body. Therefore we must add 1 to match
-                         * lexdep's level to find the afunbox whose
-                         * body contains the lexdep definition.
-                         */
-                        if (afunbox->level + 1U == lexdepLevel ||
-                            (lexdepLevel == 0 && lexdep->isLet())) {
-                            afunbox->tcflags |= TCF_FUN_HEAVYWEIGHT;
-                            break;
-                        }
-                        afunbox = afunbox->parent;
-                    }
-                    if (!afunbox && (tcflags & TCF_IN_FUNCTION))
-                        tcflags |= TCF_FUN_HEAVYWEIGHT;
-                }
             }
         }
 
@@ -5746,7 +5760,7 @@ Statement(JSContext *cx, JSTokenStream *ts, JSTreeContext *tc)
              */
             if (PN_TYPE(pn2->pn_head) == TOK_FUNCTION &&
                 !pn2->pn_head->pn_funbox->node->isFunArg()) {
-                pn2->pn_head->pn_funbox->node->pn_dflags |= PND_MODULEPAT;
+                pn2->pn_head->pn_funbox->tcflags |= TCF_FUN_MODULE_PATTERN;
             }
             break;
           case TOK_ASSIGN:

js/src/jsparse.h

@@ -399,7 +399,7 @@ struct JSParseNode {
     }
 
     JSDefinition *lexdef() const {
-        JS_ASSERT(pn_used);
+        JS_ASSERT(pn_used || isDeoptimized());
         JS_ASSERT(pn_arity == PN_NAME);
         return pn_lexdef;
     }
@@ -419,9 +419,9 @@ struct JSParseNode {
 #define PND_PLACEHOLDER 0x80            /* placeholder definition for lexdep */
 #define PND_FUNARG     0x100            /* downward or upward funarg usage */
 #define PND_BOUND      0x200            /* bound to a stack or global slot */
-#define PND_MODULEPAT  0x400            /* "module pattern", i.e., a lambda
+#define PND_DEOPTIMIZED 0x400           /* former pn_used name node, pn_lexdef
-                                           that is immediately applied and the
+                                           still valid, but this use no longer
-                                           whole of an expression statement */
+                                           optimizable via an upvar opcode */
 
 /* Flags to propagate from uses to definition. */
 #define PND_USE2DEF_FLAGS (PND_ASSIGNED | PND_FUNARG)
@@ -467,6 +467,7 @@ struct JSParseNode {
     bool isTopLevel() const     { return test(PND_TOPLEVEL); }
     bool isBlockChild() const   { return test(PND_BLOCKCHILD); }
     bool isPlaceholder() const  { return test(PND_PLACEHOLDER); }
+    bool isDeoptimized() const  { return test(PND_DEOPTIMIZED); }
 
     /* Defined below, see after struct JSDefinition. */
     bool isAssigned() const;

js/src/jstracer.cpp

@@ -12523,8 +12523,8 @@ TraceRecorder::record_JSOP_GETDSLOT()
     LIns* callee_ins = get(&cx->fp->argv[-2]);
 
     unsigned index = GET_UINT16(cx->fp->regs->pc);
-    LIns* dslots_ins = NULL;
+    LIns* dslots_ins = lir->insLoad(LIR_ldp, callee_ins, offsetof(JSObject, dslots));
-    LIns* v_ins = stobj_get_dslot(callee_ins, index, dslots_ins);
+    LIns* v_ins = lir->insLoad(LIR_ldcp, dslots_ins, index * sizeof(jsval));
 
     stack(0, unbox_jsval(callee->dslots[index], v_ins, snapshot(BRANCH_EXIT)));
     return ARECORD_CONTINUE;
@@ -12541,33 +12541,63 @@ TraceRecorder::record_JSOP_CALLDSLOT()
 JS_REQUIRES_STACK RecordingStatus
 TraceRecorder::guardCallee(jsval& callee)
 {
-    JS_ASSERT(VALUE_IS_FUNCTION(cx, callee));
-
-    VMSideExit* branchExit = snapshot(BRANCH_EXIT);
     JSObject* callee_obj = JSVAL_TO_OBJECT(callee);
-    LIns* callee_ins = get(&callee);
+    JS_ASSERT(callee_obj->isFunction());
+    JSFunction* callee_fun = (JSFunction*) callee_obj->getPrivate();
 
+    /*
+     * First, guard on the callee's function (JSFunction*) identity. This is
+     * necessary since tracing always inlines function calls. But note that
+     * TR::functionCall avoids calling TR::guardCallee for constant methods
+     * (those hit in the property cache from JSOP_CALLPROP).
+     */
+    VMSideExit* branchExit = snapshot(BRANCH_EXIT);
+    LIns* callee_ins = get(&callee);
     tree->gcthings.addUnique(callee);
+
     guard(true,
           lir->ins2(LIR_peq,
                     stobj_get_private(callee_ins),
-                    INS_CONSTPTR(callee_obj->getPrivate())),
+                    INS_CONSTPTR(callee_fun)),
           branchExit);
 
     /*
-     * As long as we have this parent guard, we're guaranteed that if we record
+     * Second, consider guarding on the parent scope of the callee.
-     * with a Call object which has a null getPrivate(), then on trace that
+     *
-     * Call object will continue to have a null private, because we're
+     * As long as we guard on parent scope, we are guaranteed when recording
-     * effectively guarding on Call object identity and Call objects can't pick
+     * variable accesses for a Call object having no private data that we can
-     * up a stack frame once they have none. callProp and setCallProp depend
+     * emit code that avoids checking for an active JSStackFrame for the Call
-     * on this and document where; if this guard is removed make sure to fix
+     * object (which would hold fresh variable values -- the Call object's
-     * those methods. Search for the "parent guard" comments in them.
+     * dslots would be stale until the stack frame is popped). This is because
+     * Call objects can't pick up a new stack frame in their private slot once
+     * they have none. TR::callProp and TR::setCallProp depend on this fact and
+     * document where; if this guard is removed make sure to fix those methods.
+     * Search for the "parent guard" comments in them.
+     *
+     * In general, a loop in an escaping function scoped by Call objects could
+     * be traced before the function has returned, and the trace then triggered
+     * after, or vice versa. The function must escape, i.e., be a "funarg", or
+     * else there's no need to guard callee parent at all. So once we know (by
+     * static analysis) that a function may escape, we cannot avoid guarding on
+     * either the private data of the Call object or the Call object itself, if
+     * we wish to optimize for the particular deactivated stack frame (null
+     * private data) case as noted above.
      */
+    if (FUN_INTERPRETED(callee_fun) &&
+        (!FUN_NULL_CLOSURE(callee_fun) || callee_fun->u.i.nupvars != 0)) {
+        JSObject* parent = callee_obj->getParent();
+
+        if (parent != globalObj) {
+            if (parent->getClass() != &js_CallClass)
+                RETURN_STOP("closure scoped by neither the global object nor a Call object");
+
             guard(true,
                   lir->ins2(LIR_peq,
                             stobj_get_parent(callee_ins),
-                    INS_CONSTOBJ(OBJ_GET_PARENT(cx, callee_obj))),
+                            INS_CONSTOBJ(parent)),
                   branchExit);
+        }
+    }
     return RECORD_CONTINUE;
 }
 
@@ -14172,7 +14202,7 @@ TraceRecorder::record_JSOP_LAMBDA_FC()
         return ARECORD_STOP;
 
     LIns* args[] = {
-        INS_CONSTOBJ(globalObj),
+        scopeChain(),
         INS_CONSTFUN(fun),
         cx_ins
     };
@@ -15389,6 +15419,20 @@ TraceRecorder::record_JSOP_UNBRAND()
     return ARECORD_CONTINUE;
 }
 
+JS_REQUIRES_STACK AbortableRecordingStatus
+TraceRecorder::record_JSOP_UNBRANDTHIS()
+{
+    LIns* this_ins;
+    RecordingStatus status = getThis(this_ins);
+    if (status != RECORD_CONTINUE)
+        return InjectStatus(status);
+
+    LIns* args_ins[] = { this_ins, cx_ins };
+    LIns* call_ins = lir->insCall(&js_Unbrand_ci, args_ins);
+    guard(true, call_ins, OOM_EXIT);
+    return ARECORD_CONTINUE;
+}
+
 JS_REQUIRES_STACK AbortableRecordingStatus
 TraceRecorder::record_JSOP_SHARPINIT()
 {

js/src/jsxdrapi.h

@@ -205,7 +205,7 @@ JS_XDRFindClassById(JSXDRState *xdr, uint32 id);
  * 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 - 59)
+#define JSXDR_BYTECODE_VERSION      (0xb973c0de - 60)
 
 /*
  * Library-private functions.

js/src/trace-test/tests/basic/testGuardCalleeSneakAttack.js

@@ -0,0 +1,38 @@
+function loop(f, expected) {
+   // This is the loop that breaks us.
+   // At record time, f's parent is a Call object with no fp.
+   // At second execute time, it is a Call object with fp,
+   // and all the Call object's dslots are still JSVAL_VOID.
+   for (var i = 0; i < 9; i++)
+       assertEq(f(), expected);
+}
+
+function C(bad) {
+   var x = bad;
+   function f() {
+       return x;  // We trick TR::callProp() into emitting code that gets
+                  // JSVAL_VOID (from the Call object's dslots)
+                  // rather than the actual value (true or false).
+   }
+   this.m = f;
+   return f;
+}
+
+var obj = {
+   set m(f) {
+       if (f())  // Call once to resolve x on the Call object,
+                 // for shape consistency. Otherwise loop gets
+                 // recorded twice.
+           loop(f, true);
+   }
+};
+
+loop(C.call(obj, false), false);
+C.call(obj, true);
+
+checkStats({
+    recorderStarted: 1,
+    recorderAborted: 0,
+    traceCompleted: 2,
+    traceTriggered: 4
+});

js/src/trace-test/tests/basic/testGuardCalleeSneakAttack2.js

@@ -0,0 +1,39 @@
+function loop(f, expected) {
+   // This is the loop that breaks us.
+   // At record time, f's parent is a Call object with no fp.
+   // At second execute time, it is a Call object with fp,
+   // and all the Call object's dslots are still JSVAL_VOID.
+   for (var i = 0; i < 9; i++)
+       assertEq(f(), expected);
+}
+
+function C(bad) {
+   var x = bad;
+   function f() {
+       return x;  // We trick TR::callProp() into emitting code that gets
+                  // JSVAL_VOID (from the Call object's dslots)
+                  // rather than the actual value (true or false).
+   }
+   if (bad)
+       void (f + "a!");
+   return f;
+}
+
+var obj = {
+};
+
+// Warm up and trace with C's Call object entrained but its stack frame gone.
+loop(C.call(obj, false), false);
+
+// Sneaky access to f via a prototype method called implicitly by operator +.
+Function.prototype.toString = function () { loop(this, true); return "hah"; };
+
+// Fail hard if we don't handle the implicit call out of C to F.p.toString.
+C.call(obj, true);
+
+checkStats({
+    recorderStarted: 1,
+    recorderAborted: 0,
+    traceCompleted: 2,
+    traceTriggered: 4
+});

js/src/trace-test/tests/basic/testPartialFlatClosure.js

@@ -0,0 +1,16 @@
+assertEq((('-r', function (s) {
+    function C(i) {
+        this.m = function () { return i * t; }
+    }
+    var t = s;
+    var a = [];
+    for (var i = 0; i < 5; i++)
+        a[a.length] = new C(i);
+    return a;
+})(42))[4].m(), 168);
+
+checkStats({
+    recorderStarted: 1,
+    recorderAborted: 0,
+    traceCompleted: 1,
+});