[parser] Give hoisting sloppy block functions a valid position

A sloppy function in a block scope implicitily creates a var in the outer
declaration scope if it's not blocked. The assignment created reads the local
lexical declaration for the function. The reference introduced automatically
takes part in NeedsHoleCheck, requiring the reference to have a valid position.
Since the assignment will happen after the local declaration, we give the
end_position() of the closure as the position of the reference, so hole checks
can be omitted.

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

src/ast/ast.h

@@ -994,9 +994,8 @@ class SloppyBlockFunctionStatement final : public Statement {
  private:
   friend class AstNodeFactory;
 
-  explicit SloppyBlockFunctionStatement(Statement* statement)
-      : Statement(kNoSourcePosition, kSloppyBlockFunctionStatement),
-        statement_(statement) {}
+  SloppyBlockFunctionStatement(int pos, Statement* statement)
+      : Statement(pos, kSloppyBlockFunctionStatement), statement_(statement) {}
 
   Statement* statement_;
 };
@@ -2967,8 +2966,8 @@ class AstNodeFactory final {
     return failure_expression_;
   }
 
-  SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement() {
-    return new (zone_) SloppyBlockFunctionStatement(EmptyStatement());
+  SloppyBlockFunctionStatement* NewSloppyBlockFunctionStatement(int pos) {
+    return new (zone_) SloppyBlockFunctionStatement(pos, EmptyStatement());
   }
 
   CaseClause* NewCaseClause(Expression* label,

src/ast/scopes.cc

@@ -542,12 +542,12 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
 
       if (factory) {
         DCHECK(!is_being_lazily_parsed_);
+        int pos = delegate->position();
         Assignment* assignment = factory->NewAssignment(
-            Token::ASSIGN, NewUnresolved(factory, name),
-            delegate->scope()->NewUnresolved(factory, name), kNoSourcePosition);
+            Token::ASSIGN, NewUnresolved(factory, name, pos),
+            delegate->scope()->NewUnresolved(factory, name, pos), pos);
         assignment->set_lookup_hoisting_mode(LookupHoistingMode::kLegacySloppy);
-        Statement* statement =
-            factory->NewExpressionStatement(assignment, kNoSourcePosition);
+        Statement* statement = factory->NewExpressionStatement(assignment, pos);
         delegate->set_statement(statement);
       }
     }

src/ast/scopes.h

