Bug 1532921 - Implement .initializers local variable. r=jorendorff

Differential Revision: https://phabricator.services.mozilla.com/D22290

--HG--
extra : moz-landing-system : lando

js/src/frontend/BytecodeEmitter.cpp

@@ -2460,19 +2460,7 @@ bool BytecodeEmitter::emitInitializeInstanceFields() {
     return true;
   }
 
-  PropOpEmitter poe(this, PropOpEmitter::Kind::Get,
-                    PropOpEmitter::ObjKind::Other);
-  if (!poe.prepareForObj()) {
-    return false;
-  }
-
-  // This is guaranteed to run after super(), so we don't need TDZ checks.
-  if (!emitGetName(cx->names().dotThis)) {
-    //              [stack] THIS
-    return false;
-  }
-
-  if (!poe.emitGet(cx->names().dotInitializers)) {
+  if (!emitGetName(cx->names().dotInitializers)) {
     //              [stack] ARRAY
     return false;
   }
@@ -8020,23 +8008,14 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
     // code (the initializer) for each field. Upon an object's construction,
     // these lambdas will be called, defining the values.
 
-    PropOpEmitter poe(this, PropOpEmitter::Kind::SimpleAssignment,
-                      PropOpEmitter::ObjKind::Other);
-    if (!poe.prepareForObj()) {
-      return false;
-    }
-
-    if (!emit1(JSOP_DUP)) {
-      //            [stack] CTOR? OBJ OBJ
-      return false;
-    }
-
-    if (!poe.prepareForRhs()) {
+    NameOpEmitter noe(this, cx->names().dotInitializers,
+                      NameOpEmitter::Kind::Initialize);
+    if (!noe.prepareForRhs()) {
       return false;
     }
 
     if (!emitUint32Operand(JSOP_NEWARRAY, numFields)) {
-      //            [stack] CTOR? OBJ OBJ ARRAY
+      //            [stack] CTOR? OBJ ARRAY
       return false;
     }
 
@@ -8049,12 +8028,12 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
         }
 
         if (!emitTree(initializer)) {
-          //        [stack] CTOR? OBJ OBJ ARRAY LAMBDA
+          //        [stack] CTOR? OBJ ARRAY LAMBDA
           return false;
         }
 
         if (!emitUint32Operand(JSOP_INITELEM_ARRAY, curFieldIndex)) {
-          //        [stack] CTOR? OBJ OBJ ARRAY
+          //        [stack] CTOR? OBJ ARRAY
           return false;
         }
 
@@ -8062,7 +8041,7 @@ bool BytecodeEmitter::emitPropertyList(ListNode* obj, PropertyEmitter& pe,
       }
     }
 
-    if (!poe.emitAssignment(cx->names().dotInitializers)) {
+    if (!noe.emitAssignment()) {
       //            [stack] CTOR? OBJ ARRAY
       return false;
     }

js/src/frontend/ParseContext.h

@@ -137,6 +137,16 @@ class UsedNameTracker {
   MOZ_MUST_USE bool noteUse(JSContext* cx, JSAtom* name, uint32_t scriptId,
                             uint32_t scopeId);
 
+  MOZ_MUST_USE bool markAsAlwaysClosedOver(JSContext* cx, JSAtom* name,
+                                           uint32_t scriptId,
+                                           uint32_t scopeId) {
+    // This marks a variable as always closed over:
+    // UsedNameInfo::noteBoundInScope only checks if scriptId and scopeId are
+    // greater than the current scriptId/scopeId, so do a simple increment to
+    // make that so.
+    return noteUse(cx, name, scriptId + 1, scopeId + 1);
+  }
+
   struct RewindToken {
    private:
     friend class UsedNameTracker;

js/src/frontend/Parser.cpp

@@ -1829,6 +1829,10 @@ GeneralParser<ParseHandler, Unit>::functionBody(InHandling inHandling,
   uint32_t startYieldOffset = pc_->lastYieldOffset;
 #endif
 
+  // One might expect noteUsedName(".initializers") here when parsing a
+  // constructor. See GeneralParser<ParseHandler, Unit>::classDefinition on why
+  // it's not here.
+
   Node body;
   if (type == StatementListBody) {
     bool inheritedStrict = pc_->sc()->strict();
@@ -7044,6 +7048,37 @@ GeneralParser<ParseHandler, Unit>::classDefinition(
       }
     }
 
+    if (numFieldsWithInitializers > 0) {
+      // .initializers is always closed over by the constructor when there are
+      // fields with initializers. However, there's some strange circumstances
+      // which prevents us from using the normal noteUsedName() system. We
+      // cannot call noteUsedName(".initializers") when parsing the constructor,
+      // because .initializers should be marked as used *only if* there are
+      // fields with initializers. Even if we haven't seen any fields yet,
+      // there may be fields after the constructor.
+      // Consider the following class:
+      //
+      //  class C {
+      //    constructor() {
+      //      // do we noteUsedName(".initializers") here?
+      //    }
+      //    // ... because there might be some fields down here.
+      //  }
+      //
+      // So, instead, at the end of class parsing (where we are now), we do some
+      // tricks to pretend that noteUsedName(".initializers") was called in the
+      // constructor.
+      if (!usedNames_.markAsAlwaysClosedOver(cx_, cx_->names().dotInitializers,
+                                             pc_->scriptId(),
+                                             pc_->innermostScope()->id())) {
+        return null();
+      }
+      if (!noteDeclaredName(cx_->names().dotInitializers,
+                            DeclarationKind::Const, namePos)) {
+        return null();
+      }
+    }
+
     classEndOffset = pos().end;
     if (!finishClassConstructor(classStmt, className, classStartOffset,
                                 classEndOffset, numFieldsWithInitializers,
@@ -7161,6 +7196,9 @@ GeneralParser<ParseHandler, Unit>::synthesizeConstructor(
     return null();
   }
 
+  // One might expect a noteUsedName(".initializers") here. See comment in
+  // GeneralParser<ParseHandler, Unit>::classDefinition on why it's not here.
+
   bool canSkipLazyClosedOverBindings = handler_.canSkipLazyClosedOverBindings();
   if (!pc_->declareFunctionThis(usedNames_, canSkipLazyClosedOverBindings)) {
     return null();

js/src/tests/non262/fields/scopes.js

@@ -0,0 +1,11 @@
+(function({
+    k = class i {
+              [_ => i]()
+              {}
+            }
+} = {}) {
+    var j = 0;
+})()
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);