Bug 1530832 - Refactor classDefinition and replace Maybe::reset() with a block. r=jorendorff
authorAshley Hauck <khyperia@mozilla.com>
Tue, 05 Mar 2019 13:42:29 +0000
changeset 520254 c9301ae941a18690438f351eee32c709bfd95ec2
parent 520253 9f4cf036e59185886c556c1db10625fda20b5c51
child 520255 84c79296c4ac7442815d6b5841910a3c51b74bd1
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1530832
milestone67.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 1530832 - Refactor classDefinition and replace Maybe::reset() with a block. r=jorendorff Differential Revision: https://phabricator.services.mozilla.com/D21270
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -6730,16 +6730,232 @@ static AccessorType ToAccessorType(Prope
     case PropertyType::DerivedConstructor:
       return AccessorType::None;
     default:
       MOZ_CRASH("unexpected property type");
   }
 }
 
 template <class ParseHandler, typename Unit>
+bool GeneralParser<ParseHandler, Unit>::classMember(
+    YieldHandling yieldHandling, DefaultHandling defaultHandling,
+    const ParseContext::ClassStatement& classStmt, HandlePropertyName className,
+    uint32_t classStartOffset, bool hasHeritage,
+    size_t& numFieldsWithInitializers, ListNodeType& classMembers, bool* done) {
+  *done = false;
+
+  TokenKind tt;
+  if (!tokenStream.getToken(&tt)) {
+    return false;
+  }
+  if (tt == TokenKind::RightCurly) {
+    *done = true;
+    return true;
+  }
+
+  if (tt == TokenKind::Semi) {
+    return true;
+  }
+
+  bool isStatic = false;
+  if (tt == TokenKind::Static) {
+    if (!tokenStream.peekToken(&tt)) {
+      return false;
+    }
+    if (tt == TokenKind::RightCurly) {
+      tokenStream.consumeKnownToken(tt);
+      error(JSMSG_UNEXPECTED_TOKEN, "property name", TokenKindToDesc(tt));
+      return false;
+    }
+
+    if (tt != TokenKind::LeftParen) {
+      isStatic = true;
+    } else {
+      anyChars.ungetToken();
+    }
+  } else {
+    anyChars.ungetToken();
+  }
+
+  uint32_t propNameOffset;
+  if (!tokenStream.peekOffset(&propNameOffset)) {
+    return false;
+  }
+
+  RootedAtom propAtom(cx_);
+  PropertyType propType;
+  Node propName = propertyName(yieldHandling, PropertyNameInClass,
+                               /* maybeDecl = */ Nothing(), classMembers,
+                               &propType, &propAtom);
+  if (!propName) {
+    return false;
+  }
+
+  if (propType == PropertyType::Field) {
+    // TODO(khyperia): Delete the two lines below once fields are fully
+    // supported in the backend. We can't fail in BytecodeCompiler because of
+    // lazy parsing.
+    errorAt(propNameOffset, JSMSG_FIELDS_NOT_SUPPORTED);
+    return false;
+
+    if (isStatic) {
+      errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
+      return false;
+    }
+    if (!tokenStream.getToken(&tt)) {
+      return false;
+    }
+
+    FunctionNodeType initializer = null();
+    if (tt == TokenKind::Assign) {
+      initializer = fieldInitializer(yieldHandling, propAtom);
+      if (!initializer) {
+        return false;
+      }
+
+      numFieldsWithInitializers++;
+      if (!tokenStream.getToken(&tt)) {
+        return false;
+      }
+    }
+
+    // TODO(khyperia): Implement ASI
+    if (tt != TokenKind::Semi) {
+      error(JSMSG_MISSING_SEMI_FIELD);
+      return false;
+    }
+
+    return handler_.addClassFieldDefinition(classMembers, propName,
+                                            initializer);
+  }
+
+  if (propType != PropertyType::Getter && propType != PropertyType::Setter &&
+      propType != PropertyType::Method &&
+      propType != PropertyType::GeneratorMethod &&
+      propType != PropertyType::AsyncMethod &&
+      propType != PropertyType::AsyncGeneratorMethod) {
+    errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
+    return false;
+  }
+
+  bool isConstructor = !isStatic && propAtom == cx_->names().constructor;
+  if (isConstructor) {
+    if (propType != PropertyType::Method) {
+      errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
+      return false;
+    }
+    if (classStmt.constructorBox) {
+      errorAt(propNameOffset, JSMSG_DUPLICATE_PROPERTY, "constructor");
+      return false;
+    }
+    propType = hasHeritage ? PropertyType::DerivedConstructor
+                           : PropertyType::Constructor;
+  } else if (isStatic && propAtom == cx_->names().prototype) {
+    errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
+    return false;
+  }
+
+  RootedAtom funName(cx_);
+  switch (propType) {
+    case PropertyType::Getter:
+    case PropertyType::Setter:
+      if (!anyChars.isCurrentTokenType(TokenKind::RightBracket)) {
+        funName = prefixAccessorName(propType, propAtom);
+        if (!funName) {
+          return false;
+        }
+      }
+      break;
+    case PropertyType::Constructor:
+    case PropertyType::DerivedConstructor:
+      funName = className;
+      break;
+    default:
+      if (!anyChars.isCurrentTokenType(TokenKind::RightBracket)) {
+        funName = propAtom;
+      }
+  }
+
+  // Calling toString on constructors need to return the source text for
+  // the entire class. The end offset is unknown at this point in
+  // parsing and will be amended when class parsing finishes below.
+  FunctionNodeType funNode = methodDefinition(
+      isConstructor ? classStartOffset : propNameOffset, propType, funName);
+  if (!funNode) {
+    return false;
+  }
+
+  AccessorType atype = ToAccessorType(propType);
+  return handler_.addClassMethodDefinition(classMembers, propName, funNode,
+                                           atype, isStatic);
+}
+
+template <class ParseHandler, typename Unit>
+bool GeneralParser<ParseHandler, Unit>::finishClassConstructor(
+    const ParseContext::ClassStatement& classStmt, HandlePropertyName className,
+    uint32_t classStartOffset, uint32_t classEndOffset,
+    size_t numFieldsWithInitializers, ListNodeType& classMembers) {
+  // Fields cannot re-use the constructor obtained via JSOP_CLASSCONSTRUCTOR or
+  // JSOP_DERIVEDCONSTRUCTOR due to needing to emit calls to the field
+  // initializers in the constructor. So, synthesize a new one.
+  if (classStmt.constructorBox == nullptr && numFieldsWithInitializers > 0) {
+    // synthesizeConstructor assigns to classStmt.constructorBox
+    FunctionNodeType synthesizedCtor =
+        synthesizeConstructor(className, classStartOffset);
+    if (!synthesizedCtor) {
+      return false;
+    }
+
+    MOZ_ASSERT(classStmt.constructorBox != nullptr);
+
+    // Note: the *function* has the name of the class, but the *property*
+    // containing the function has the name "constructor"
+    Node constructorNameNode =
+        handler_.newObjectLiteralPropertyName(cx_->names().constructor, pos());
+    if (!constructorNameNode) {
+      return false;
+    }
+
+    if (!handler_.addClassMethodDefinition(classMembers, constructorNameNode,
+                                           synthesizedCtor, AccessorType::None,
+                                           /* isStatic = */ false)) {
+      return false;
+    }
+  }
+
+  if (FunctionBox* ctorbox = classStmt.constructorBox) {
+    // Amend the toStringEnd offset for the constructor now that we've
+    // finished parsing the class.
+    ctorbox->toStringEnd = classEndOffset;
+
+    if (numFieldsWithInitializers > 0) {
+      // Field initialization need access to `this`.
+      ctorbox->setHasThisBinding();
+    }
+
+    // Set the same information, but on the lazyScript.
+    if (ctorbox->function()->isInterpretedLazy()) {
+      ctorbox->function()->lazyScript()->setToStringEnd(classEndOffset);
+
+      if (numFieldsWithInitializers > 0) {
+        ctorbox->function()->lazyScript()->setHasThisBinding();
+      }
+
+      // Field initializers can be retrieved if the class and constructor are
+      // being compiled at the same time, but we need to stash the field
+      // information if the constructor is being compiled lazily.
+      FieldInitializers fieldInfo(numFieldsWithInitializers);
+      ctorbox->function()->lazyScript()->setFieldInitializers(fieldInfo);
+    }
+  }
+
+  return true;
+}
+
+template <class ParseHandler, typename Unit>
 typename ParseHandler::ClassNodeType
 GeneralParser<ParseHandler, Unit>::classDefinition(
     YieldHandling yieldHandling, ClassContext classContext,
     DefaultHandling defaultHandling) {
   MOZ_ASSERT(anyChars.isCurrentTokenType(TokenKind::Class));
 
   uint32_t classStartOffset = pos().begin;
   bool savedStrictness = setLocalStrictMode(true);
@@ -6764,294 +6980,103 @@ GeneralParser<ParseHandler, Unit>::class
       error(JSMSG_UNNAMED_CLASS_STMT);
       return null();
     }
   } else {
     // Make sure to put it back, whatever it was
     anyChars.ungetToken();
   }
 