@@ -53,10 +53,12 @@ class SloppyBlockFunctionMap : public ZoneHashMap {
     Delegate(Scope* scope, SloppyBlockFunctionStatement* statement, int index)
         : scope_(scope), statement_(statement), next_(nullptr), index_(index) {}
     void set_statement(Statement* statement);
+
     void set_next(Delegate* next) { next_ = next; }
     Delegate* next() const { return next_; }
     Scope* scope() const { return scope_; }
     int index() const { return index_; }
+    int position() const { return statement_->position(); }
 
    private:
     Scope* scope_;
@@ -240,8 +242,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
 
   // Create a new unresolved variable.
   VariableProxy* NewUnresolved(AstNodeFactory* factory,
-                               const AstRawString* name,
-                               int start_pos = kNoSourcePosition,
+                               const AstRawString* name, int start_pos,
                                VariableKind kind = NORMAL_VARIABLE) {
     // Note that we must not share the unresolved variables with
     // the same name because they may be removed selectively via

src/parsing/parser-base.h

@@ -3610,7 +3610,8 @@ ParserBase<Impl>::ParseHoistableDeclaration(
                                   flags == ParseFunctionFlag::kIsNormal;
 
   return impl()->DeclareFunction(variable_name, function, mode, pos,
-                                 is_sloppy_block_function, names);
+                                 end_position(), is_sloppy_block_function,
+                                 names);
 }
 
 template <typename Impl>

src/parsing/parser.cc

@@ -1404,17 +1404,18 @@ Statement* Parser::BuildInitializationBlock(
 
 Statement* Parser::DeclareFunction(const AstRawString* variable_name,
                                    FunctionLiteral* function, VariableMode mode,
-                                   int pos, bool is_sloppy_block_function,
+                                   int beg_pos, int end_pos,
+                                   bool is_sloppy_block_function,
                                    ZonePtrList<const AstRawString>* names) {
   VariableProxy* proxy =
-      factory()->NewVariableProxy(variable_name, NORMAL_VARIABLE, pos);
+      factory()->NewVariableProxy(variable_name, NORMAL_VARIABLE, beg_pos);
   Declaration* declaration = factory()->NewFunctionDeclaration(
-      proxy, function, is_sloppy_block_function, pos);
+      proxy, function, is_sloppy_block_function, beg_pos);
   Declare(declaration, NORMAL_VARIABLE, mode, kCreatedInitialized, scope());
   if (names) names->Add(variable_name, zone());
   if (is_sloppy_block_function) {
     SloppyBlockFunctionStatement* statement =
-        factory()->NewSloppyBlockFunctionStatement();
+        factory()->NewSloppyBlockFunctionStatement(end_pos);
     GetDeclarationScope()->DeclareSloppyBlockFunction(variable_name, scope(),
                                                       statement);
     return statement;

src/parsing/parser.h

@@ -338,7 +338,8 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
 
   Statement* DeclareFunction(const AstRawString* variable_name,
                              FunctionLiteral* function, VariableMode mode,
-                             int pos, bool is_sloppy_block_function,
+                             int beg_pos, int end_pos,
+                             bool is_sloppy_block_function,
                              ZonePtrList<const AstRawString>* names);
   Variable* CreateSyntheticContextVariable(const AstRawString* synthetic_name);
   FunctionLiteral* CreateInitializerFunction(

src/parsing/preparser.h

@@ -1196,10 +1196,11 @@ class PreParser : public ParserBase<PreParser> {
     return PreParserStatement::Default();
   }
 
-  V8_INLINE PreParserStatement DeclareFunction(
-      const PreParserIdentifier& variable_name,
-      const PreParserExpression& function, VariableMode mode, int pos,
-      bool is_sloppy_block_function, ZonePtrList<const AstRawString>* names) {
+  V8_INLINE PreParserStatement
+  DeclareFunction(const PreParserIdentifier& variable_name,
+                  const PreParserExpression& function, VariableMode mode,
+                  int beg_pos, int end_pos, bool is_sloppy_block_function,
+                  ZonePtrList<const AstRawString>* names) {
     DCHECK_NULL(names);
     if (variable_name.string_ != nullptr) {
       scope()->DeclareVariableName(variable_name.string_, mode);

test/cctest/interpreter/bytecode_expectations/BasicLoops.golden

@@ -708,7 +708,7 @@ bytecodes: [
                 B(Star), R(0),
   /*   73 S> */ B(LdaSmi), I8(1),
   /*   73 E> */ B(StaCurrentContextSlot), U8(4),
-                B(Mov), R(0), R(2),
+  /*  102 S> */ B(Mov), R(0), R(2),
   /*  106 S> */ B(LdaCurrentContextSlot), U8(4),
                 B(JumpIfToBooleanFalse), U8(6),
   /*  113 S> */ B(PopContext), R(3),

test/cctest/interpreter/bytecode_expectations/BreakableBlocks.golden

@@ -112,7 +112,7 @@ bytecodes: [
                 B(Star), R(0),
   /*   53 S> */ B(LdaSmi), I8(10),
   /*   53 E> */ B(StaCurrentContextSlot), U8(4),
-                B(Mov), R(0), R(1),
+  /*   85 S> */ B(Mov), R(0), R(1),
                 B(Ldar), R(0),
   /*   88 S> */ B(Jump), U8(2),
                 B(PopContext), R(2),
@@ -158,7 +158,7 @@ bytecodes: [
                 B(Star), R(0),
   /*   76 S> */ B(LdaSmi), I8(2),
   /*   76 E> */ B(StaCurrentContextSlot), U8(4),
-                B(Mov), R(0), R(1),
+  /*  113 S> */ B(Mov), R(0), R(1),
   /*  118 S> */ B(LdaCurrentContextSlot), U8(4),
                 B(JumpIfToBooleanFalse), U8(6),
   /*  125 S> */ B(PopContext), R(3),

test/mjsunit/regress/regress-917755.js

+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+{
+    function a() {}
+}
+
+{
+    let a;
+    function a() {};
+}