Move function name var initialization to BytecodeGenerator

Besides avoiding the weird hack of inserting a statement at the 0th
index of the function body, we also avoid allocating (and initializing)
the variable if it's unreferenced (which I'd wager is the common case).

Bug: v8:6092
Change-Id: If917d422bb4818cf21e8272aa786ca84d4472802
Reviewed-on: https://chromium-review.googlesource.com/784092Reviewed-by: Marja Hölttä <marja@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#49646}

src/ast/scopes.cc

@@ -2235,8 +2235,10 @@ void DeclarationScope::AllocateLocals() {
   // allocated in the context, it must be the last slot in the context,
   // because of the current ScopeInfo implementation (see
   // ScopeInfo::ScopeInfo(FunctionScope* scope) constructor).
-  if (function_ != nullptr) {
+  if (function_ != nullptr && MustAllocate(function_)) {
     AllocateNonParameterLocal(function_);
+  } else {
+    function_ = nullptr;
   }
 
   DCHECK(!has_rest_ || !MustAllocate(rest_parameter()) ||

src/ast/scopes.h

@@ -762,10 +762,7 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
 
   // The variable holding the function literal for named function
   // literals, or nullptr.  Only valid for function scopes.
-  Variable* function_var() const {
-    DCHECK(is_function_scope());
-    return function_;
-  }
+  Variable* function_var() const { return function_; }
 
   Variable* generator_object_var() const {
     DCHECK(is_function_scope() || is_module_scope());

src/interpreter/bytecode-generator.cc

@@ -1016,7 +1016,9 @@ void BytecodeGenerator::GenerateBytecodeBody() {
   Variable* rest_parameter = closure_scope()->rest_parameter();
   VisitRestArgumentsArray(rest_parameter);
 
-  // Build assignment to {.this_function} variable if it is used.
+  // Build assignment to the function name or {.this_function}
+  // variables if used.
+  VisitThisFunctionVariable(closure_scope()->function_var());
   VisitThisFunctionVariable(closure_scope()->this_function_var());
 
   // Build assignment to {new.target} variable if it is used.

src/parsing/parser-base.h

@@ -4141,18 +4141,6 @@ void ParserBase<Impl>::ParseFunctionBody(
     typename ParserBase<Impl>::StatementListT result, IdentifierT function_name,
     int pos, const FormalParametersT& parameters, FunctionKind kind,
     FunctionLiteral::FunctionType function_type, bool* ok) {
-  static const int kFunctionNameAssignmentIndex = 0;
-  if (function_type == FunctionLiteral::kNamedExpression) {
-    DCHECK(!impl()->IsNull(function_name));
-    // If we have a named function expression, we add a local variable
-    // declaration to the body of the function with the name of the
-    // function and let it refer to the function itself (closure).
-    // Not having parsed the function body, the language mode may still change,
-    // so we reserve a spot and create the actual const assignment later.
-    DCHECK_EQ(kFunctionNameAssignmentIndex, result->length());
-    result->Add(impl()->NullStatement(), zone());
-  }
-
   DeclarationScope* function_scope = scope()->AsDeclarationScope();
   DeclarationScope* inner_scope = function_scope;
   BlockT inner_block = impl()->NullStatement();
@@ -4233,9 +4221,7 @@ void ParserBase<Impl>::ParseFunctionBody(
     function_scope->DeclareArguments(ast_value_factory());
   }
 
-  impl()->CreateFunctionNameAssignment(function_name, pos, function_type,
-                                       function_scope, result,
-                                       kFunctionNameAssignmentIndex);
+  impl()->DeclareFunctionNameVar(function_name, function_type, function_scope);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -1828,25 +1828,13 @@ void Parser::ParseAndRewriteAsyncGeneratorFunctionBody(
             zone());
 }
 
-void Parser::CreateFunctionNameAssignment(
-    const AstRawString* function_name, int pos,
-    FunctionLiteral::FunctionType function_type,
-    DeclarationScope* function_scope, ZoneList<Statement*>* result, int index) {
-  if (function_type == FunctionLiteral::kNamedExpression) {
-    StatementT statement = factory()->NewEmptyStatement(kNoSourcePosition);
-    if (function_scope->LookupLocal(function_name) == nullptr) {
-      // Now that we know the language mode, we can create the const assignment
-      // in the previously reserved spot.
-      DCHECK_EQ(function_scope, scope());
-      Variable* fvar = function_scope->DeclareFunctionVar(function_name);
-      VariableProxy* fproxy = factory()->NewVariableProxy(fvar);
-      statement = factory()->NewExpressionStatement(
-          factory()->NewAssignment(Token::INIT, fproxy,
-                                   factory()->NewThisFunction(pos),
-                                   kNoSourcePosition),
-          kNoSourcePosition);
-    }
-    result->Set(index, statement);
+void Parser::DeclareFunctionNameVar(const AstRawString* function_name,
+                                    FunctionLiteral::FunctionType function_type,
+                                    DeclarationScope* function_scope) {
+  if (function_type == FunctionLiteral::kNamedExpression &&
+      function_scope->LookupLocal(function_name) == nullptr) {
+    DCHECK_EQ(function_scope, scope());
+    function_scope->DeclareFunctionVar(function_name);
   }
 }
 

src/parsing/parser.h

@@ -346,10 +346,9 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   void ParseAndRewriteAsyncGeneratorFunctionBody(int pos, FunctionKind kind,
                                                  ZoneList<Statement*>* body,
                                                  bool* ok);
-  void CreateFunctionNameAssignment(const AstRawString* function_name, int pos,
-                                    FunctionLiteral::FunctionType function_type,
-                                    DeclarationScope* function_scope,
-                                    ZoneList<Statement*>* result, int index);
+  void DeclareFunctionNameVar(const AstRawString* function_name,
+                              FunctionLiteral::FunctionType function_type,
+                              DeclarationScope* function_scope);
 
   Statement* DeclareFunction(const AstRawString* variable_name,
                              FunctionLiteral* function, VariableMode mode,

src/parsing/preparser.cc

@@ -207,7 +207,7 @@ PreParser::PreParseResult PreParser::PreParseFunction(
 
   if (!IsArrowFunction(kind) && track_unresolved_variables_ &&
       result == kLazyParsingComplete) {
-    CreateFunctionNameAssignment(function_name, function_type, function_scope);
+    DeclareFunctionNameVar(function_name, function_type, function_scope);
 
     // Declare arguments after parsing the function since lexical 'arguments'
     // masks the arguments object. Declare arguments before declaring the

src/parsing/preparser.h

@@ -1073,27 +1073,24 @@ class PreParser : public ParserBase<PreParser> {
       int pos, FunctionKind kind, PreParserStatementList body, bool* ok) {
     ParseStatementList(body, Token::RBRACE, ok);
   }
-  V8_INLINE void CreateFunctionNameAssignment(
+  V8_INLINE void DeclareFunctionNameVar(
       const AstRawString* function_name,
       FunctionLiteral::FunctionType function_type,
       DeclarationScope* function_scope) {
     if (track_unresolved_variables_ &&
-        function_type == FunctionLiteral::kNamedExpression) {
-      if (function_scope->LookupLocal(function_name) == nullptr) {
-        DCHECK_EQ(function_scope, scope());
-        Variable* fvar = function_scope->DeclareFunctionVar(function_name);
-        fvar->set_is_used();
-      }
+        function_type == FunctionLiteral::kNamedExpression &&
+        function_scope->LookupLocal(function_name) == nullptr) {
+      DCHECK_EQ(function_scope, scope());
+      function_scope->DeclareFunctionVar(function_name);
     }
   }
 
-  V8_INLINE void CreateFunctionNameAssignment(
-      const PreParserIdentifier& function_name, int pos,
+  V8_INLINE void DeclareFunctionNameVar(
+      const PreParserIdentifier& function_name,
       FunctionLiteral::FunctionType function_type,
-      DeclarationScope* function_scope, PreParserStatementList result,
-      int index) {
-    CreateFunctionNameAssignment(function_name.string_, function_type,
-                                 function_scope);
+      DeclarationScope* function_scope) {
+    DeclareFunctionNameVar(function_name.string_, function_type,
+                           function_scope);
   }
 
   V8_INLINE PreParserExpression RewriteDoExpression(PreParserStatement body,

test/cctest/interpreter/bytecode_expectations/ThisFunction.golden

@@ -12,12 +12,11 @@ snippet: "
   f = function f() {};
   f();
 "
-frame size: 1
+frame size: 0
 parameter count: 1
-bytecode array length: 6
+bytecode array length: 3
 bytecodes: [
   /*   21 E> */ B(StackCheck),
-                B(Mov), R(closure), R(0),
                 B(LdaUndefined),
   /*   25 S> */ B(Return),
 ]
@@ -36,8 +35,8 @@ frame size: 1
 parameter count: 1
 bytecode array length: 7
 bytecodes: [
-  /*   21 E> */ B(StackCheck),
                 B(Mov), R(closure), R(0),
+  /*   21 E> */ B(StackCheck),
   /*   26 S> */ B(Ldar), R(0),
   /*   35 S> */ B(Return),
 ]