[preparser] Split GetSymbol into GetSymbol and GetIdentifier

GetSymbol previously always internalized the underlying string, even if the
preparser does not need it. The most common case where this isn't needed is
property name parsing. This seems to speed up preparsing quite a bit.

For future reference: Property names in object literals still are needed due to
various checks (e.g., get 'constructor', duplicate __proto__, ...); as well as
cover grammar parsing (property names can turn into variable references). If we
turn all strings that the preparser needs to identify back into contextual
keywords we may be able to avoid the former.

Change-Id: I549e4600053de3136ca08d4915cc04db36d66a89
Reviewed-on: https://chromium-review.googlesource.com/c/1488764Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59866}

src/parsing/parser-base.h

@@ -995,6 +995,8 @@ class ParserBase {
   // mode.
   IdentifierT ParseNonRestrictedIdentifier();
 
+  // This method should be used to ambiguously parse property names that can
+  // become destructuring identifiers.
   V8_INLINE IdentifierT ParsePropertyName();
 
   ExpressionT ParsePropertyOrPrivatePropertyName();
@@ -1482,7 +1484,7 @@ typename ParserBase<Impl>::IdentifierT
 ParserBase<Impl>::ParseAndClassifyIdentifier(Token::Value next) {
   DCHECK_EQ(scanner()->current_token(), next);
   if (V8_LIKELY(IsInRange(next, Token::IDENTIFIER, Token::ASYNC))) {
-    IdentifierT name = impl()->GetSymbol();
+    IdentifierT name = impl()->GetIdentifier();
     if (V8_UNLIKELY(impl()->IsArguments(name) &&
                     scope()->ShouldBanArguments())) {
       ReportMessage(MessageTemplate::kArgumentsDisallowedInInitializer);
@@ -1500,13 +1502,13 @@ ParserBase<Impl>::ParseAndClassifyIdentifier(Token::Value next) {
   if (next == Token::AWAIT) {
     expression_scope()->RecordAsyncArrowParametersError(
         scanner()->location(), MessageTemplate::kAwaitBindingIdentifier);
-    return impl()->GetSymbol();
+    return impl()->GetIdentifier();
   }
 
   DCHECK(Token::IsStrictReservedWord(next));
   expression_scope()->RecordStrictModeParameterError(
       scanner()->location(), MessageTemplate::kUnexpectedStrictReserved);
-  return impl()->GetSymbol();
+  return impl()->GetIdentifier();
 }
 
 template <class Impl>
@@ -1521,7 +1523,7 @@ typename ParserBase<Impl>::IdentifierT ParserBase<Impl>::ParseIdentifier(
     return impl()->EmptyIdentifierString();
   }
 
-  return impl()->GetSymbol();
+  return impl()->GetIdentifier();
 }
 
 template <typename Impl>
@@ -1541,7 +1543,7 @@ ParserBase<Impl>::ParseNonRestrictedIdentifier() {
 template <typename Impl>
 typename ParserBase<Impl>::IdentifierT ParserBase<Impl>::ParsePropertyName() {
   Token::Value next = Next();
-  if (V8_LIKELY(Token::IsPropertyName(next))) return impl()->GetSymbol();
+  if (V8_LIKELY(Token::IsPropertyName(next))) return impl()->GetIdentifier();
 
   ReportUnexpectedToken(next);
   return impl()->EmptyIdentifierString();
@@ -1574,7 +1576,7 @@ ParserBase<Impl>::ParsePropertyOrPrivatePropertyName() {
       ReportMessage(MessageTemplate::kInvalidPrivateFieldResolution);
     }
 
-    name = impl()->GetSymbol();
+    name = impl()->GetIdentifier();
     key = impl()->ExpressionFromIdentifier(name, pos, InferName::kNo);
   } else {
     ReportUnexpectedToken(next);
@@ -1964,7 +1966,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseProperty(
     Token::Value token = peek();
     if ((token != Token::MUL && prop_info->ParsePropertyKindFromToken(token)) ||
         scanner()->HasLineTerminatorBeforeNext()) {
-      prop_info->name = impl()->GetSymbol();
+      prop_info->name = impl()->GetIdentifier();
       impl()->PushLiteralName(prop_info->name);
       return factory()->NewStringLiteral(prop_info->name, position());
     }
@@ -1984,7 +1986,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseProperty(
       IsInRange(peek(), Token::GET, Token::SET)) {
     Token::Value token = Next();
     if (prop_info->ParsePropertyKindFromToken(peek())) {
-      prop_info->name = impl()->GetSymbol();
+      prop_info->name = impl()->GetIdentifier();
       impl()->PushLiteralName(prop_info->name);
       return factory()->NewStringLiteral(prop_info->name, position());
     }
@@ -2019,7 +2021,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseProperty(
       if (prop_info->kind == ParsePropertyKind::kNotSet) {
         prop_info->ParsePropertyKindFromToken(peek());
       }
-      prop_info->name = impl()->GetSymbol();
+      prop_info->name = impl()->GetIdentifier();
       if (V8_UNLIKELY(prop_info->position ==
                       PropertyPosition::kObjectLiteral)) {
         ReportUnexpectedToken(Token::PRIVATE_NAME);
@@ -2037,7 +2039,7 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseProperty(
 
     case Token::STRING:
       Consume(Token::STRING);
-      prop_info->name = impl()->GetSymbol();
+      prop_info->name = impl()->GetIdentifier();
       is_array_index = impl()->IsArrayIndex(prop_info->name, &index);
       break;
 
@@ -2130,13 +2132,13 @@ ParserBase<Impl>::ParseClassPropertyDefinition(ClassInfo* class_info,
     if (peek() == Token::LPAREN) {
       prop_info->kind = ParsePropertyKind::kMethod;
       // TODO(bakkot) specialize on 'static'
-      prop_info->name = impl()->GetSymbol();
+      prop_info->name = impl()->GetIdentifier();
       name_expression =
           factory()->NewStringLiteral(prop_info->name, position());
     } else if (peek() == Token::ASSIGN || peek() == Token::SEMICOLON ||
                peek() == Token::RBRACE) {
       // TODO(bakkot) specialize on 'static'
-      prop_info->name = impl()->GetSymbol();
+      prop_info->name = impl()->GetIdentifier();
       name_expression =
           factory()->NewStringLiteral(prop_info->name, position());
     } else {

src/parsing/parser.h

@@ -777,6 +777,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
     return result;
   }
 
+  V8_INLINE const AstRawString* GetIdentifier() const { return GetSymbol(); }
+
   V8_INLINE const AstRawString* GetNextSymbol() const {
     return scanner()->NextSymbol(ast_value_factory());
   }

src/parsing/preparser.cc

@@ -21,9 +21,9 @@ namespace internal {
 
 namespace {
 
-PreParserIdentifier GetSymbolHelper(Scanner* scanner,
-                                    const AstRawString* string,
-                                    AstValueFactory* avf) {
+PreParserIdentifier GetIdentifierHelper(Scanner* scanner,
+                                        const AstRawString* string,
+                                        AstValueFactory* avf) {
   // These symbols require slightly different treatement:
   // - regular keywords (async, await, etc.; treated in 1st switch.)
   // - 'contextual' keywords (and may contain escaped; treated in 2nd switch.)
@@ -58,10 +58,10 @@ PreParserIdentifier GetSymbolHelper(Scanner* scanner,
 
 }  // unnamed namespace
 
-PreParserIdentifier PreParser::GetSymbol() const {
+PreParserIdentifier PreParser::GetIdentifier() const {
   const AstRawString* result = scanner()->CurrentSymbol(ast_value_factory());
   PreParserIdentifier symbol =
-      GetSymbolHelper(scanner(), result, ast_value_factory());
+      GetIdentifierHelper(scanner(), result, ast_value_factory());
   DCHECK_NOT_NULL(result);
   symbol.string_ = result;
   return symbol;

src/parsing/preparser.h

@@ -1527,7 +1527,11 @@ class PreParser : public ParserBase<PreParser> {
   }
 
   // Producing data during the recursive descent.
-  PreParserIdentifier GetSymbol() const;
+  PreParserIdentifier GetSymbol() const {
+    return PreParserIdentifier::Default();
+  }
+
+  PreParserIdentifier GetIdentifier() const;
 
   V8_INLINE PreParserIdentifier GetNextSymbol() const {
     return PreParserIdentifier::Default();