Bug 1529772 - Part 2: Factor out PropertyName parsing from Parser::propertyName(). r=khyperia
authorJason Orendorff <jorendorff@mozilla.com>
Mon, 15 Apr 2019 20:54:48 +0000
changeset 469565 0f4a343adc59
parent 469564 7d26d6d473cf
child 469566 e771e63200c6
push id35874
push userccoroiu@mozilla.com
push dateTue, 16 Apr 2019 04:04:58 +0000
treeherdermozilla-central@be3f40425b52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhyperia
bugs1529772
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 1529772 - Part 2: Factor out PropertyName parsing from Parser::propertyName(). r=khyperia The existing propertyName() method parses a little more than just PropertyNames; this patch renames it, to clarify that, but the behavior is unchanged. There was a lot of redundancy in the implementation of getters and setters, so this deletes a bunch of code. Differential Revision: https://phabricator.services.mozilla.com/D26036
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -3767,18 +3767,19 @@ GeneralParser<ParseHandler, Unit>::objec
 
       if (!handler_.addSpreadProperty(literal, begin, inner)) {
         return null();
       }
     } else {
       TokenPos namePos = anyChars.nextToken().pos;
 
       PropertyType propType;
-      Node propName = propertyName(yieldHandling, PropertyNameInPattern,
-                                   declKind, literal, &propType, &propAtom);
+      Node propName =
+          propertyOrMethodName(yieldHandling, PropertyNameInPattern, declKind,
+                               literal, &propType, &propAtom);
       if (!propName) {
         return null();
       }
 
       if (propType == PropertyType::Normal) {
         // Handle e.g., |var {p: x} = o| and |var {p: x=0} = o|.
 
         if (!tokenStream.getToken(&tt, TokenStream::SlashIsRegExp)) {
@@ -6770,19 +6771,19 @@ bool GeneralParser<ParseHandler, Unit>::
 
   uint32_t propNameOffset;
   if (!tokenStream.peekOffset(&propNameOffset, TokenStream::SlashIsInvalid)) {
     return false;
   }
 
   RootedAtom propAtom(cx_);
   PropertyType propType;
-  Node propName = propertyName(yieldHandling, PropertyNameInClass,
-                               /* maybeDecl = */ Nothing(), classMembers,
-                               &propType, &propAtom);
+  Node propName = propertyOrMethodName(yieldHandling, PropertyNameInClass,
+                                       /* maybeDecl = */ Nothing(),
+                                       classMembers, &propType, &propAtom);
   if (!propName) {
     return false;
   }
 
   if (propType == PropertyType::Field) {
     if (!options().fieldsEnabledOption) {
       errorAt(propNameOffset, JSMSG_FIELDS_NOT_SUPPORTED);
       return false;
@@ -9637,16 +9638,73 @@ GeneralParser<ParseHandler, Unit>::array
   handler_.setEndPosition(literal, pos().end);
   return literal;
 }
 
 template <class ParseHandler, typename Unit>
 typename ParseHandler::Node GeneralParser<ParseHandler, Unit>::propertyName(
     YieldHandling yieldHandling, PropertyNameContext propertyNameContext,
     const Maybe<DeclarationKind>& maybeDecl, ListNodeType propList,
+    MutableHandleAtom propAtom) {
+  // PropertyName[Yield, Await]:
+  //   LiteralPropertyName
+  //   ComputedPropertyName[?Yield, ?Await]
+  //
+  // LiteralPropertyName:
+  //   IdentifierName
+  //   StringLiteral
+  //   NumericLiteral
+  TokenKind ltok = anyChars.currentToken().type;
+
+  propAtom.set(nullptr);
+  switch (ltok) {
+    case TokenKind::Number:
+      propAtom.set(NumberToAtom(cx_, anyChars.currentToken().number()));
+      if (!propAtom.get()) {
+        return null();
+      }
+      return newNumber(anyChars.currentToken());
+
+    case TokenKind::String: {
+      propAtom.set(anyChars.currentToken().atom());
+      uint32_t index;
+      if (propAtom->isIndex(&index)) {
+        return handler_.newNumber(index, NoDecimal, pos());
+      }
+      return stringLiteral();
+    }
+
+    case TokenKind::LeftBracket:
+      return computedPropertyName(yieldHandling, maybeDecl, propertyNameContext,
+                                  propList);
+
+    default: {
+      if (!TokenKindIsPossibleIdentifierName(ltok)) {
+        error(JSMSG_UNEXPECTED_TOKEN, "property name", TokenKindToDesc(ltok));
+        return null();
+      }
+
+      propAtom.set(anyChars.currentName());
+      return handler_.newObjectLiteralPropertyName(propAtom, pos());
+    }
+  }
+}
+
+// True if `kind` can be the first token of a PropertyName.
+static bool TokenKindCanStartPropertyName(TokenKind tt) {
+  return TokenKindIsPossibleIdentifierName(tt) || tt == TokenKind::String ||
+         tt == TokenKind::Number || tt == TokenKind::LeftBracket ||
+         tt == TokenKind::Mul;
+}
+
+template <class ParseHandler, typename Unit>
+typename ParseHandler::Node
+GeneralParser<ParseHandler, Unit>::propertyOrMethodName(
+    YieldHandling yieldHandling, PropertyNameContext propertyNameContext,
+    const Maybe<DeclarationKind>& maybeDecl, ListNodeType propList,
     PropertyType* propType, MutableHandleAtom propAtom) {
   // We're parsing an object literal, class, or destructuring pattern;
   // propertyNameContext tells which one. This method parses any of the
   // following, storing the corresponding PropertyType in `*propType` to tell
   // the caller what we parsed:
   //
   //     async [no LineTerminator here] PropertyName
   //                            ==> PropertyType::AsyncMethod
@@ -9664,213 +9722,108 @@ typename ParseHandler::Node GeneralParse
   //     `=` or `;`             ==> PropertyType::Field (classes only)
   //     `=`                    ==> PropertyType::CoverInitializedName
   //     `,` or `}`             ==> PropertyType::Shorthand
   //     `(`                    ==> PropertyType::Method
   //
   // The caller must check `*propType` and throw if whatever we parsed isn't
   // allowed here (for example, a getter in a destructuring pattern).
   //
-  // The nonterminal PropertyName comes from the standard:
-  //
-  //     PropertyName[Yield, Await]:
-  //       LiteralPropertyName
-  //       ComputedPropertyName[?Yield, ?Await]
-  //
-  //     LiteralPropertyName:
-  //       IdentifierName
-  //       StringLiteral
-  //       NumericLiteral
-  //
-  //     ComputedPropertyName[Yield, Await]:
-  //       [ AssignmentExpression[+In, ?Yield, ?Await] ]
-  //
   // This method does *not* match `static` (allowed in classes) or `...`
   // (allowed in object literals and patterns). The caller must take care of
   // those before calling this method.
 
   TokenKind ltok;
   if (!tokenStream.getToken(&ltok, TokenStream::SlashIsInvalid)) {
     return null();
   }
 
   MOZ_ASSERT(ltok != TokenKind::RightCurly,
              "caller should have handled TokenKind::RightCurly");
 
-  // Accept `async` and/or `*`, indicating an async or generator method.
+  // Accept `async` and/or `*`, indicating an async or generator method;
+  // or `get` or `set`, indicating an accessor.
   bool isGenerator = false;
   bool isAsync = false;
+  bool isGetter = false;
+  bool isSetter = false;
 
   if (ltok == TokenKind::Async) {
     // `async` is also a PropertyName by itself (it's a conditional keyword),
     // so peek at the next token to see if we're really looking at a method.
     TokenKind tt = TokenKind::Eof;
     if (!tokenStream.peekTokenSameLine(&tt)) {
       return null();
     }
-    if (tt == TokenKind::String || tt == TokenKind::Number ||
-        tt == TokenKind::LeftBracket || TokenKindIsPossibleIdentifierName(tt) ||
-        tt == TokenKind::Mul) {
+    if (TokenKindCanStartPropertyName(tt)) {
       isAsync = true;
       tokenStream.consumeKnownToken(tt);
       ltok = tt;
     }
   }
 
   if (ltok == TokenKind::Mul) {
     isGenerator = true;
     if (!tokenStream.getToken(&ltok)) {
       return null();
     }
   }
 
-  // Parse the PropertyName itself at last.
-  propAtom.set(nullptr);
-  Node propName;
-  switch (ltok) {
-    case TokenKind::Number:
-      propAtom.set(NumberToAtom(cx_, anyChars.currentToken().number()));
-      if (!propAtom.get()) {
-        return null();
-      }
-      propName = newNumber(anyChars.currentToken());
-      if (!propName) {
-        return null();
-      }
-      break;
-
-    case TokenKind::String: {
-      propAtom.set(anyChars.currentToken().atom());
-      uint32_t index;
-      if (propAtom->isIndex(&index)) {
-        propName = handler_.newNumber(index, NoDecimal, pos());
-        if (!propName) {
-          return null();
-        }
-        break;
-      }
-      propName = stringLiteral();
-      if (!propName) {
-        return null();
-      }
-      break;
-    }
-
-    case TokenKind::LeftBracket:
-      propName = computedPropertyName(yieldHandling, maybeDecl,
-                                      propertyNameContext, propList);
-      if (!propName) {
-        return null();
-      }
-      break;
-
-    default: {
-      if (!TokenKindIsPossibleIdentifierName(ltok)) {
-        error(JSMSG_UNEXPECTED_TOKEN, "property name", TokenKindToDesc(ltok));
-        return null();
-      }
-
-      propAtom.set(anyChars.currentName());
-
-      // Do not look for accessor syntax on generator or async methods.
-      if (isGenerator || isAsync ||
-          !(ltok == TokenKind::Get || ltok == TokenKind::Set)) {
-        propName = handler_.newObjectLiteralPropertyName(propAtom, pos());
-        if (!propName) {
-          return null();
-        }
-        break;
-      }
-
-      *propType =
-          ltok == TokenKind::Get ? PropertyType::Getter : PropertyType::Setter;
-
-      // We have parsed |get| or |set|. Look for an accessor property
-      // name next.
-      TokenKind tt;
-      if (!tokenStream.peekToken(&tt)) {
-        return null();
-      }
-      if (TokenKindIsPossibleIdentifierName(tt)) {
-        tokenStream.consumeKnownToken(tt);
-
-        propAtom.set(anyChars.currentName());
-        return handler_.newObjectLiteralPropertyName(propAtom, pos());
-      }
-      if (tt == TokenKind::String) {
-        tokenStream.consumeKnownToken(TokenKind::String);
-
-        propAtom.set(anyChars.currentToken().atom());
-
-        uint32_t index;
-        if (propAtom->isIndex(&index)) {
-          propAtom.set(NumberToAtom(cx_, index));
-          if (!propAtom.get()) {
-            return null();
-          }
-          return handler_.newNumber(index, NoDecimal, pos());
-        }
-        return stringLiteral();
-      }
-      if (tt == TokenKind::Number) {
-        tokenStream.consumeKnownToken(TokenKind::Number);
-
-        propAtom.set(NumberToAtom(cx_, anyChars.currentToken().number()));
-        if (!propAtom.get()) {
-          return null();
-        }
-        return newNumber(anyChars.currentToken());
-      }
-      if (tt == TokenKind::LeftBracket) {
-        tokenStream.consumeKnownToken(TokenKind::LeftBracket);
-
-        return computedPropertyName(yieldHandling, maybeDecl,
-                                    propertyNameContext, propList);
-      }
-
-      // Not an accessor property after all.
-      propName = handler_.newObjectLiteralPropertyName(propAtom.get(), pos());
-      if (!propName) {
-        return null();
-      }
-      break;
-    }
+  if (!isAsync && !isGenerator &&
+      (ltok == TokenKind::Get || ltok == TokenKind::Set)) {
+    // We have parsed |get| or |set|. Look for an accessor property
+    // name next.
+    TokenKind tt;
+    if (!tokenStream.peekToken(&tt)) {
+      return null();
+    }
+    if (TokenKindCanStartPropertyName(tt)) {
+      tokenStream.consumeKnownToken(tt);
+      isGetter = (ltok == TokenKind::Get);
+      isSetter = (ltok == TokenKind::Set);
+    }
+  }
+
+  Node propName = propertyName(yieldHandling, propertyNameContext, maybeDecl,
+                               propList, propAtom);
+  if (!propName) {
+    return null();
   }
 
   // Grab the next token following the property/method name.
   // (If this isn't a colon, we're going to either put it back or throw.)
   TokenKind tt;
   if (!tokenStream.getToken(&tt)) {
     return null();
   }
 
   if (tt == TokenKind::Colon) {
-    if (isGenerator || isAsync) {
+    if (isGenerator || isAsync || isGetter || isSetter) {
       error(JSMSG_BAD_PROP_ID);
       return null();
     }
     *propType = PropertyType::Normal;
     return propName;
   }
 
   if (propertyNameContext == PropertyNameInClass &&
       (tt == TokenKind::Semi || tt == TokenKind::Assign)) {
-    if (isGenerator || isAsync) {
+    if (isGenerator || isAsync || isGetter || isSetter) {
       error(JSMSG_BAD_PROP_ID);
       return null();
     }
     anyChars.ungetToken();
     *propType = PropertyType::Field;
     return propName;
   }
 
   if (TokenKindIsPossibleIdentifierName(ltok) &&
       (tt == TokenKind::Comma || tt == TokenKind::RightCurly ||
        tt == TokenKind::Assign)) {
-    if (isGenerator || isAsync) {
+    if (isGenerator || isAsync || isGetter || isSetter) {
       error(JSMSG_BAD_PROP_ID);
       return null();
     }
 
     anyChars.ungetToken();
     *propType = tt == TokenKind::Assign ? PropertyType::CoverInitializedName
                                         : PropertyType::Shorthand;
     return propName;
@@ -9880,16 +9833,20 @@ typename ParseHandler::Node GeneralParse
     anyChars.ungetToken();
 
     if (isGenerator && isAsync) {
       *propType = PropertyType::AsyncGeneratorMethod;
     } else if (isGenerator) {
       *propType = PropertyType::GeneratorMethod;
     } else if (isAsync) {
       *propType = PropertyType::AsyncMethod;
+    } else if (isGetter) {
+      *propType = PropertyType::Getter;
+    } else if (isSetter) {
+      *propType = PropertyType::Setter;
     } else {
       *propType = PropertyType::Method;
     }
     return propName;
   }
 
   error(JSMSG_COLON_AFTER_ID);
   return null();
@@ -9972,18 +9929,19 @@ GeneralParser<ParseHandler, Unit>::objec
       }
       if (!handler_.addSpreadProperty(literal, begin, inner)) {
         return null();
       }
     } else {
       TokenPos namePos = anyChars.nextToken().pos;
 
       PropertyType propType;
-      Node propName = propertyName(yieldHandling, PropertyNameInLiteral,
-                                   declKind, literal, &propType, &propAtom);
+      Node propName =
+          propertyOrMethodName(yieldHandling, PropertyNameInLiteral, declKind,
+                               literal, &propType, &propAtom);
       if (!propName) {
         return null();
       }
 
       if (propType == PropertyType::Normal) {
         TokenPos exprPos;
         if (!tokenStream.peekTokenPos(&exprPos, TokenStream::SlashIsRegExp)) {
           return null();
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -1354,18 +1354,22 @@ class MOZ_STACK_CLASS GeneralParser : pu
   enum PropertyNameContext {
     PropertyNameInLiteral,
     PropertyNameInPattern,
     PropertyNameInClass
   };
   Node propertyName(YieldHandling yieldHandling,
                     PropertyNameContext propertyNameContext,
                     const mozilla::Maybe<DeclarationKind>& maybeDecl,
-                    ListNodeType propList, PropertyType* propType,
-                    MutableHandleAtom propAtom);
+                    ListNodeType propList, MutableHandleAtom propAtom);
+  Node propertyOrMethodName(YieldHandling yieldHandling,
+                            PropertyNameContext propertyNameContext,
+                            const mozilla::Maybe<DeclarationKind>& maybeDecl,
+                            ListNodeType propList, PropertyType* propType,
+                            MutableHandleAtom propAtom);
   UnaryNodeType computedPropertyName(
       YieldHandling yieldHandling,
       const mozilla::Maybe<DeclarationKind>& maybeDecl,
       PropertyNameContext propertyNameContext, ListNodeType literal);
   ListNodeType arrayInitializer(YieldHandling yieldHandling,
                                 PossibleError* possibleError);
   inline RegExpLiteralType newRegExp();