Bug 1071646 - Introduce JSOP_BINDVAR to support Annex B.3.3.3. (r=jorendorff)
authorShu-yu Guo <shu@rfrn.org>
Wed, 09 Dec 2015 07:52:58 -0800
changeset 297662 a44b841c33fb129b236e5dceab9129370511f418
parent 297661 5b4fe5acd50c8e16c1df2d9cb312c6875f3028fc
child 297663 52ad4194c0b33df6f30b8149f415303a52d18591
push id8824
push userraliiev@mozilla.com
push dateMon, 14 Dec 2015 20:18:56 +0000
treeherdermozilla-aurora@e2031358e2a6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1071646
milestone45.0a1
Bug 1071646 - Introduce JSOP_BINDVAR to support Annex B.3.3.3. (r=jorendorff)
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/BytecodeEmitter.h
js/src/frontend/Parser.cpp
js/src/tests/ecma_6/LexicalEnvironment/block-scoped-functions-annex-b-eval.js
js/src/vm/Interpreter.cpp
js/src/vm/Opcodes.h
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -4374,17 +4374,17 @@ BytecodeEmitter::emitVariables(ParseNode
         } else if (binding->isKind(PNK_ASSIGN)) {
             /*
              * A destructuring initialiser assignment preceded by var will
              * never occur to the left of 'in' in a for-in loop.  As with 'for
              * (var x = i in o)...', this will cause the entire 'var [a, b] =
              * i' to be hoisted out of the loop.
              */
             MOZ_ASSERT(binding->isOp(JSOP_NOP));
-            MOZ_ASSERT(emitOption != DefineVars);
+            MOZ_ASSERT(emitOption != DefineVars && emitOption != AnnexB);
 
             /*
              * To allow the front end to rewrite var f = x; as f = x; when a
              * function f(){} precedes the var, detect simple name assignment
              * here and initialize the name.
              */
             if (binding->pn_left->isKind(PNK_NAME)) {
                 if (!emitSingleVariable(pn, binding->pn_left, binding->pn_right, emitOption))
@@ -4435,23 +4435,28 @@ BytecodeEmitter::emitSingleVariable(Pars
     if (initializer) {
         MOZ_ASSERT(emitOption != DefineVars);
         if (op == JSOP_SETNAME ||
             op == JSOP_STRICTSETNAME ||
             op == JSOP_SETGNAME ||
             op == JSOP_STRICTSETGNAME)
         {
             MOZ_ASSERT(emitOption != PushInitialValues);
-            JSOp bindOp;
-            if (op == JSOP_SETNAME || op == JSOP_STRICTSETNAME)
-                bindOp = JSOP_BINDNAME;
-            else
-                bindOp = JSOP_BINDGNAME;
-            if (!emitIndex32(bindOp, atomIndex))
-                return false;
+            if (op == JSOP_SETGNAME || op == JSOP_STRICTSETGNAME) {
+                if (!emitIndex32(JSOP_BINDGNAME, atomIndex))
+                    return false;
+            } else if (emitOption == AnnexB) {
+                // Annex B vars always go on the nearest variable environment,
+                // even if scopes on the chain contain same-named bindings.
+                if (!emit1(JSOP_BINDVAR))
+                    return false;
+            } else {
+                if (!emitIndex32(JSOP_BINDNAME, atomIndex))
+                    return false;
+            }
         }
 
         bool oldEmittingForInit = emittingForInit;
         emittingForInit = false;
         if (!emitTree(initializer))
             return false;
         emittingForInit = oldEmittingForInit;
     } else if (op == JSOP_INITLEXICAL ||
@@ -4465,17 +4470,17 @@ BytecodeEmitter::emitSingleVariable(Pars
             return false;
     } else {
         // The declaration is like `var x;`. Nothing to do.
         return true;
     }
 
     // If we are not initializing, nothing to pop. If we are initializing
     // lets, we must emit the pops.
-    if (emitOption == InitializeVars) {
+    if (emitOption == InitializeVars || emitOption == AnnexB) {
         MOZ_ASSERT_IF(binding->isDefn(), initializer == binding->pn_expr);
         if (!binding->pn_scopecoord.isFree()) {
             if (!emitVarOp(binding, op))
                 return false;
         } else {
             if (!emitIndexOp(op, atomIndex))
                 return false;
         }
@@ -6228,22 +6233,23 @@ BytecodeEmitter::emitFunction(ParseNode*
      * function will be seen by emitFunction in two places.
      */
     if (funbox->wasEmitted) {
         // Annex B block-scoped functions are hoisted like any other
         // block-scoped function to the top of their scope. When their
         // definitions are seen for the second time, we need to emit the
         // assignment that assigns the function to the outer 'var' binding.
         if (assignmentForAnnexB) {
-            if (!emitTree(assignmentForAnnexB))
-                return false;
-
-            // If we did not synthesize a new binding and only a simple
-            // assignment, manually pop the result.
-            if (assignmentForAnnexB->isKind(PNK_ASSIGN)) {
+            if (assignmentForAnnexB->isKind(PNK_VAR)) {
+                if (!emitVariables(assignmentForAnnexB, AnnexB))
+                    return false;
+            } else {
+                MOZ_ASSERT(assignmentForAnnexB->isKind(PNK_ASSIGN));
+                if (!emitTree(assignmentForAnnexB))
+                    return false;
                 if (!emit1(JSOP_POP))
                     return false;
             }
         }
 
         MOZ_ASSERT_IF(fun->hasScript(), fun->nonLazyScript());
         MOZ_ASSERT(pn->functionIsHoisted());
         return true;
--- a/js/src/frontend/BytecodeEmitter.h
+++ b/js/src/frontend/BytecodeEmitter.h
@@ -116,17 +116,22 @@ enum VarEmitOption {
     // used in one case: `for (var $BindingPattern in/of obj)`. If we're at
     // toplevel, the variable(s) must be defined with JSOP_DEFVAR, but they're
     // populated inside the loop, via emitAssignment.
     DefineVars,
 
     // Emit code to evaluate initializer expressions and leave those values on
     // the stack. This is used to implement `for (let/const ...;;)` and
     // deprecated `let` blocks.
-    PushInitialValues
+    PushInitialValues,
+
+    // Like InitializeVars, but bind using BINDVAR instead of
+    // BINDNAME/BINDGNAME. Only used for emitting declarations synthesized for
+    // Annex B block-scoped function semantics.
+    AnnexB,
 };
 
 struct BytecodeEmitter
 {
     SharedContext* const sc;      /* context shared between parsing and bytecode generation */
 
     ExclusiveContext* const cx;
 
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -2355,18 +2355,18 @@ Parser<FullParseHandler>::checkFunctionD
         } else {
             Definition* annexDef = nullptr;
             Node synthesizedDeclarationList = null();
 
             if (!pc->sc->strict()) {
                 // Under non-strict mode, try ES6 Annex B.3.3 semantics. If
                 // making an additional 'var' binding of the same name does
                 // not throw an early error, do so. This 'var' binding would
-                // be assigned the function object in situ, e.g., when its
-                // declaration is reached, not at the start of the block.
+                // be assigned the function object when its declaration is
+                // reached, not at the start of the block.
 
                 annexDef = pc->decls().lookupFirst(funName);
                 if (annexDef) {
                     if (annexDef->kind() == Definition::CONSTANT ||
                         annexDef->kind() == Definition::LET)
                     {
                         // Do not emit Annex B assignment if we would've
                         // thrown a redeclaration error.
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/LexicalEnvironment/block-scoped-functions-annex-b-eval.js
@@ -0,0 +1,38 @@
+var log = "";
+
+function f() {
+  log += g();
+  function g() { return "outer-g"; }
+
+  var o = { g: function () { return "with-g"; } };
+  with (o) {
+    // Annex B.3.3.3 says g should be set on the nearest VariableEnvironment,
+    // and so should not change o.g.
+    eval(`{
+      function g() { return "eval-g"; }
+    }`);
+  }
+
+  log += g();
+  log += o.g();
+}
+
+f();
+
+function h() {
+  eval(`
+    // Should return true, as var bindings introduced by eval are configurable.
+    log += (delete q);
+    {
+      function q() { log += "q"; }
+      // Should return false, as lexical bindings introduced by eval are not
+      // configurable.
+      log += (delete q);
+    }
+  `);
+  return q;
+}
+
+h()();
+
+reportCompare(log, "outer-geval-gwith-gtruefalseq");
--- a/js/src/vm/Interpreter.cpp
+++ b/js/src/vm/Interpreter.cpp
@@ -1749,17 +1749,16 @@ CASE(JSOP_UNUSED181)
 CASE(JSOP_UNUSED182)
 CASE(JSOP_UNUSED183)
 CASE(JSOP_UNUSED187)
 CASE(JSOP_UNUSED192)
 CASE(JSOP_UNUSED209)
 CASE(JSOP_UNUSED210)
 CASE(JSOP_UNUSED211)
 CASE(JSOP_UNUSED212)
-CASE(JSOP_UNUSED213)
 CASE(JSOP_UNUSED219)
 CASE(JSOP_UNUSED220)
 CASE(JSOP_UNUSED221)
 CASE(JSOP_UNUSED222)
 CASE(JSOP_UNUSED223)
 CASE(JSOP_CONDSWITCH)
 CASE(JSOP_TRY)
 {
@@ -2085,16 +2084,22 @@ CASE(JSOP_BINDNAME)
 
     PUSH_OBJECT(*scope);
 
     static_assert(JSOP_BINDNAME_LENGTH == JSOP_BINDGNAME_LENGTH,
                   "We're sharing the END_CASE so the lengths better match");
 }
 END_CASE(JSOP_BINDNAME)
 
+CASE(JSOP_BINDVAR)
+{
+    PUSH_OBJECT(REGS.fp()->varObj());
+}
+END_CASE(JSOP_BINDVAR)
+
 #define BITWISE_OP(OP)                                                        \
     JS_BEGIN_MACRO                                                            \
         int32_t i, j;                                                         \
         if (!ToInt32(cx, REGS.stackHandleAt(-2), &i))                         \
             goto error;                                                       \
         if (!ToInt32(cx, REGS.stackHandleAt(-1), &j))                         \
             goto error;                                                       \
         i = i OP j;                                                           \
--- a/js/src/vm/Opcodes.h
+++ b/js/src/vm/Opcodes.h
@@ -2040,17 +2040,25 @@ 1234567890123456789012345678901234567890
      *   Operands:
      *   Stack: =>
      */ \
     macro(JSOP_DEBUGAFTERYIELD,  208, "debugafteryield",  NULL,  1,  0,  0,  JOF_BYTE) \
     macro(JSOP_UNUSED209,     209, "unused209",    NULL,  1,  0,  0,  JOF_BYTE) \
     macro(JSOP_UNUSED210,     210, "unused210",    NULL,  1,  0,  0,  JOF_BYTE) \
     macro(JSOP_UNUSED211,     211, "unused211",    NULL,  1,  0,  0,  JOF_BYTE) \
     macro(JSOP_UNUSED212,     212, "unused212",    NULL,  1,  0,  0,  JOF_BYTE) \
-    macro(JSOP_UNUSED213,     213, "unused213",    NULL,  1,  0,  0,  JOF_BYTE) \
+    /*
+     * Pushes the nearest 'var' environment.
+     *
+     *   Category: Variables and Scopes
+     *   Type: Free Variables
+     *   Operands:
+     *   Stack: => scope
+     */ \
+    macro(JSOP_BINDVAR,       213, "bindvar",      NULL,  1,  0,  1,  JOF_BYTE) \
     /*
      * Pushes the global scope onto the stack if the script doesn't have a
      * non-syntactic global scope.  Otherwise will act like JSOP_BINDNAME.
      *
      * 'nameIndex' is only used when acting like JSOP_BINDNAME.
      *   Category: Variables and Scopes
      *   Type: Free Variables
      *   Operands: uint32_t nameIndex