[parser] Improve propagation of SharedFunctionInfo::has_shared_name().

The initial implementation did not work in certain cases.
For example, in the following case 'f' didn't have a shared name while
it should have had an empty shared name:
  var f = (function() { return function() { return 42; } }();

The new implementation ensures that all anonymous functions have empty
shared name and if any of them happen to be an object literal property
value or an accessor function or a concise method then such a function
is marked as having no shared name.

Bug: v8:6459
Change-Id: I0f936afce0c152d91b2b41c1dc475a5ed841eca0
Reviewed-on: https://chromium-review.googlesource.com/538666Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46026}

src/ast/ast.cc

@@ -151,6 +151,14 @@ bool Expression::IsAnonymousFunctionDefinition() const {
           AsClassLiteral()->IsAnonymousFunctionDefinition());
 }
 
+bool Expression::IsConciseMethodDefinition() const {
+  return IsFunctionLiteral() && IsConciseMethod(AsFunctionLiteral()->kind());
+}
+
+bool Expression::IsAccessorFunctionDefinition() const {
+  return IsFunctionLiteral() && IsAccessorFunction(AsFunctionLiteral()->kind());
+}
+
 void Expression::MarkTail() {
   if (IsConditional()) {
     AsConditional()->MarkTail();
@@ -395,10 +403,9 @@ void LiteralProperty::SetStoreDataPropertySlot(FeedbackSlot slot) {
 }
 
 bool LiteralProperty::NeedsSetFunctionName() const {
-  return is_computed_name_ &&
-         (value_->IsAnonymousFunctionDefinition() ||
-          (value_->IsFunctionLiteral() &&
-           IsConciseMethod(value_->AsFunctionLiteral()->kind())));
+  return is_computed_name_ && (value_->IsAnonymousFunctionDefinition() ||
+                               value_->IsConciseMethodDefinition() ||
+                               value_->IsAccessorFunctionDefinition());
 }
 
 ClassLiteralProperty::ClassLiteralProperty(Expression* key, Expression* value,

src/ast/ast.h

@@ -315,6 +315,12 @@ class Expression : public AstNode {
   // a syntactic name.
   bool IsAnonymousFunctionDefinition() const;
 
+  // True iff the expression is a concise method definition.
+  bool IsConciseMethodDefinition() const;
+
+  // True iff the expression is an accessor function definition.
+  bool IsAccessorFunctionDefinition() const;
+
   // True iff the expression is a literal represented as a smi.
   bool IsSmiLiteral() const;
 

src/parsing/parser-base.h

@@ -2336,9 +2336,12 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
             has_initializer, CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
         ExpectSemicolon(CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
         *property_kind = ClassLiteralProperty::FIELD;
-        return factory()->NewClassLiteralProperty(
+        ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
             name_expression, function_literal, *property_kind, *is_static,
             *is_computed_name);
+        impl()->SetFunctionNameFromPropertyName(result, name);
+        return result;
+
       } else {
         ReportUnexpectedToken(Next());
         *ok = false;
@@ -2378,9 +2381,11 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
           CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
 
       *property_kind = ClassLiteralProperty::METHOD;
-      return factory()->NewClassLiteralProperty(name_expression, value,
-                                                *property_kind, *is_static,
+      ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
+          name_expression, value, *property_kind, *is_static,
           *is_computed_name);
+      impl()->SetFunctionNameFromPropertyName(result, name);
+      return result;
     }
 
     case PropertyKind::kAccessorProperty: {
@@ -2407,15 +2412,16 @@ ParserBase<Impl>::ParseClassPropertyDefinition(
           FunctionLiteral::kAccessorOrMethod, language_mode(),
           CHECK_OK_CUSTOM(EmptyClassLiteralProperty));
 
-      if (!*is_computed_name) {
-        impl()->AddAccessorPrefixToFunctionName(is_get, value, name);
-      }
-
       *property_kind =
           is_get ? ClassLiteralProperty::GETTER : ClassLiteralProperty::SETTER;
-      return factory()->NewClassLiteralProperty(name_expression, value,
-                                                *property_kind, *is_static,
+      ClassLiteralPropertyT result = factory()->NewClassLiteralProperty(
+          name_expression, value, *property_kind, *is_static,
           *is_computed_name);
+      const AstRawString* prefix =
+          is_get ? ast_value_factory()->get_space_string()
+                 : ast_value_factory()->set_space_string();
+      impl()->SetFunctionNameFromPropertyName(result, name, prefix);
+      return result;
     }
     case PropertyKind::kSpreadProperty:
       ReportUnexpectedTokenAt(
@@ -2509,14 +2515,7 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
 
       ObjectLiteralPropertyT result = factory()->NewObjectLiteralProperty(
           name_expression, value, *is_computed_name);
-
-      if (*is_computed_name) {
-        impl()->SetFunctionNameFromPropertyName(result,
-                                                impl()->EmptyIdentifier());
-      } else {
       impl()->SetFunctionNameFromPropertyName(result, name);
-      }
-
       return result;
     }
 
@@ -2583,8 +2582,10 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
         value = lhs;
       }
 
-      return factory()->NewObjectLiteralProperty(
+      ObjectLiteralPropertyT result = factory()->NewObjectLiteralProperty(
           name_expression, value, ObjectLiteralProperty::COMPUTED, false);
+      impl()->SetFunctionNameFromPropertyName(result, name);
+      return result;
     }
 
     case PropertyKind::kMethodProperty: {
@@ -2606,9 +2607,11 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
           FunctionLiteral::kAccessorOrMethod, language_mode(),
           CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
 
-      return factory()->NewObjectLiteralProperty(
+      ObjectLiteralPropertyT result = factory()->NewObjectLiteralProperty(
           name_expression, value, ObjectLiteralProperty::COMPUTED,
           *is_computed_name);
+      impl()->SetFunctionNameFromPropertyName(result, name);
+      return result;
     }
 
     case PropertyKind::kAccessorProperty: {
@@ -2636,14 +2639,16 @@ ParserBase<Impl>::ParseObjectPropertyDefinition(ObjectLiteralChecker* checker,
           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
+      ObjectLiteralPropertyT result = factory()->NewObjectLiteralProperty(
+          name_expression, value,
+          is_get ? ObjectLiteralProperty::GETTER
                  : ObjectLiteralProperty::SETTER,
           *is_computed_name);
+      const AstRawString* prefix =
+          is_get ? ast_value_factory()->get_space_string()
+                 : ast_value_factory()->set_space_string();
+      impl()->SetFunctionNameFromPropertyName(result, name, prefix);
+      return result;
     }
 
     case PropertyKind::kClassField:

src/parsing/parser.cc

@@ -2551,6 +2551,12 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
   // handle to decide whether to invoke function name inference.
   bool should_infer_name = function_name == NULL;
 
+  // We want a non-null handle as the function name by default. We will handle
+  // the "function does not have a shared name" case later.
+  if (should_infer_name) {
+    function_name = ast_value_factory()->empty_string();
+  }
+
   FunctionLiteral::EagerCompileHint eager_compile_hint =
       function_state_->next_function_is_likely_called()
           ? FunctionLiteral::kShouldEagerCompile
@@ -3195,9 +3201,9 @@ ZoneList<Statement*>* Parser::ParseFunction(
   if (expected_parameters_end_pos != kNoSourcePosition) {
     // This is the first function encountered in a CreateDynamicFunction eval.
     parameters_end_pos_ = kNoSourcePosition;
-    // The function name should have been ignored, giving us the nullptr
+    // The function name should have been ignored, giving us the empty string
     // here.
-    DCHECK_NULL(function_name);
+    DCHECK_EQ(function_name, ast_value_factory()->empty_string());
   }
 
   ParserFormalParameters formals(function_scope);
@@ -4156,32 +4162,41 @@ 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(LiteralProperty* property,
+                                             const AstRawString* name,
+                                             const AstRawString* prefix) {
+  // Ensure that the function we are going to create has shared name iff
+  // we are not going to set it later.
+  if (property->NeedsSetFunctionName()) {
+    name = nullptr;
+    prefix = nullptr;
+  } else {
+    // If the property value is an anonymous function or an anonymous class or
+    // a concise method or an accessor function which doesn't require the name
+    // to be set then the shared name must be provided.
+    DCHECK_IMPLIES(property->value()->IsAnonymousFunctionDefinition() ||
+                       property->value()->IsConciseMethodDefinition() ||
+                       property->value()->IsAccessorFunctionDefinition(),
+                   name != nullptr);
+  }
+
+  Expression* value = property->value();
+  SetFunctionName(value, name, prefix);
 }
 
 void Parser::SetFunctionNameFromPropertyName(ObjectLiteralProperty* property,
-                                             const AstRawString* name) {
-  DCHECK(property->kind() != ObjectLiteralProperty::GETTER);
-  DCHECK(property->kind() != ObjectLiteralProperty::SETTER);
-
-  // Computed name setting must happen at runtime.
-  DCHECK_IMPLIES(property->is_computed_name(), name == nullptr);
-
+                                             const AstRawString* name,
+                                             const AstRawString* prefix) {
   // Ignore "__proto__" as a name when it's being used to set the [[Prototype]]
   // of an object literal.
+  // See ES #sec-__proto__-property-names-in-object-initializers.
   if (property->IsPrototype()) return;
 
-  Expression* value = property->value();
-
-  DCHECK(!value->IsAnonymousFunctionDefinition() ||
+  DCHECK(!property->value()->IsAnonymousFunctionDefinition() ||
          property->kind() == ObjectLiteralProperty::COMPUTED);
-  SetFunctionName(value, name);
+
+  SetFunctionNameFromPropertyName(static_cast<LiteralProperty*>(property), name,
+                                  prefix);
 }
 
 void Parser::SetFunctionNameFromIdentifierRef(Expression* value,
@@ -4190,15 +4205,29 @@ void Parser::SetFunctionNameFromIdentifierRef(Expression* value,
   SetFunctionName(value, identifier->AsVariableProxy()->raw_name());
 }
 
-void Parser::SetFunctionName(Expression* value, const AstRawString* name) {
-  if (!value->IsAnonymousFunctionDefinition()) return;
+void Parser::SetFunctionName(Expression* value, const AstRawString* name,
+                             const AstRawString* prefix) {
+  if (!value->IsAnonymousFunctionDefinition() &&
+      !value->IsConciseMethodDefinition() &&
+      !value->IsAccessorFunctionDefinition()) {
+    return;
+  }
   auto function = value->AsFunctionLiteral();
   if (value->IsClassLiteral()) {
     function = value->AsClassLiteral()->constructor();
   }
   if (function != nullptr) {
-    function->set_raw_name(
-        name != nullptr ? ast_value_factory()->NewConsString(name) : nullptr);
+    AstConsString* cons_name = nullptr;
+    if (name != nullptr) {
+      if (prefix != nullptr) {
+        cons_name = ast_value_factory()->NewConsString(prefix, name);
+      } else {
+        cons_name = ast_value_factory()->NewConsString(name);
+      }
+    } else {
+      DCHECK_NULL(prefix);
+    }
+    function->set_raw_name(cons_name);
   }
 }
 

src/parsing/parser.h

@@ -694,7 +694,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   void AddArrowFunctionFormalParameters(ParserFormalParameters* parameters,
                                         Expression* params, int end_pos,
                                         bool* ok);
-  void SetFunctionName(Expression* value, const AstRawString* name);
+  void SetFunctionName(Expression* value, const AstRawString* name,
+                       const AstRawString* prefix = nullptr);
 
   // Helper functions for recursive descent.
   V8_INLINE bool IsEval(const AstRawString* identifier) const {
@@ -1111,11 +1112,12 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   Expression* ExpressionListToExpression(ZoneList<Expression*>* args);
 
-  void AddAccessorPrefixToFunctionName(bool is_get, FunctionLiteral* function,
-                                       const AstRawString* name);
-
+  void SetFunctionNameFromPropertyName(LiteralProperty* property,
+                                       const AstRawString* name,
+                                       const AstRawString* prefix = nullptr);
   void SetFunctionNameFromPropertyName(ObjectLiteralProperty* property,
-                                       const AstRawString* name);
+                                       const AstRawString* name,
+                                       const AstRawString* prefix = nullptr);
 
   void SetFunctionNameFromIdentifierRef(Expression* value,
                                         Expression* identifier);

src/parsing/preparser.h

@@ -1678,11 +1678,9 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserExpression::Default(args.variables_);
   }
 
-  V8_INLINE void AddAccessorPrefixToFunctionName(bool is_get,
-                                                 PreParserExpression function,
-                                                 PreParserIdentifier name) {}
-  V8_INLINE void SetFunctionNameFromPropertyName(PreParserExpression property,
-                                                 PreParserIdentifier name) {}
+  V8_INLINE void SetFunctionNameFromPropertyName(
+      PreParserExpression property, PreParserIdentifier name,
+      const AstRawString* prefix = nullptr) {}
   V8_INLINE void SetFunctionNameFromIdentifierRef(
       PreParserExpression value, PreParserExpression identifier) {}
 

test/mjsunit/es6/function-name.js

@@ -106,12 +106,14 @@
   var sym2 = Symbol('2');
   var sym3 = Symbol('3');
   var symNoDescription = Symbol();
+  var proto = "__proto__";
   var obj = {
     ['']: function() {},
     [a]: function() {},
     [sym1]: function() {},
     [sym2]: function withName() {},
     [symNoDescription]: function() {},
+    [proto]: function() {},
 
     get [sym3]() {},
     set [b](val) {},
@@ -122,6 +124,7 @@
   assertEquals('[1]', obj[sym1].name);
   assertEquals('withName', obj[sym2].name);
   assertEquals('', obj[symNoDescription].name);
+  assertEquals('__proto__', obj[proto].name);
 
   assertEquals('get [3]', Object.getOwnPropertyDescriptor(obj, sym3).get.name);
   assertEquals('set b', Object.getOwnPropertyDescriptor(obj, 'b').set.name);
@@ -130,13 +133,15 @@
     ['']() {},
     [a]() {},
     [sym1]() {},
-    [symNoDescription]: function() {},
+    [symNoDescription]() {},
+    [proto]() {},
   };
 
   assertEquals('', objMethods[''].name);
   assertEquals('a', objMethods[a].name);
   assertEquals('[1]', objMethods[sym1].name);
   assertEquals('', objMethods[symNoDescription].name);
+  assertEquals('__proto__', objMethods[proto].name);
 
   class C {
     ['']() { }