+  // Because the binding definitions keep track of their blockId, we need to
+  // create at least the inner binding later. Keep track of the name's
+  // position in order to provide it for the nodes created later.
+  TokenPos namePos = pos();
+
   // Push a ParseContext::ClassStatement to keep track of the constructor
   // funbox.
   ParseContext::ClassStatement classStmt(pc_);
 
-  RootedAtom propAtom(cx_);
-
-  // A named class creates a new lexical scope with a const binding of the
-  // class name for the "inner name".
-  Maybe<ParseContext::Statement> innerScopeStmt;
-  Maybe<ParseContext::Scope> innerScope;
-  if (className) {
-    innerScopeStmt.emplace(pc_, StatementKind::Block);
-    innerScope.emplace(this);
-    if (!innerScope->init(pc_)) {
-      return null();
-    }
-  }
-
-  // Because the binding definitions keep track of their blockId, we need to
-  // create at least the inner binding later. Keep track of the name's position
-  // in order to provide it for the nodes created later.
-  TokenPos namePos = pos();
-
+  NameNodeType innerName;
+  Node nameNode = null();
   Node classHeritage = null();
-  bool hasHeritage;
-  if (!tokenStream.matchToken(&hasHeritage, TokenKind::Extends)) {
-    return null();
-  }
-  if (hasHeritage) {
-    if (!tokenStream.getToken(&tt)) {
-      return null();
-    }
-    classHeritage = memberExpr(yieldHandling, TripledotProhibited, tt);
-    if (!classHeritage) {
-      return null();
-    }
-  }
-
-  if (!mustMatchToken(TokenKind::LeftCurly, JSMSG_CURLY_BEFORE_CLASS)) {
-    return null();
-  }
-
-  ListNodeType classMembers = handler_.newClassMemberList(pos().begin);
-  if (!classMembers) {
-    return null();
-  }
-
-  Maybe<DeclarationKind> declKind = Nothing();
-  size_t numFieldsWithInitializers = 0;
-  for (;;) {
-    TokenKind tt;
-    if (!tokenStream.getToken(&tt)) {
-      return null();
-    }
-    if (tt == TokenKind::RightCurly) {
-      break;
-    }
-
-    if (tt == TokenKind::Semi) {
-      continue;
-    }
-
-    bool isStatic = false;
-    if (tt == TokenKind::Static) {
-      if (!tokenStream.peekToken(&tt)) {
-        return null();
-      }
-      if (tt == TokenKind::RightCurly) {
-        tokenStream.consumeKnownToken(tt);
-        error(JSMSG_UNEXPECTED_TOKEN, "property name", TokenKindToDesc(tt));
-        return null();
-      }
-
-      if (tt != TokenKind::LeftParen) {
-        isStatic = true;
-      } else {
-        anyChars.ungetToken();
-      }
-    } else {
-      anyChars.ungetToken();
-    }
-
-    uint32_t propNameOffset;
-    if (!tokenStream.peekOffset(&propNameOffset)) {
-      return null();
-    }
-
-    PropertyType propType;
-    Node propName = propertyName(yieldHandling, PropertyNameInClass, declKind,
-                                 classMembers, &propType, &propAtom);
-    if (!propName) {
-      return null();
-    }
-
-    if (propType == PropertyType::Field) {
-      // TODO(khyperia): Delete the two lines below once fields are fully
-      // supported in the backend. We can't fail in BytecodeCompiler because of
-      // lazy parsing.
-      errorAt(propNameOffset, JSMSG_FIELDS_NOT_SUPPORTED);
-      return null();
-
-      if (isStatic) {
-        errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
-        return null();
-      }
+  Node 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();
+    }
+
+    bool hasHeritage;
+    if (!tokenStream.matchToken(&hasHeritage, TokenKind::Extends)) {
+      return null();
+    }
+    if (hasHeritage) {
       if (!tokenStream.getToken(&tt)) {
         return null();
       }
-
-      FunctionNodeType initializer = null();
-      if (tt == TokenKind::Assign) {
-        initializer = fieldInitializer(yieldHandling, propAtom);
-        if (!initializer) {
-          return null();
-        }
-
-        numFieldsWithInitializers++;
-        if (!tokenStream.getToken(&tt)) {
-          return null();
-        }
-      }
-
-      // TODO(khyperia): Implement ASI
-      if (tt != TokenKind::Semi) {
-        error(JSMSG_MISSING_SEMI_FIELD);
-        return null();
-      }
-
-      if (!handler_.addClassFieldDefinition(classMembers, propName,
-                                            initializer)) {
-        return null();
-      }
-
-      continue;
-    }
-
-    if (propType != PropertyType::Getter && propType != PropertyType::Setter &&
-        propType != PropertyType::Method &&
-        propType != PropertyType::GeneratorMethod &&
-        propType != PropertyType::AsyncMethod &&
-        propType != PropertyType::AsyncGeneratorMethod) {
-      errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
-      return null();
-    }
-
-    bool isConstructor = !isStatic && propAtom == cx_->names().constructor;
-    if (isConstructor) {
-      if (propType != PropertyType::Method) {
-        errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
-        return null();
-      }
-      if (classStmt.constructorBox) {
-        errorAt(propNameOffset, JSMSG_DUPLICATE_PROPERTY, "constructor");
-        return null();
-      }
-      propType = hasHeritage ? PropertyType::DerivedConstructor
-                             : PropertyType::Constructor;
-    } else if (isStatic && propAtom == cx_->names().prototype) {
-      errorAt(propNameOffset, JSMSG_BAD_METHOD_DEF);
-      return null();
-    }
-
-    RootedAtom funName(cx_);
-    switch (propType) {
-      case PropertyType::Getter:
-      case PropertyType::Setter:
-        if (!anyChars.isCurrentTokenType(TokenKind::RightBracket)) {
-          funName = prefixAccessorName(propType, propAtom);
-          if (!funName) {
-            return null();
-          }
-        }
-        break;
-      case PropertyType::Constructor:
-      case PropertyType::DerivedConstructor:
-        funName = className;
+      classHeritage = memberExpr(yieldHandling, TripledotProhibited, tt);
+      if (!classHeritage) {
+        return null();
+      }
+    }
+
+    if (!mustMatchToken(TokenKind::LeftCurly, JSMSG_CURLY_BEFORE_CLASS)) {
+      return null();
+    }
+
+    ListNodeType classMembers = handler_.newClassMemberList(pos().begin);
+    if (!classMembers) {
+      return null();
+    }
+
+    size_t numFieldsWithInitializers = 0;
+    for (;;) {
+      bool done;
+      if (!classMember(yieldHandling, defaultHandling, classStmt, className,
+                       classStartOffset, hasHeritage, numFieldsWithInitializers,
+                       classMembers, &done)) {
+        return null();
+      }
+      if (done) {
         break;
-      default:
-        if (!anyChars.isCurrentTokenType(TokenKind::RightBracket)) {
-          funName = propAtom;
-        }
-    }
-
-    // Calling toString on constructors need to return the source text for
-    // the entire class. The end offset is unknown at this point in
-    // parsing and will be amended when class parsing finishes below.
-    FunctionNodeType funNode = methodDefinition(
-        isConstructor ? classStartOffset : propNameOffset, propType, funName);
-    if (!funNode) {
-      return null();
-    }
-
-    AccessorType atype = ToAccessorType(propType);
-    if (!handler_.addClassMethodDefinition(classMembers, propName, funNode,
-                                           atype, isStatic)) {
-      return null();
-    }
-  }
-
-  // Fields cannot re-use the constructor obtained via JSOP_CLASSCONSTRUCTOR or
-  // JSOP_DERIVEDCONSTRUCTOR due to needing to emit calls to the field
-  // initializers in the constructor. So, synthesize a new one.
-  if (classStmt.constructorBox == nullptr && numFieldsWithInitializers > 0) {
-    // synthesizeConstructor assigns to classStmt.constructorBox
-    FunctionNodeType synthesizedCtor =
-        synthesizeConstructor(className, classStartOffset);
-    if (!synthesizedCtor) {
-      return null();
-    }
-
-    MOZ_ASSERT(classStmt.constructorBox != nullptr);
-
-    // Note: the *function* has the name of the class, but the *property*
-    // containing the function has the name "constructor"
-    Node constructorNameNode =
-        handler_.newObjectLiteralPropertyName(cx_->names().constructor, pos());
-    if (!constructorNameNode) {
-      return null();
-    }
-
-    if (!handler_.addClassMethodDefinition(classMembers, constructorNameNode,
-                                           synthesizedCtor, AccessorType::None,
-                                           /* isStatic = */ false)) {
-      return null();
-    }
-  }
-
-  uint32_t classEndOffset = pos().end;
-  if (FunctionBox* ctorbox = classStmt.constructorBox) {
-    // Amend the toStringEnd offset for the constructor now that we've
-    // finished parsing the class.
-    ctorbox->toStringEnd = classEndOffset;
-
-    if (numFieldsWithInitializers > 0) {
-      // Field initialization need access to `this`.
-      ctorbox->setHasThisBinding();
-    }
-
-    // Set the same information, but on the lazyScript.
-    if (ctorbox->function()->isInterpretedLazy()) {
-      ctorbox->function()->lazyScript()->setToStringEnd(classEndOffset);
-
-      if (numFieldsWithInitializers > 0) {
-        ctorbox->function()->lazyScript()->setHasThisBinding();
-      }
-
-      // Field initializers can be retrieved if the class and constructor are
-      // being compiled at the same time, but we need to stash the field
-      // information if the constructor is being compiled lazily.
-      FieldInitializers fieldInfo(numFieldsWithInitializers);
-      ctorbox->function()->lazyScript()->setFieldInitializers(fieldInfo);
-    }
-  }
-
-  Node nameNode = null();
-  Node membersOrBlock = classMembers;
-  if (className) {
-    // The inner name is immutable.
-    if (!noteDeclaredName(className, DeclarationKind::Const, namePos)) {
-      return null();
-    }
-
-    NameNodeType innerName = newName(className, namePos);
-    if (!innerName) {
-      return null();
-    }
-
-    Node classBlock = finishLexicalScope(*innerScope, classMembers);
+      }
+    }
+
+    classEndOffset = pos().end;
+    if (!finishClassConstructor(classStmt, className, classStartOffset,
+                                classEndOffset, numFieldsWithInitializers,
+                                classMembers)) {
+      return null();
+    }
+
+    if (className) {
+      // The inner name is immutable.
+      if (!noteDeclaredName(className, DeclarationKind::Const, namePos)) {
+        return null();
+      }
+
+      innerName = newName(className, namePos);
+      if (!innerName) {
+        return null();
+      }
+    }
+
+    classBlock = finishLexicalScope(innerScope, classMembers);
     if (!classBlock) {
       return null();
     }
 
