Bug 790424 (part 1) - Set strict mode on functions in default arguments more lazily. code=benjamin,nnethercote. r=nnethercote.
authorNicholas Nethercote <nnethercote@mozilla.com>
Wed, 12 Sep 2012 18:50:37 -0700
changeset 107814 43ab91c9c8e16cf45a1724d2f32f4ba10fca6296
parent 107813 4050caa0b9c5684cbabfb37c55c788da955d071f
child 107815 65cf1362d58c5ba29c793858da5545f8c3ef2e71
push id82
push usershu@rfrn.org
push dateFri, 05 Oct 2012 13:20:22 +0000
reviewersnnethercote
bugs790424
milestone18.0a1
Bug 790424 (part 1) - Set strict mode on functions in default arguments more lazily. code=benjamin,nnethercote. r=nnethercote.
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/frontend/SharedContext.h
js/src/jit-test/tests/arguments/defaults-bug790424.js
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4846,16 +4846,23 @@ EmitFunc(JSContext *cx, BytecodeEmitter 
         JS_ASSERT(pn->functionIsHoisted());
         JS_ASSERT(bce->sc->isFunction);
         return EmitFunctionDefNop(cx, bce, pn->pn_index);
     }
 
     {
         SharedContext *outersc = bce->sc;
         FunctionBox *funbox = pn->pn_funbox;
+
+        // If funbox's strictModeState is still unknown (as can happen for
+        // functions defined in defaults), inherit it from the parent.
+        if (funbox->strictModeState == StrictMode::UNKNOWN) {
+            JS_ASSERT(outersc->strictModeState != StrictMode::UNKNOWN);
+            funbox->strictModeState = outersc->strictModeState;
+        }
         if (outersc->isFunction && outersc->asFunbox()->mightAliasLocals())
             funbox->setMightAliasLocals();      // inherit mightAliasLocals from parent
         JS_ASSERT_IF(outersc->inStrictMode(), funbox->inStrictMode());
 
         // Inherit most things (principals, version, etc) from the parent.
         Rooted<JSScript*> parent(cx, bce->script);
         Rooted<JSObject*> enclosingScope(cx, EnclosingStaticScope(bce));
         CompileOptions options(cx);
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1790,71 +1790,63 @@ Parser::functionExpr()
     JS_ASSERT(tokenStream.currentToken().type == TOK_FUNCTION);
     if (tokenStream.getToken(TSF_KEYWORD_IS_NAME) == TOK_NAME)
         name = tokenStream.currentToken().name();
     else
         tokenStream.ungetToken();
     return functionDef(name, Normal, Expression);
 }
 
