[parser] Simplify parse-time function name inference for properties

Move the code to perform function name inference for properties into
parsing the properties themselves, instead of the containing object.

This allows us to avoid unnecessary calls when parsing shorthand
properties and methods and simplifies the logic in the remaining cases.

Also fixes an edge case bug: inferring the name of the getter in
`class { static get constructor(){} }`.

Review-Url: https://codereview.chromium.org/2313723005
Cr-Commit-Position: refs/heads/master@{#39222}

src/parsing/parser-base.h

@@ -1103,10 +1103,9 @@ class ParserBase {
   ExpressionT ParseObjectLiteral(bool* ok);
   ClassLiteralPropertyT ParseClassPropertyDefinition(
       ClassLiteralChecker* checker, bool has_extends, bool* is_computed_name,
-      bool* has_seen_constructor, IdentifierT* name, bool* ok);
+      bool* has_seen_constructor, bool* ok);
   ObjectLiteralPropertyT ParseObjectPropertyDefinition(
-      ObjectLiteralChecker* checker, bool* is_computed_name, IdentifierT* name,
-      bool* ok);
+      ObjectLiteralChecker* checker, bool* is_computed_name, bool* ok);
   ExpressionListT ParseArguments(Scanner::Location* first_spread_pos,
                                  bool maybe_arrow, bool* ok);
   ExpressionListT ParseArguments(Scanner::Location* first_spread_pos,
@@ -2009,7 +2008,7 @@ ParserBase<Impl>::ParseClassPropertyDefinition(ClassLiteralChecker* checker,
                                                bool has_extends,
                                                bool* is_computed_name,
                                                bool* has_seen_constructor,
-                                               IdentifierT* name, bool* ok) {
+                                               bool* ok) {
   DCHECK(has_seen_constructor != nullptr);
   bool is_get = false;
   bool is_set = false;
@@ -2020,22 +2019,23 @@ ParserBase<Impl>::ParseClassPropertyDefinition(ClassLiteralChecker* checker,
 
   Token::Value name_token = peek();
 
+  IdentifierT name = impl()->EmptyIdentifier();
   ExpressionT name_expression;
   if (name_token == Token::STATIC) {
     Consume(Token::STATIC);
     if (peek() == Token::LPAREN) {
       kind = PropertyKind::kMethodProperty;
-      *name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
-      name_expression = factory()->NewStringLiteral(*name, position());
+      name = impl()->GetSymbol();  // TODO(bakkot) specialize on 'static'
+      name_expression = factory()->NewStringLiteral(name, position());
     } else {
       is_static = true;
       name_expression = ParsePropertyName(
-          name, &kind, &is_generator, &is_get, &is_set, &is_async,
+          &name, &kind, &is_generator, &is_get, &is_set, &is_async,
           is_computed_name, CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
     }
   } else {
     name_expression = ParsePropertyName(
-        name, &kind, &is_generator, &is_get, &is_set, &is_async,
+        &name, &kind, &is_generator, &is_get, &is_set, &is_async,
         is_computed_name, CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
   }
 
@@ -2064,14 +2064,14 @@ ParserBase<Impl>::ParseClassPropertyDefinition(ClassLiteralChecker* checker,
                               : is_async ? FunctionKind::kAsyncConciseMethod
                                          : FunctionKind::kConciseMethod;
 
-      if (!is_static && impl()->IsConstructor(*name)) {
+      if (!is_static && impl()->IsConstructor(name)) {
         *has_seen_constructor = true;
         kind = has_extends ? FunctionKind::kSubclassConstructor
                            : FunctionKind::kBaseConstructor;
       }
 
       ExpressionT value = impl()->ParseFunctionLiteral(
-          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
+          name, scanner()->location(), kSkipFunctionNameCheck, kind,
           kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
           language_mode(), CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
 
@@ -2091,17 +2091,21 @@ ParserBase<Impl>::ParseClassPropertyDefinition(ClassLiteralChecker* checker,
         // Runtime_DefineAccessorPropertyUnchecked and since we can determine
         // this statically we can skip the extra runtime check.
         name_expression =
-            factory()->NewStringLiteral(*name, name_expression->position());
+            factory()->NewStringLiteral(name, name_expression->position());
       }
 
       FunctionKind kind = is_get ? FunctionKind::kGetterFunction
                                  : FunctionKind::kSetterFunction;
 
-      ExpressionT value = impl()->ParseFunctionLiteral(
-          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
+      FunctionLiteralT value = impl()->ParseFunctionLiteral(
+          name, scanner()->location(), kSkipFunctionNameCheck, kind,
           kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
           language_mode(), CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
 
+      if (!*is_computed_name) {
+        impl()->AddAccessorPrefixToFunctionName(is_get, value, name);
+      }
+
       return factory()->NewClassLiteralProperty(
           name_expression, value,
           is_get ? ClassLiteralProperty::GETTER : ClassLiteralProperty::SETTER,
@@ -2121,20 +2125,21 @@ template <typename Impl>
 typename ParserBase<Impl>::ObjectLiteralPropertyT
 ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
                                                 bool* is_computed_name,
-                                                IdentifierT* name, bool* ok) {
+                                                bool* ok) {
   bool is_get = false;
   bool is_set = false;
   bool is_generator = false;
   bool is_async = false;
   PropertyKind kind = PropertyKind::kNotSet;
 
+  IdentifierT name = impl()->EmptyIdentifier();
   Token::Value name_token = peek();
   int next_beg_pos = scanner()->peek_location().beg_pos;
   int next_end_pos = scanner()->peek_location().end_pos;
 
   ExpressionT name_expression = ParsePropertyName(
-      name, &kind, &is_generator, &is_get, &is_set, &is_async, is_computed_name,
-      CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
+      &name, &kind, &is_generator, &is_get, &is_set, &is_async,
+      is_computed_name, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
   switch (kind) {
     case PropertyKind::kValueProperty: {
@@ -2149,8 +2154,14 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
           true, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
       CheckDestructuringElement(value, beg_pos, scanner()->location().end_pos);
 
-      return factory()->NewObjectLiteralProperty(name_expression, value,
-                                                 *is_computed_name);
+      ObjectLiteralPropertyT result = factory()->NewObjectLiteralProperty(
+          name_expression, value, *is_computed_name);
+
+      if (!*is_computed_name) {
+        impl()->SetFunctionNameFromPropertyName(result, name);
+      }
+
+      return result;
     }
 
     case PropertyKind::kShorthandProperty: {
@@ -2178,7 +2189,7 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
             scanner()->location());
       }
 
-      if (impl()->IsEvalOrArguments(*name) && is_strict(language_mode())) {
+      if (impl()->IsEvalOrArguments(name) && is_strict(language_mode())) {
         classifier()->RecordBindingPatternError(
             scanner()->location(), MessageTemplate::kStrictEvalArguments);
       }
@@ -2194,7 +2205,7 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
             MessageTemplate::kAwaitBindingIdentifier);
       }
       ExpressionT lhs =
-          impl()->ExpressionFromIdentifier(*name, next_beg_pos, next_end_pos);
+          impl()->ExpressionFromIdentifier(name, next_beg_pos, next_end_pos);
       CheckDestructuringElement(lhs, next_beg_pos, next_end_pos);
 
       ExpressionT value;
@@ -2237,7 +2248,7 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
                                          : FunctionKind::kConciseMethod;
 
       ExpressionT value = impl()->ParseFunctionLiteral(
-          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
+          name, scanner()->location(), kSkipFunctionNameCheck, kind,
           kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
           language_mode(), CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
@@ -2259,17 +2270,21 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
         // Runtime_DefineAccessorPropertyUnchecked and since we can determine
         // this statically we can skip the extra runtime check.
         name_expression =
-            factory()->NewStringLiteral(*name, name_expression->position());
+            factory()->NewStringLiteral(name, name_expression->position());
       }
 
       FunctionKind kind = is_get ? FunctionKind::kGetterFunction
                                  : FunctionKind::kSetterFunction;
 
-      ExpressionT value = impl()->ParseFunctionLiteral(
-          *name, scanner()->location(), kSkipFunctionNameCheck, kind,
+      FunctionLiteralT value = impl()->ParseFunctionLiteral(
+          name, scanner()->location(), kSkipFunctionNameCheck, kind,
           kNoSourcePosition, FunctionLiteral::kAccessorOrMethod,
           language_mode(), CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
+      if (!*is_computed_name) {
+        impl()->AddAccessorPrefixToFunctionName(is_get, value, name);
+      }
+
       return factory()->NewObjectLiteralProperty(
           name_expression, value, is_get ? ObjectLiteralProperty::GETTER
                                          : ObjectLiteralProperty::SETTER,
@@ -2303,9 +2318,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseObjectLiteral(
     FuncNameInferrer::State fni_state(fni_);
 
     bool is_computed_name = false;
-    IdentifierT name = impl()->EmptyIdentifier();
-    ObjectLiteralPropertyT property = ParseObjectPropertyDefinition(
-        &checker, &is_computed_name, &name, CHECK_OK);
+    ObjectLiteralPropertyT property =
+        ParseObjectPropertyDefinition(&checker, &is_computed_name, CHECK_OK);
 
     if (is_computed_name) {
       has_computed_names = true;
@@ -2323,8 +2337,6 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseObjectLiteral(
     }
 
     if (fni_ != nullptr) fni_->Infer();
-
-    impl()->SetFunctionNameFromPropertyName(property, name);
   }
   Expect(Token::RBRACE, CHECK_OK);
 

src/parsing/parser.cc

@@ -4249,10 +4249,9 @@ Expression* Parser::ParseClassLiteral(const AstRawString* name,
     bool is_computed_name = false;  // Classes do not care about computed
                                     // property names here.
     ExpressionClassifier property_classifier(this);
-    const AstRawString* property_name = nullptr;
-    ClassLiteral::Property* property = ParseClassPropertyDefinition(
-        &checker, has_extends, &is_computed_name, &has_seen_constructor,
-        &property_name, CHECK_OK);
+    ClassLiteral::Property* property =
+        ParseClassPropertyDefinition(&checker, has_extends, &is_computed_name,
+                                     &has_seen_constructor, CHECK_OK);
     RewriteNonPattern(CHECK_OK);
     impl()->AccumulateFormalParameterContainmentErrors();
 
@@ -4267,10 +4266,6 @@ Expression* Parser::ParseClassLiteral(const AstRawString* name,
 
     DCHECK_NOT_NULL(fni_);
     fni_->Infer();
-
-    if (property_name != ast_value_factory()->constructor_string()) {
-      SetFunctionNameFromPropertyName(property, property_name);
-    }
   }
 
   Expect(Token::RBRACE, CHECK_OK);
@@ -5209,63 +5204,31 @@ void Parser::QueueNonPatternForRewriting(Expression* expr, bool* ok) {
   function_state_->AddNonPatternForRewriting(expr, ok);
 }
 
+void Parser::AddAccessorPrefixToFunctionName(bool is_get,
+                                             FunctionLiteral* function,
+                                             const AstRawString* name) {
+  DCHECK_NOT_NULL(name);
+  const AstRawString* prefix = is_get ? ast_value_factory()->get_space_string()
+                                      : ast_value_factory()->set_space_string();
+  function->set_raw_name(ast_value_factory()->NewConsString(prefix, name));
+}
+
 void Parser::SetFunctionNameFromPropertyName(ObjectLiteralProperty* property,
                                              const AstRawString* name) {
-  Expression* value = property->value();
+  DCHECK(property->kind() != ObjectLiteralProperty::GETTER);
+  DCHECK(property->kind() != ObjectLiteralProperty::SETTER);
 
   // Computed name setting must happen at runtime.
-  if (property->is_computed_name()) return;
-
-  // Getter and setter names are handled here because their names
-  // change in ES2015, even though they are not anonymous.
-  auto function = value->AsFunctionLiteral();
-  if (function != nullptr) {
-    bool is_getter = property->kind() == ObjectLiteralProperty::GETTER;
-    bool is_setter = property->kind() == ObjectLiteralProperty::SETTER;
-    if (is_getter || is_setter) {
-      DCHECK_NOT_NULL(name);
-      const AstRawString* prefix =
-          is_getter ? ast_value_factory()->get_space_string()
-                    : ast_value_factory()->set_space_string();
-      function->set_raw_name(ast_value_factory()->NewConsString(prefix, name));
-      return;
-    }
-  }
+  DCHECK(!property->is_computed_name());
 
   // Ignore "__proto__" as a name when it's being used to set the [[Prototype]]
   // of an object literal.
   if (property->kind() == ObjectLiteralProperty::PROTOTYPE) return;
 
-  DCHECK(!value->IsAnonymousFunctionDefinition() ||
-         property->kind() == ObjectLiteralProperty::COMPUTED);
-  SetFunctionName(value, name);
-}
-
-void Parser::SetFunctionNameFromPropertyName(ClassLiteralProperty* property,
-                                             const AstRawString* name) {
-  // TODO(bakkot) move this logic into Parse{Object,Class}PropertyDefinition and
-  // clean it up.
   Expression* value = property->value();
 
-  // Computed name setting must happen at runtime.
-  if (property->is_computed_name()) return;
-
-  // Getter and setter names are handled here because their names
-  // change in ES2015, even though they are not anonymous.
-  auto function = value->AsFunctionLiteral();
-  DCHECK_NOT_NULL(function);
-
-  bool is_getter = property->kind() == ClassLiteralProperty::GETTER;
-  bool is_setter = property->kind() == ClassLiteralProperty::SETTER;
-  if (is_getter || is_setter) {
-    DCHECK_NOT_NULL(name);
-    const AstRawString* prefix = is_getter
-                                     ? ast_value_factory()->get_space_string()
-                                     : ast_value_factory()->set_space_string();
-    function->set_raw_name(ast_value_factory()->NewConsString(prefix, name));
-    return;
-  }
-
+  DCHECK(!value->IsAnonymousFunctionDefinition() ||
+         property->kind() == ObjectLiteralProperty::COMPUTED);
   SetFunctionName(value, name);
 }
 

src/parsing/parser.h

@@ -1017,9 +1017,10 @@ class Parser : public ParserBase<Parser> {
 
   Expression* ExpressionListToExpression(ZoneList<Expression*>* args);
 
-  void SetFunctionNameFromPropertyName(ObjectLiteralProperty* property,
+  void AddAccessorPrefixToFunctionName(bool is_get, FunctionLiteral* function,
                                        const AstRawString* name);
-  void SetFunctionNameFromPropertyName(ClassLiteralProperty* property,
+
+  void SetFunctionNameFromPropertyName(ObjectLiteralProperty* property,
                                        const AstRawString* name);
 
   void SetFunctionNameFromIdentifierRef(Expression* value,

src/parsing/preparser.cc

@@ -856,10 +856,9 @@ PreParserExpression PreParser::ParseClassLiteral(
     if (Check(Token::SEMICOLON)) continue;
     bool is_computed_name = false;  // Classes do not care about computed
                                     // property names here.
-    Identifier name;
     ExpressionClassifier property_classifier(this);
     ParseClassPropertyDefinition(&checker, has_extends, &is_computed_name,
-                                 &has_seen_constructor, &name, CHECK_OK);
+                                 &has_seen_constructor, CHECK_OK);
     ValidateExpression(CHECK_OK);
     impl()->AccumulateFormalParameterContainmentErrors();
   }

src/parsing/preparser.h

@@ -1223,6 +1223,9 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserExpression::Default();
   }
 
+  V8_INLINE void AddAccessorPrefixToFunctionName(bool is_get,
+                                                 PreParserExpression function,
+                                                 PreParserIdentifier name) {}
   V8_INLINE void SetFunctionNameFromPropertyName(PreParserExpression property,
                                                  PreParserIdentifier name) {}
   V8_INLINE void SetFunctionNameFromIdentifierRef(

test/mjsunit/es6/function-name.js

@@ -73,6 +73,8 @@
     static 43() { }
     get 44() { }
     set 44(val) { }
+    static get constructor() { }
+    static set constructor(val) { }
   };
 
   assertEquals('a', C.prototype.a.name);
@@ -85,6 +87,9 @@
   var descriptor = Object.getOwnPropertyDescriptor(C.prototype, '44');
   assertEquals('get 44', descriptor.get.name);
   assertEquals('set 44', descriptor.set.name);
+  var descriptor = Object.getOwnPropertyDescriptor(C, 'constructor');
+  assertEquals('get constructor', descriptor.get.name);
+  assertEquals('set constructor', descriptor.set.name);
 })();
 
 (function testComputedProperties() {