Bug 1022962 - Evaluate default parameters before function declarations. r=jorendorff
authorTooru Fujisawa <arai_a@mac.com>
Tue, 15 Jul 2014 11:47:00 +0200
changeset 216221 b3918dd99a0eff3f061233a7c6bb4cf37cf31b49
parent 216220 0152913dbac70a56b44ee68a8c013c7ba0df7844
child 216222 f8f2e03fcf8876f43a45267fca40b23f6c21a8ca
push id515
push userraliiev@mozilla.com
push dateMon, 06 Oct 2014 12:51:51 +0000
treeherdermozilla-release@267c7a481bef [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1022962
milestone33.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1022962 - Evaluate default parameters before function declarations. r=jorendorff
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/Parser.cpp
js/src/jit-test/tests/arguments/alias-function-closed.js
js/src/jit-test/tests/arguments/alias-function-not-closed.js
js/src/jit-test/tests/arguments/defaults-bound-to-function.js
js/src/jsreflect.cpp
js/src/tests/js1_8_5/extensions/reflect-parse.js
js/src/vm/Xdr.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -6242,17 +6242,17 @@ EmitUnary(ExclusiveContext *cx, Bytecode
 
 static bool
 EmitDefaults(ExclusiveContext *cx, BytecodeEmitter *bce, ParseNode *pn)
 {
     JS_ASSERT(pn->isKind(PNK_ARGSBODY));
 
     ParseNode *arg, *pnlast = pn->last();
     for (arg = pn->pn_head; arg != pnlast; arg = arg->pn_next) {
-        if (!(arg->pn_dflags & PND_DEFAULT) || !arg->isKind(PNK_NAME))
+        if (!(arg->pn_dflags & PND_DEFAULT))
             continue;
         if (!BindNameToSlot(cx, bce, arg))
             return false;
         if (!EmitVarOp(cx, arg, JSOP_GETARG, bce))
             return false;
         if (Emit1(cx, bce, JSOP_UNDEFINED) < 0)
             return false;
         if (Emit1(cx, bce, JSOP_STRICTEQ) < 0)
@@ -6297,45 +6297,28 @@ frontend::EmitTree(ExclusiveContext *cx,
 
       case PNK_ARGSBODY:
       {
         RootedFunction fun(cx, bce->sc->asFunctionBox()->function());
         ParseNode *pnlast = pn->last();
 
         // Carefully emit everything in the right order:
         // 1. Destructuring
-        // 2. Functions
-        // 3. Defaults
+        // 2. Defaults
+        // 3. Functions
         ParseNode *pnchild = pnlast->pn_head;
         if (pnlast->pn_xflags & PNX_DESTRUCT) {
             // Assign the destructuring arguments before defining any functions,
             // see bug 419662.
             JS_ASSERT(pnchild->isKind(PNK_SEMI));
             JS_ASSERT(pnchild->pn_kid->isKind(PNK_VAR) || pnchild->pn_kid->isKind(PNK_CONST));
             if (!EmitTree(cx, bce, pnchild))
                 return false;
             pnchild = pnchild->pn_next;
         }
-        if (pnlast->pn_xflags & PNX_FUNCDEFS) {
-            // This block contains top-level function definitions. To ensure
-            // that we emit the bytecode defining them before 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.
-            for (ParseNode *pn2 = pnchild; pn2; pn2 = pn2->pn_next) {
-                if (pn2->isKind(PNK_FUNCTION) && pn2->functionIsHoisted()) {
-                    if (!EmitTree(cx, bce, pn2))
-                        return false;
-                }
-            }
-        }
         bool hasDefaults = bce->sc->asFunctionBox()->hasDefaults();
         if (hasDefaults) {
             ParseNode *rest = nullptr;
             bool restIsDefn = false;
             if (fun->hasRest()) {
                 JS_ASSERT(!bce->sc->asFunctionBox()->argumentsHasLocalBinding());
 
                 // Defaults with a rest parameter need special handling. The
@@ -6390,16 +6373,33 @@ frontend::EmitTree(ExclusiveContext *cx,
                 CheckTypeSet(cx, bce, JSOP_REST);
                 if (!EmitVarOp(cx, pn2, JSOP_SETARG, bce))
                     return false;
                 if (Emit1(cx, bce, JSOP_POP) < 0)
                     return false;
                 bce->switchToMain();
             }
         }
+        if (pnlast->pn_xflags & PNX_FUNCDEFS) {
+            // This block contains top-level function definitions. To ensure
+            // that we emit the bytecode defining them before 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.
+            for (ParseNode *pn2 = pnchild; pn2; pn2 = pn2->pn_next) {
+                if (pn2->isKind(PNK_FUNCTION) && pn2->functionIsHoisted()) {
+                    if (!EmitTree(cx, bce, pn2))
+                        return false;
+                }
+            }
+        }
         ok = EmitTree(cx, bce, pnlast);
         break;
       }
 
       case PNK_IF:
         ok = EmitIf(cx, bce, pn);
         break;
 
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -1161,17 +1161,17 @@ Parser<FullParseHandler>::makeDefIntoUse
         pn->dn_uses = dn->pn_link;
         handler.prepareNodeForMutation(dn);
         dn->setKind(PNK_NOP);
         dn->setArity(PN_NULLARY);
         return true;
     }
 
     /*
-     * If dn is arg, or in [var, const, let] and has an initializer, then we
+     * If dn is in [var, const, let] and has an initializer, then we
      * must rewrite it to be an assignment node, whose freshly allocated
      * left-hand side becomes a use of pn.
      */
     if (dn->canHaveInitializer()) {
         if (ParseNode *rhs = dn->expr()) {
             ParseNode *lhs = handler.makeAssignment(dn, rhs);
             if (!lhs)
                 return false;
@@ -1748,18 +1748,32 @@ Parser<FullParseHandler>::checkFunctionD
             /*
              * Body-level function statements are effectively variable
              * declarations where the initialization is hoisted to the
              * beginning of the block. This means that any other variable
              * declaration with the same name is really just an assignment to
              * the function's binding (which is mutable), so turn any existing
              * declaration into a use.
              */
-            if (bodyLevel && !makeDefIntoUse(dn, pn, funName))
-                return false;
+            if (bodyLevel) {
+                if (dn->kind() == Definition::ARG) {
+                    // The exception to the above comment is when the function
+                    // has the same name as an argument. Then the argument node
+                    // remains a definition. But change the function node pn so
+                    // that it knows where the argument is located.
+                    pn->setOp(JSOP_GETARG);
+                    pn->setDefn(true);
+                    pn->pn_cookie = dn->pn_cookie;
+                    pn->pn_dflags |= PND_BOUND;
+                    dn->markAsAssigned();
+                } else {
+                    if (!makeDefIntoUse(dn, pn, funName))
+                        return false;
+                }
+            }
         } else if (bodyLevel) {
             /*
              * If this function was used before it was defined, claim the
              * pre-created definition node for this function that primaryExpr
              * put in pc->lexdeps on first forward reference, and recycle pn.
              */
             if (Definition *fn = pc->lexdeps.lookupDefn<FullParseHandler>(funName)) {
                 JS_ASSERT(fn->isDefn());
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/arguments/alias-function-closed.js
@@ -0,0 +1,169 @@
+function f1(a, aIs,        // without default parameter
+            b=()=>62, bIs, // with default parameter
+            // before function body
+            c=(assertEq(a(), aIs), assertEq(b(), bIs),
+               ()=>63),
+            cIs) {
+  // before function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 53);
+  assertEq(c(), 55);
+
+  function a() {
+    return 52;
+  }
+  function c() {
+    return 54;
+  }
+  function g() {
+    // close here
+    assertEq(a(), 52); // after function declaration
+    assertEq(b(), 53); // before function declaration
+    assertEq(c(), 55); // before last function declaration
+  }
+  // function declaration after closed
+  function b() {
+    return 53;
+  }
+
+  assertEq(a(), 52); // after function declaration
+  assertEq(b(), 53); // after function declaration
+  assertEq(c(), 55); // before last function declaration
+
+  g();
+  c = ()=>72;
+  assertEq(c(), 72); // after assignment in body
+  h();
+  assertEq(c(), 82); // after assignment in closed function
+
+  function h() {
+    assertEq(c(), 72); // after assignment in body
+    c = ()=>82;
+    assertEq(c(), 82); // after assignment in closed function
+  }
+  // function re-declaration after closed and assignment
+  function c() {
+    return 55;
+  }
+}
+f1(()=>42, 42, undefined, 62, undefined, 63);
+f1(()=>42, 42, undefined, 62, ()=>44, 44);
+f1(()=>42, 42, ()=>43, 43, undefined, 63);
+f1(()=>42, 42, ()=>43, 43, ()=>44, 44);
+
+function f2(a, aIs,
+            // call before body
+            b=(function() { assertEq(a(), aIs); })(),
+            // call inside body
+            c=function() { assertEq(a(), 52); }) {
+  function a() {
+    return 52;
+  }
+  function g() {
+    // close here
+    assertEq(a(), 52);
+  }
+
+  assertEq(a(), 52);
+  g();
+  c();
+}
+f2(()=>42, 42);
+
+function f3(a, aIs,
+            // call before body
+            // close here
+            b=(function() { assertEq(a(), aIs); })(),
+            // call inside body
+            c=function() { assertEq(a(), 52); }) {
+  function a() {
+    return 52;
+  }
+
+  assertEq(a(), 52);
+  c();
+}
+f3(()=>42, 42);
+
+function f4(a,
+            // assignment before body
+            b=a=()=>62,
+            c=(assertEq(a(), 62)),
+            // function declaration before body
+            d=eval("function a() { return 72; }"),
+            e=(assertEq(a(), 72))) {
+  function a() {
+    return 52;
+  }
+  function g() {
+    // close here
+    assertEq(a(), 52);
+  }
+
+  assertEq(a(), 52);
+  g();
+}
+f4(()=>42);
+
+function f5(a, b, c, d) {
+  // before var/function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 53);
+  assertEq(c(), 54);
+  assertEq(d(), 55);
+
+  function g() {
+    // before var/function declarations, called after var declarations
+    // close here
+    assertEq(a(), 52);
+    assertEq(b(), 63);
+    assertEq(c(), 54);
+    assertEq(d(), 65);
+  }
+
+  var a, b = ()=>63;
+  let c, d = ()=>65;
+
+  // after var declarations, before function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 63);
+  assertEq(c(), 54);
+  assertEq(d(), 65);
+
+  function h() {
+    // after var declarations, before function declarations
+    assertEq(a(), 52);
+    assertEq(b(), 63);
+    assertEq(c(), 54);
+    assertEq(d(), 65);
+  }
+  function a() {
+    return 52;
+  }
+  function b() {
+    return 53;
+  }
+  function c() {
+    return 54;
+  }
+  function d() {
+    return 55;
+  }
+  function i() {
+    // after var/function declarations
+    assertEq(a(), 52);
+    assertEq(b(), 63);
+    assertEq(c(), 54);
+    assertEq(d(), 65);
+  }
+
+  // after var/function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 63);
+  assertEq(c(), 54);
+  assertEq(d(), 65);
+  g();
+  h();
+  i();
+}
+f5(()=>42, ()=>43, ()=>44, ()=>45);
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/arguments/alias-function-not-closed.js
@@ -0,0 +1,89 @@
+function f1(a, aIs,        // without default parameter
+            b=()=>62, bIs, // with default parameter
+            // before function body
+            c=(assertEq(a(), aIs), assertEq(b(), bIs),
+               ()=>63),
+            cIs) {
+  // before function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 53);
+  assertEq(c(), 55);
+
+  function a() {
+    return 52;
+  }
+  function b() {
+    return 53;
+  }
+  function c() {
+    return 54;
+  }
+
+  assertEq(a(), 52); // after function declaration
+  assertEq(b(), 53); // after function declaration
+  assertEq(c(), 55); // before last function declaration
+
+  c = ()=>72;
+  assertEq(c(), 72); // after assignment in body
+
+  // function re-declaration after assignment
+  function c() {
+    return 55;
+  }
+}
+f1(()=>42, 42, undefined, 62, undefined, 63);
+f1(()=>42, 42, undefined, 62, ()=>44, 44);
+f1(()=>42, 42, ()=>43, 43, undefined, 63);
+f1(()=>42, 42, ()=>43, 43, ()=>44, 44);
+
+function f2(a,
+            // assignment before body
+            b=a=()=>62,
+            c=(assertEq(a(), 62)),
+            // function declaration before body
+            d=eval("function a() { return 72; }"),
+            e=(assertEq(a(), 72))) {
+  function a() {
+    return 52;
+  }
+
+  assertEq(a(), 52);
+}
+f2(()=>42);
+
+function f3(a, b, c, d) {
+  // before var/function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 53);
+  assertEq(c(), 54);
+  assertEq(d(), 55);
+
+  var a, b = ()=>63;
+  let c, d = ()=>65;
+
+  // after var declarations, before function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 63);
+  assertEq(c(), 54);
+  assertEq(d(), 65);
+
+  function a() {
+    return 52;
+  }
+  function b() {
+    return 53;
+  }
+  function c() {
+    return 54;
+  }
+  function d() {
+    return 55;
+  }
+
+  // after var/function declarations
+  assertEq(a(), 52);
+  assertEq(b(), 63);
+  assertEq(c(), 54);
+  assertEq(d(), 65);
+}
+f3(()=>42, ()=>43, ()=>44, ()=>45);
--- a/js/src/jit-test/tests/arguments/defaults-bound-to-function.js
+++ b/js/src/jit-test/tests/arguments/defaults-bound-to-function.js
@@ -5,20 +5,30 @@ function f(a=42) {
     function a() { return 19; }
 }
 assertEq(f()(), 19);
 function h(a=b, b=43) {
     return [a, b];
     function b() { return 42; }
 }
 var res = h();
-assertEq(res[0], res[1]);
-assertEq(res[0](), 42);
+assertEq(res[0], undefined);
+assertEq(res[1](), 42);
 function i(b=FAIL) {
     function b() {}
 }
-i();
+assertThrowsInstanceOf(i, ReferenceError);
 i(42);
 function j(a=(b=42), b=8) {
     return b;
-    function b() {}
+    function b() { return 43; }
+}
+assertEq(j()(), 43);
+function k(a=(b=42), b=8) {
+    return b;
+    function a() { return 43; }
 }
-assertEq(j(), 42);
+assertEq(k(), 42);
+function l(a=8, b=a) {
+    return b;
+    function a() { return 42; }
+}
+assertEq(l(), 8);
--- a/js/src/jsreflect.cpp
+++ b/js/src/jsreflect.cpp
@@ -3200,17 +3200,17 @@ ASTSerializer::functionArgs(ParseNode *p
             ParseNode *argName = arg->isKind(PNK_NAME) ? arg : arg->pn_left;
             if (!identifier(argName, &node))
                 return false;
             if (rest.isUndefined() && arg->pn_next == pnbody)
                 rest.setObject(node.toObject());
             else if (!args.append(node))
                 return false;
             if (arg->pn_dflags & PND_DEFAULT) {
-                ParseNode *expr = arg->isDefn() ? arg->expr() : arg->pn_kid->pn_right;
+                ParseNode *expr = arg->expr();
                 RootedValue def(cx);
                 if (!expression(expr, &def) || !defaults.append(def))
                     return false;
             }
             arg = arg->pn_next;
         } else {
             LOCAL_NOT_REACHED("missing function argument");
         }
--- a/js/src/tests/js1_8_5/extensions/reflect-parse.js
+++ b/js/src/tests/js1_8_5/extensions/reflect-parse.js
@@ -214,17 +214,17 @@ assertDecl("function foo(...rest) { }",
            funDecl(ident("foo"), [], blockStmt([]), [], ident("rest")));
 
 assertDecl("function foo(a=4) { }", funDecl(ident("foo"), [ident("a")], blockStmt([]), [lit(4)]));
 assertDecl("function foo(a, b=4) { }", funDecl(ident("foo"), [ident("a"), ident("b")], blockStmt([]), [lit(4)]));
 assertDecl("function foo(a, b=4, ...rest) { }",
            funDecl(ident("foo"), [ident("a"), ident("b")], blockStmt([]), [lit(4)], ident("rest")));
 assertDecl("function foo(a=(function () {})) { function a() {} }",
            funDecl(ident("foo"), [ident("a")], blockStmt([funDecl(ident("a"), [], blockStmt([]))]),
-                   [funExpr(ident("a"), [], blockStmt([]))]));
+                   [funExpr(null, [], blockStmt([]))]));
 
 
 // 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() { } }",
--- a/js/src/vm/Xdr.h
+++ b/js/src/vm/Xdr.h
@@ -23,17 +23,17 @@ namespace js {
  * versions.  If deserialization fails, the data should be invalidated if
  * possible.
  *
  * When you change this, run make_opcode_doc.py and copy the new output into
  * this wiki page:
  *
  *  https://developer.mozilla.org/en-US/docs/SpiderMonkey/Internals/Bytecode
  */
-static const uint32_t XDR_BYTECODE_VERSION = uint32_t(0xb973c0de - 177);
+static const uint32_t XDR_BYTECODE_VERSION = uint32_t(0xb973c0de - 178);
 
 class XDRBuffer {
   public:
     explicit XDRBuffer(JSContext *cx)
       : context(cx), base(nullptr), cursor(nullptr), limit(nullptr) { }
 
     JSContext *cx() const {
         return context;