-    membersOrBlock = classBlock;
-
     // Pop the inner scope.
-    innerScope.reset();
-    innerScopeStmt.reset();
-
+  }
+
+  if (className) {
     NameNodeType outerName = null();
     if (classContext == ClassStatement) {
       // The outer name is mutable.
       if (!noteDeclaredName(className, DeclarationKind::Class, namePos)) {
         return null();
       }
 
       outerName = newName(className, namePos);
@@ -7063,17 +7088,17 @@ GeneralParser<ParseHandler, Unit>::class
     nameNode = handler_.newClassNames(outerName, innerName, namePos);
     if (!nameNode) {
       return null();
     }
   }
 
   MOZ_ALWAYS_TRUE(setLocalStrictMode(savedStrictness));
 
-  return handler_.newClass(nameNode, classHeritage, membersOrBlock,
+  return handler_.newClass(nameNode, classHeritage, classBlock,
                            TokenPos(classStartOffset, classEndOffset));
 }
 
 template <class ParseHandler, typename Unit>
 typename ParseHandler::FunctionNodeType
 GeneralParser<ParseHandler, Unit>::synthesizeConstructor(
     HandleAtom className, uint32_t classNameOffset) {
   FunctionSyntaxKind functionSyntaxKind = FunctionSyntaxKind::ClassConstructor;
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -1298,16 +1298,28 @@ class MOZ_STACK_CLASS GeneralParser : pu
   inline bool checkExportedNameForFunction(FunctionNodeType funNode);
   inline bool checkExportedNameForClass(ClassNodeType classNode);
   inline bool checkExportedNameForClause(NameNodeType nameNode);
 
   enum ClassContext { ClassStatement, ClassExpression };
   ClassNodeType classDefinition(YieldHandling yieldHandling,
                                 ClassContext classContext,
                                 DefaultHandling defaultHandling);
+  MOZ_MUST_USE bool classMember(YieldHandling yieldHandling,
+                                DefaultHandling defaultHandling,
+                                const ParseContext::ClassStatement& classStmt,
+                                HandlePropertyName className,
+                                uint32_t classStartOffset, bool hasHeritage,
+                                size_t& numFieldsWithInitializers,
+                                ListNodeType& classMembers, bool* done);
+  MOZ_MUST_USE bool finishClassConstructor(
+      const ParseContext::ClassStatement& classStmt,
+      HandlePropertyName className, uint32_t classStartOffset,
+      uint32_t classEndOffset, size_t numFieldsWithInitializers,
+      ListNodeType& classMembers);
 
   FunctionNodeType fieldInitializer(YieldHandling yieldHandling,
                                     HandleAtom atom);
   FunctionNodeType synthesizeConstructor(HandleAtom className,
                                          uint32_t classNameOffset);
 
   bool checkBindingIdentifier(PropertyName* ident, uint32_t offset,
                               YieldHandling yieldHandling,