Bug 1542448 - Always emit scopes for classes. r=jorendorff
authorAshley Hauck <khyperia@mozilla.com>
Thu, 11 Apr 2019 23:07:04 +0000
changeset 469149 83ab9bf5138e6e4a4d1ab32fa5ae672593141dfc
parent 469148 aa39f59b4c79f1714a94265a0ac1ff6abaf95e98
child 469150 d11fc84ce16f489bac63f4055ddd484aa9435589
push id35856
push usercsabou@mozilla.com
push dateFri, 12 Apr 2019 03:19:48 +0000
treeherdermozilla-central@940684cd1065 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1542448
milestone68.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 1542448 - Always emit scopes for classes. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D26966
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/FullParseHandler.h
js/src/frontend/ObjectEmitter.cpp
js/src/frontend/ObjectEmitter.h
js/src/frontend/ParseNode.h
js/src/frontend/Parser.cpp
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -7970,17 +7970,17 @@ bool BytecodeEmitter::emitCreateFieldKey
 
   NameOpEmitter noe(this, cx->names().dotFieldKeys,
                     NameOpEmitter::Kind::Initialize);
   if (!noe.prepareForRhs()) {
     return false;
   }
 
   if (!emitUint32Operand(JSOP_NEWARRAY, numFieldKeys)) {
-    //            [stack] ARRAY
+    //              [stack] ARRAY
     return false;
   }
 
   size_t curFieldKeyIndex = 0;
   for (ParseNode* propdef : obj->contents()) {
     if (propdef->is<ClassField>()) {
       ClassField* field = &propdef->as<ClassField>();
       if (field->name().getKind() == ParseNodeKind::ComputedName) {
@@ -7998,22 +7998,22 @@ bool BytecodeEmitter::emitCreateFieldKey
 
         curFieldKeyIndex++;
       }
     }
   }
   MOZ_ASSERT(curFieldKeyIndex == numFieldKeys);
 
   if (!noe.emitAssignment()) {
-    //            [stack] ARRAY
+    //              [stack] ARRAY
     return false;
   }
 
   if (!emit1(JSOP_POP)) {
-    //            [stack]
+    //              [stack]
     return false;
   }
 
   return true;
 }
 
 bool BytecodeEmitter::emitCreateFieldInitializers(ListNode* obj) {
   FieldInitializers fieldInitializers = setupFieldInitializers(obj);
@@ -8030,49 +8030,49 @@ bool BytecodeEmitter::emitCreateFieldIni
 
   NameOpEmitter noe(this, cx->names().dotInitializers,
                     NameOpEmitter::Kind::Initialize);
   if (!noe.prepareForRhs()) {
     return false;
   }
 
   if (!emitUint32Operand(JSOP_NEWARRAY, numFields)) {
-    //            [stack] CTOR? OBJ ARRAY
+    //              [stack] CTOR? OBJ ARRAY
     return false;
   }
 
   size_t curFieldIndex = 0;
   for (ParseNode* propdef : obj->contents()) {
     if (propdef->is<ClassField>()) {
       FunctionNode* initializer = propdef->as<ClassField>().initializer();
       if (initializer == nullptr) {
         continue;
       }
 
       if (!emitTree(initializer)) {
-        //        [stack] CTOR? OBJ ARRAY LAMBDA
+        //          [stack] CTOR? OBJ ARRAY LAMBDA
         return false;
       }
 
       if (!emitUint32Operand(JSOP_INITELEM_ARRAY, curFieldIndex)) {
-        //        [stack] CTOR? OBJ ARRAY
+        //          [stack] CTOR? OBJ ARRAY
         return false;
       }
 
       curFieldIndex++;
     }
   }
 
   if (!noe.emitAssignment()) {
-    //            [stack] CTOR? OBJ ARRAY
+    //              [stack] CTOR? OBJ ARRAY
     return false;
   }
 
   if (!emit1(JSOP_POP)) {
-    //            [stack] CTOR? OBJ
+    //              [stack] CTOR? OBJ
     return false;
   }
 
   return true;
 }
 
 const FieldInitializers& BytecodeEmitter::findFieldInitializersForCall() {
   for (BytecodeEmitter* current = this; current; current = current->parent) {
@@ -8678,18 +8678,22 @@ bool BytecodeEmitter::emitClass(
     innerName = names->innerBinding()->name();
     MOZ_ASSERT(innerName);
 
     if (names->outerBinding()) {
       MOZ_ASSERT(names->outerBinding()->name());
       MOZ_ASSERT(names->outerBinding()->name() == innerName);
       kind = ClassEmitter::Kind::Declaration;
     }
-
-    if (!ce.emitScopeForNamedClass(classNode->scopeBindings())) {
+  }
+
+  if (!classNode->isEmptyScope()) {
+    if (!ce.emitScope(classNode->scopeBindings(),
+                      classNode->names() ? ClassEmitter::HasName::Yes
+                                         : ClassEmitter::HasName::No)) {
       //            [stack]
       return false;
     }
   }
 
   // This is kind of silly. In order to the get the home object defined on
   // the constructor, we have to make it second, but we want the prototype
   // on top for EmitPropertyList, because we expect static properties to be
--- a/js/src/frontend/FullParseHandler.h
+++ b/js/src/frontend/FullParseHandler.h
@@ -321,17 +321,18 @@ class FullParseHandler {
     return new_<CallNode>(ParseNodeKind::TaggedTemplateExpr, callOp, tag, args);
   }
 
   ListNodeType newObjectLiteral(uint32_t begin) {
     return new_<ListNode>(ParseNodeKind::ObjectExpr,
                           TokenPos(begin, begin + 1));
   }
 
-  ClassNodeType newClass(Node name, Node heritage, Node memberBlock,
+  ClassNodeType newClass(Node name, Node heritage,
+                         LexicalScopeNodeType memberBlock,
                          const TokenPos& pos) {
     return new_<ClassNode>(name, heritage, memberBlock, pos);
   }
   ListNodeType newClassMemberList(uint32_t begin) {
     return new_<ListNode>(ParseNodeKind::ClassMemberList,
                           TokenPos(begin, begin + 1));
   }
   ClassNamesType newClassNames(Node outer, Node inner, const TokenPos& pos) {
--- a/js/src/frontend/ObjectEmitter.cpp
+++ b/js/src/frontend/ObjectEmitter.cpp
@@ -478,30 +478,34 @@ void AutoSaveLocalStrictMode::restore() 
 ClassEmitter::ClassEmitter(BytecodeEmitter* bce)
     : PropertyEmitter(bce),
       strictMode_(bce->sc),
       name_(bce->cx),
       nameForAnonymousClass_(bce->cx) {
   isClass_ = true;
 }
 
-bool ClassEmitter::emitScopeForNamedClass(
-    JS::Handle<LexicalScope::Data*> scopeBindings) {
+bool ClassEmitter::emitScope(JS::Handle<LexicalScope::Data*> scopeBindings,
+                             HasName hasName) {
   MOZ_ASSERT(propertyState_ == PropertyState::Start);
   MOZ_ASSERT(classState_ == ClassState::Start);
 
-  tdzCacheForInnerName_.emplace(bce_);
-  innerNameScope_.emplace(bce_);
-  if (!innerNameScope_->enterLexical(bce_, ScopeKind::Lexical, scopeBindings)) {
+  if (hasName == HasName::Yes) {
+    tdzCacheForInnerName_.emplace(bce_);
+  }
+
+  innerScope_.emplace(bce_);
+  if (!innerScope_->enterLexical(bce_, ScopeKind::Lexical, scopeBindings)) {
     return false;
   }
 
 #ifdef DEBUG
   classState_ = ClassState::Scope;
 #endif
+
   return true;
 }
 
 bool ClassEmitter::emitClass(JS::Handle<JSAtom*> name,
                              JS::Handle<JSAtom*> nameForAnonymousClass,
                              bool hasNameOnStack) {
   MOZ_ASSERT(propertyState_ == PropertyState::Start);
   MOZ_ASSERT(classState_ == ClassState::Start ||
@@ -761,45 +765,54 @@ bool ClassEmitter::emitEnd(Kind kind) {
 
   if (!bce_->emit1(JSOP_POP)) {
     //              [stack] CTOR
     return false;
   }
 
   if (name_) {
     MOZ_ASSERT(tdzCacheForInnerName_.isSome());
-    MOZ_ASSERT(innerNameScope_.isSome());
+    MOZ_ASSERT(innerScope_.isSome());
 
     if (!bce_->emitLexicalInitialization(name_)) {
       //            [stack] CTOR
       return false;
     }
 
-    if (!innerNameScope_->leave(bce_)) {
+    if (!innerScope_->leave(bce_)) {
       return false;
     }
-    innerNameScope_.reset();
+    innerScope_.reset();
 
     if (kind == Kind::Declaration) {
       if (!bce_->emitLexicalInitialization(name_)) {
         //          [stack] CTOR
         return false;
       }
       // Only class statements make outer bindings, and they do not leave
       // themselves on the stack.
       if (!bce_->emit1(JSOP_POP)) {
         //          [stack]
         return false;
       }
     }
 
     tdzCacheForInnerName_.reset();
+  } else if (innerScope_.isSome()) {
+    //              [stack] CTOR
+    MOZ_ASSERT(kind == Kind::Expression);
+    MOZ_ASSERT(tdzCacheForInnerName_.isNothing());
+
+    if (!innerScope_->leave(bce_)) {
+      return false;
+    }
+    innerScope_.reset();
   } else {
     //              [stack] CTOR
-
+    MOZ_ASSERT(kind == Kind::Expression);
     MOZ_ASSERT(tdzCacheForInnerName_.isNothing());
   }
 
   //                [stack] # class declaration
   //                [stack]
   //                [stack] # class expression
   //                [stack] CTOR
 
--- a/js/src/frontend/ObjectEmitter.h
+++ b/js/src/frontend/ObjectEmitter.h
@@ -432,67 +432,69 @@ class MOZ_RAII AutoSaveLocalStrictMode {
 };
 
 // Class for emitting bytecode for JS class.
 //
 // Usage: (check for the return value is omitted for simplicity)
 //
 //   `class {}`
 //     ClassEmitter ce(this);
+//     ce.emitScope(scopeBindings, HasName::No);
 //     ce.emitClass(nullptr, nullptr, false);
 //
 //     ce.emitInitDefaultConstructor(Some(offset_of_class),
 //                                   Some(offset_of_closing_bracket));
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class { constructor() { ... } }`
 //     ClassEmitter ce(this);
+//     ce.emitScope(scopeBindings, HasName::No);
 //     ce.emitClass(nullptr, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     ce.emitInitConstructor(/* needsHomeObject = */ false);
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X { constructor() { ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScopeForNamedClass(scopeBindingForName);
+//     ce.emitScope(scopeBindings, HasName::Yes);
 //     ce.emitClass(atom_of_X, nullptr, false);
 //
 //     ce.emitInitDefaultConstructor(Some(offset_of_class),
 //                                   Some(offset_of_closing_bracket));
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X { constructor() { ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScopeForNamedClass(scopeBindingForName);
+//     ce.emitScope(scopeBindings, HasName::Yes);
 //     ce.emitClass(atom_of_X, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     ce.emitInitConstructor(/* needsHomeObject = */ false);
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X extends Y { constructor() { ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScopeForNamedClass(scopeBindingForName);
+//     ce.emitScope(scopeBindings, HasName::Yes);
 //
 //     emit(Y);
 //     ce.emitDerivedClass(atom_of_X, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     ce.emitInitConstructor(/* needsHomeObject = */ false);
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X extends Y { constructor() { ... super.f(); ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScopeForNamedClass(scopeBindingForName);
+//     ce.emitScope(scopeBindings, HasName::Yes);
 //
 //     emit(Y);
 //     ce.emitDerivedClass(atom_of_X, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     // pass true if constructor contains super.prop access
 //     ce.emitInitConstructor(/* needsHomeObject = */ true);
 //
@@ -596,31 +598,31 @@ class MOZ_STACK_CLASS ClassEmitter : pub
   //   cons.prototype = homeObject;
   //   homeObject.constructor = cons;
   //
   //   EmitPropertyList(...)
 
   bool isDerived_ = false;
 
   mozilla::Maybe<TDZCheckCache> tdzCacheForInnerName_;
-  mozilla::Maybe<EmitterScope> innerNameScope_;
+  mozilla::Maybe<EmitterScope> innerScope_;
   AutoSaveLocalStrictMode strictMode_;
 
 #ifdef DEBUG
   // The state of this emitter.
   //
   // +-------+
-  // | Start |-+------------------------------------>+-+
-  // +-------+ |                                     ^ |
-  //           | [named class]                       | |
-  //           |   emitScopeForNamedClass  +-------+ | |
-  //           +-------------------------->| Scope |-+ |
-  //                                       +-------+   |
-  //                                                   |
-  //   +-----------------------------------------------+
+  // | Start |-+------------------------>+-+
+  // +-------+ |                         ^ |
+  //           | [has scope]             | |
+  //           |   emitScope   +-------+ | |
+  //           +-------------->| Scope |-+ |
+  //                           +-------+   |
+  //                                       |
+  //   +-----------------------------------+
   //   |
   //   |   emitClass           +-------+
   //   +-+----------------->+->| Class |-+
   //     |                  ^  +-------+ |
   //     | emitDerivedClass |            |
   //     +------------------+            |
   //                                     |
   //     +-------------------------------+
@@ -636,17 +638,17 @@ class MOZ_STACK_CLASS ClassEmitter : pub
   //       |
   //       | (do PropertyEmitter operation)  emitEnd  +-----+
   //       +-------------------------------+--------->| End |
   //                                                  +-----+
   enum class ClassState {
     // The initial state.
     Start,
 
-    // After calling emitScopeForNamedClass.
+    // After calling emitScope.
     Scope,
 
     // After calling emitClass or emitDerivedClass.
     Class,
 
     // After calling emitInitConstructor or emitInitDefaultConstructor.
     InitConstructor,
 
@@ -658,18 +660,19 @@ class MOZ_STACK_CLASS ClassEmitter : pub
 
   JS::Rooted<JSAtom*> name_;
   JS::Rooted<JSAtom*> nameForAnonymousClass_;
   bool hasNameOnStack_ = false;
 
  public:
   explicit ClassEmitter(BytecodeEmitter* bce);
 
-  MOZ_MUST_USE bool emitScopeForNamedClass(
-      JS::Handle<LexicalScope::Data*> scopeBindings);
+  enum class HasName : bool { No, Yes };
+  bool emitScope(JS::Handle<LexicalScope::Data*> scopeBindings,
+                 HasName hasName);
 
   // @param name
   //        Name of the class (nullptr if this is anonymous class)
   // @param nameForAnonymousClass
   //        Statically inferred name of the class (only for anonymous classes)
   // @param hasNameOnStack
   //        If true the name is on the stack (only for anonymous classes)
   MOZ_MUST_USE bool emitClass(JS::Handle<JSAtom*> name,
--- a/js/src/frontend/ParseNode.h
+++ b/js/src/frontend/ParseNode.h
@@ -2059,46 +2059,43 @@ class ClassNames : public BinaryNode {
     return nullptr;
   }
 
   NameNode* innerBinding() const { return &right()->as<NameNode>(); }
 };
 
 class ClassNode : public TernaryNode {
  public:
-  ClassNode(ParseNode* names, ParseNode* heritage, ParseNode* membersOrBlock,
-            const TokenPos& pos)
-      : TernaryNode(ParseNodeKind::ClassDecl, names, heritage, membersOrBlock,
+  ClassNode(ParseNode* names, ParseNode* heritage,
+            LexicalScopeNode* memberBlock, const TokenPos& pos)
+      : TernaryNode(ParseNodeKind::ClassDecl, names, heritage, memberBlock,
                     pos) {
     MOZ_ASSERT_IF(names, names->is<ClassNames>());
-    MOZ_ASSERT(membersOrBlock->is<LexicalScopeNode>() ||
-               membersOrBlock->isKind(ParseNodeKind::ClassMemberList));
   }
 
   static bool test(const ParseNode& node) {
     bool match = node.isKind(ParseNodeKind::ClassDecl);
     MOZ_ASSERT_IF(match, node.is<TernaryNode>());
     return match;
   }
 
   ClassNames* names() const {
     return kid1() ? &kid1()->as<ClassNames>() : nullptr;
   }
   ParseNode* heritage() const { return kid2(); }
   ListNode* memberList() const {
-    ParseNode* membersOrBlock = kid3();
-    if (membersOrBlock->isKind(ParseNodeKind::ClassMemberList)) {
-      return &membersOrBlock->as<ListNode>();
-    }
-
     ListNode* list =
-        &membersOrBlock->as<LexicalScopeNode>().scopeBody()->as<ListNode>();
+        &kid3()->as<LexicalScopeNode>().scopeBody()->as<ListNode>();
     MOZ_ASSERT(list->isKind(ParseNodeKind::ClassMemberList));
     return list;
   }
+  bool isEmptyScope() const {
+    ParseNode* scope = kid3();
+    return scope->as<LexicalScopeNode>().isEmptyScope();
+  }
   Handle<LexicalScope::Data*> scopeBindings() const {
     ParseNode* scope = kid3();
     return scope->as<LexicalScopeNode>().scopeBindings();
   }
 };
 
 #ifdef DEBUG
 void DumpParseTree(ParseNode* pn, GenericPrinter& out, int indent = 0);
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -2829,17 +2829,21 @@ bool GeneralParser<ParseHandler, Unit>::
 template <typename Unit>
 FunctionNode* Parser<FullParseHandler, Unit>::standaloneLazyFunction(
     HandleFunction fun, uint32_t toStringStart, bool strict,
     GeneratorKind generatorKind, FunctionAsyncKind asyncKind) {
   MOZ_ASSERT(checkOptionsCalled_);
 
   FunctionSyntaxKind syntaxKind = FunctionSyntaxKind::Statement;
   if (fun->isClassConstructor()) {
-    syntaxKind = FunctionSyntaxKind::ClassConstructor;
+    if (fun->isDerivedClassConstructor()) {
+      syntaxKind = FunctionSyntaxKind::DerivedClassConstructor;
+    } else {
+      syntaxKind = FunctionSyntaxKind::ClassConstructor;
+    }
   } else if (fun->isMethod()) {
     syntaxKind = FunctionSyntaxKind::Method;
   } else if (fun->isGetter()) {
     syntaxKind = FunctionSyntaxKind::Getter;
   } else if (fun->isSetter()) {
     syntaxKind = FunctionSyntaxKind::Setter;
   } else if (fun->isArrow()) {
     syntaxKind = FunctionSyntaxKind::Arrow;
@@ -6973,17 +6977,17 @@ GeneralParser<ParseHandler, Unit>::class
 
   // Push a ParseContext::ClassStatement to keep track of the constructor
   // funbox.
   ParseContext::ClassStatement classStmt(pc_);
 
   NameNodeType innerName;
   Node nameNode = null();
   Node classHeritage = null();
-  Node classBlock = null();
+  LexicalScopeNodeType classBlock = null();
   uint32_t classEndOffset;
   {
     // A named class creates a new lexical scope with a const binding of the
     // class name for the "inner name".
     ParseContext::Statement innerScopeStmt(pc_, StatementKind::Block);
     ParseContext::Scope innerScope(this);
     if (!innerScope.init(pc_)) {
       return null();
@@ -7166,16 +7170,18 @@ GeneralParser<ParseHandler, Unit>::synth
       handler_.newList(ParseNodeKind::ParamsBody, synthesizedBodyPos);
   if (!argsbody) {
     return null();
   }
   handler_.setFunctionFormalParametersAndBody(funNode, argsbody);
   funbox->function()->setArgCount(0);
   tokenStream.setFunctionStart(funbox);
 
+  pc_->functionScope().useAsVarScope(pc_);
+
   // Push a LexicalScope on to the stack.
   ParseContext::Scope lexicalScope(this);
   if (!lexicalScope.init(pc_)) {
     return null();
   }
 
   auto stmtList = handler_.newStatementList(synthesizedBodyPos);
   if (!stmtList) {