-void
-FunctionBox::recursivelySetStrictMode(StrictMode strictness)
-{
-    if (strictModeState == StrictMode::UNKNOWN) {
-        strictModeState = strictness;
-        for (FunctionBox *kid = kids; kid; kid = kid->siblings)
-            kid->recursivelySetStrictMode(strictness);
-    }
-}
-
 /*
  * Indicate that the current scope can't switch to strict mode with a body-level
  * "use strict" directive anymore. Return false on error.
  */
 bool
 Parser::setStrictMode(bool strictMode)
 {
     if (pc->sc->strictModeState != StrictMode::UNKNOWN) {
         // Strict mode was inherited.
         JS_ASSERT(pc->sc->strictModeState == StrictMode::STRICT);
         if (pc->sc->isFunction) {
             JS_ASSERT(pc->parent->sc->strictModeState == StrictMode::STRICT);
         } else {
-            JS_ASSERT(StrictModeFromContext(context) == StrictMode::STRICT || pc->staticLevel);
+            JS_ASSERT_IF(pc->staticLevel == 0,
+                         StrictModeFromContext(context) == StrictMode::STRICT);
         }
         return true;
     }
+
     if (strictMode) {
         if (pc->queuedStrictModeError) {
             // There was a strict mode error in this scope before we knew it was
             // strict. Throw it.
             JS_ASSERT(!(pc->queuedStrictModeError->report.flags & JSREPORT_WARNING));
             pc->queuedStrictModeError->throwError();
             return false;
         }
         pc->sc->strictModeState = StrictMode::STRICT;
-    } else if (!pc->parent || pc->parent->sc->strictModeState == StrictMode::NOTSTRICT) {
-        // This scope will not be strict.
-        pc->sc->strictModeState = StrictMode::NOTSTRICT;
-        if (pc->queuedStrictModeError && context->hasStrictOption() &&
-            pc->queuedStrictModeError->report.errorNumber != JSMSG_STRICT_CODE_WITH) {
-            // Convert queued strict mode error to a warning.
-            pc->queuedStrictModeError->report.flags |= JSREPORT_WARNING;
-            pc->queuedStrictModeError->throwError();
+    } else {
+        if (!pc->parent || pc->parent->sc->strictModeState == StrictMode::NOTSTRICT) {
+            // This scope lacks a strict directive, and its parent (if it has
+            // one) definitely isn't strict, so it definitely won't be strict.
+            pc->sc->strictModeState = StrictMode::NOTSTRICT;
+            if (pc->queuedStrictModeError && context->hasStrictOption() &&
+                pc->queuedStrictModeError->report.errorNumber != JSMSG_STRICT_CODE_WITH) {
+                // Convert queued strict mode error to a warning.
+                pc->queuedStrictModeError->report.flags |= JSREPORT_WARNING;
+                pc->queuedStrictModeError->throwError();
+            }
+        } else {
+            // This scope (which has a parent and so must be a function) lacks
+            // a strict directive, but it's not yet clear if its parent is
+            // strict.  (This can only happen for functions in default
+            // arguments.)  Leave it in the UNKNOWN state for now.
+            JS_ASSERT(pc->sc->isFunction);
         }
     }
-    JS_ASSERT_IF(!pc->sc->isFunction, !pc->functionList);
-    if (pc->sc->strictModeState != StrictMode::UNKNOWN && pc->sc->isFunction) {
-        // We changed the strict mode state. Retroactively recursively set
-        // strict mode status on all the function children we've seen so far
-        // children (That is, functions in default expressions).
-        pc->sc->asFunbox()->strictModeState = pc->sc->strictModeState;
-        for (FunctionBox *kid = pc->functionList; kid; kid = kid->siblings)
-            kid->recursivelySetStrictMode(pc->sc->strictModeState);
-    }
     return true;
 }
 
 /*
  * Return true if this token, known to be an unparenthesized string literal,
  * could be the string of a directive in a Directive Prologue. Directive
  * strings never contain escape sequences or line continuations.
  */
--- a/js/src/frontend/SharedContext.h
+++ b/js/src/frontend/SharedContext.h
@@ -147,25 +147,23 @@ class SharedContext
     // should be STRICT or NOTSTRICT. However, it can be UNKNOWN when parsing
     // code for which the strictness has not yet been determined. This happens
     // when parsing the defaults of a functions and non-"use strict" directive
     // prologue strings.
     //
     // Unless its parent is strict, a context starts out in the UNKNOWN
     // state. Parser::setStrictMode() should be called when a context has been
     // determined to be strict or it cannot possibly become strict through the
-    // directive prologue. (It might become strict later if it is in the default
-    // expressions of a strict function.)
+    // directive prologue.
     //
-    // If the state is STRICT, all context children are STRICT, too. Neither of
-    // the other two states have this behavior. A funbox with the UNKNOWN state
-    // can have STRICT children but not NOTSTRICT children. NOTSTRICT funboxes
-    // can have any kind of children.
+    // When parsing is done, no contexts can be in the UNKNOWN state, with the
+    // exception of functions defined in default expressions.  Any such context
+    // subsequently inherits its parent's state when it starts being used in
+    // BytecodeEmitter (see EmitFunc()).
     //
-    // When parsing is done, no context may be in the UNKNOWN strictness state.
     StrictMode strictModeState;
 
     // If it's function code, funbox must be non-NULL and scopeChain must be NULL.
     // If it's global code, funbox must be NULL.
     inline SharedContext(JSContext *cx, bool isFun, StrictMode sms);
 
     inline GlobalSharedContext *asGlobal();
     inline FunctionBox *asFunbox();
@@ -208,18 +206,16 @@ class FunctionBox : public SharedContext
 
     FunctionContextFlags funCxFlags;
 
     FunctionBox(JSContext *cx, ObjectBox* traceListHead, JSFunction *fun, ParseContext *pc,
                 StrictMode sms);
 
     JSFunction *fun() const { return objbox.object->toFunction(); }
 
-    void recursivelySetStrictMode(StrictMode strictness);
-
     bool isGenerator()              const { return funCxFlags.isGenerator; }
     bool mightAliasLocals()         const { return funCxFlags.mightAliasLocals; }
     bool hasExtensibleScope()       const { return funCxFlags.hasExtensibleScope; }
     bool argumentsHasLocalBinding() const { return funCxFlags.argumentsHasLocalBinding; }
     bool definitelyNeedsArgsObj()   const { return funCxFlags.definitelyNeedsArgsObj; }
 
     void setIsGenerator()                  { funCxFlags.isGenerator              = true; }
     void setMightAliasLocals()             { funCxFlags.mightAliasLocals         = true; }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/arguments/defaults-bug790424.js
@@ -0,0 +1,4 @@
+function f1(g=((function () { return 4; }) for (x of [1]))) {
+  return g.next()();
+}
+assertEq(f1(), 4);