[parser] Fix arrowhead parsing in the script scope

When analyzing functions scopes with the script_scope as parent, don't
skip migrating unresolved variables upwards if we could still be inside
an arrow head, which means accesses to those variables will be
correctly context allocated.

Bug: v8:8510, chromium:1000094
Change-Id: I684f2f8bc692de420203990f93e5c943b5b769c9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1789705Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63635}

src/ast/scopes.cc

@@ -1386,9 +1386,10 @@ void Scope::CollectNonLocals(DeclarationScope* max_outer_scope,
 
 void Scope::AnalyzePartially(DeclarationScope* max_outer_scope,
                              AstNodeFactory* ast_node_factory,
-                             UnresolvedList* new_unresolved_list) {
-  this->ForEach([max_outer_scope, ast_node_factory,
-                 new_unresolved_list](Scope* scope) {
+                             UnresolvedList* new_unresolved_list,
+                             bool maybe_in_arrowhead) {
+  this->ForEach([max_outer_scope, ast_node_factory, new_unresolved_list,
+                 maybe_in_arrowhead](Scope* scope) {
     DCHECK_IMPLIES(scope->is_declaration_scope(),
                    !scope->AsDeclarationScope()->was_lazily_parsed());
 
@@ -1401,7 +1402,8 @@ void Scope::AnalyzePartially(DeclarationScope* max_outer_scope,
         // Don't copy unresolved references to the script scope, unless it's a
         // reference to a private name or method. In that case keep it so we
         // can fail later.
-        if (!max_outer_scope->outer_scope()->is_script_scope()) {
+        if (!max_outer_scope->outer_scope()->is_script_scope() ||
+            maybe_in_arrowhead) {
           VariableProxy* copy = ast_node_factory->CopyVariableProxy(proxy);
           new_unresolved_list->Add(copy);
         }
@@ -1490,17 +1492,19 @@ void DeclarationScope::SavePreparseDataForDeclarationScope(Parser* parser) {
 }
 
 void DeclarationScope::AnalyzePartially(Parser* parser,
-                                        AstNodeFactory* ast_node_factory) {
+                                        AstNodeFactory* ast_node_factory,
+                                        bool maybe_in_arrowhead) {
   DCHECK(!force_eager_compilation_);
   UnresolvedList new_unresolved_list;
   if (!IsArrowFunction(function_kind_) &&
-      (!outer_scope_->is_script_scope() ||
+      (!outer_scope_->is_script_scope() || maybe_in_arrowhead ||
        (preparse_data_builder_ != nullptr &&
         preparse_data_builder_->HasInnerFunctions()))) {
     // Try to resolve unresolved variables for this Scope and migrate those
     // which cannot be resolved inside. It doesn't make sense to try to resolve
     // them in the outer Scopes here, because they are incomplete.
-    Scope::AnalyzePartially(this, ast_node_factory, &new_unresolved_list);
+    Scope::AnalyzePartially(this, ast_node_factory, &new_unresolved_list,
+                            maybe_in_arrowhead);
 
     // Migrate function_ to the right Zone.
     if (function_ != nullptr) {

src/ast/scopes.h

@@ -610,7 +610,8 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // list along the way, so full resolution cannot be done afterwards.
   void AnalyzePartially(DeclarationScope* max_outer_scope,
                         AstNodeFactory* ast_node_factory,
-                        UnresolvedList* new_unresolved_list);
+                        UnresolvedList* new_unresolved_list,
+                        bool maybe_in_arrowhead);
   void CollectNonLocals(DeclarationScope* max_outer_scope, Isolate* isolate,
                         ParseInfo* info, Handle<StringSet>* non_locals);
 
@@ -1019,7 +1020,8 @@ class V8_EXPORT_PRIVATE DeclarationScope : public Scope {
   // this records variables which cannot be resolved inside the Scope (we don't
   // yet know what they will resolve to since the outer Scopes are incomplete)
   // and recreates them with the correct Zone with ast_node_factory.
-  void AnalyzePartially(Parser* parser, AstNodeFactory* ast_node_factory);
+  void AnalyzePartially(Parser* parser, AstNodeFactory* ast_node_factory,
+                        bool maybe_in_arrowhead);
 
   // Allocate ScopeInfos for top scope and any inner scopes that need them.
   // Does nothing if ScopeInfo is already allocated.

src/parsing/expression-scope.h

@@ -189,6 +189,10 @@ class ExpressionScope {
     return variable_index;
   }
 
+  bool has_possible_arrow_parameter_in_scope_chain() const {
+    return has_possible_arrow_parameter_in_scope_chain_;
+  }
+
  protected:
   enum ScopeType : uint8_t {
     // Expression or assignment target.
@@ -217,7 +221,11 @@ class ExpressionScope {
         type_(type),
         has_possible_parameter_in_scope_chain_(
             CanBeParameterDeclaration() ||
-            (parent_ && parent_->has_possible_parameter_in_scope_chain_)) {
+            (parent_ && parent_->has_possible_parameter_in_scope_chain_)),
+        has_possible_arrow_parameter_in_scope_chain_(
+            CanBeArrowParameterDeclaration() ||
+            (parent_ &&
+             parent_->has_possible_arrow_parameter_in_scope_chain_)) {
     parser->expression_scope_ = this;
   }
 
@@ -281,6 +289,10 @@ class ExpressionScope {
     return IsInRange(type_, kMaybeArrowParameterDeclaration,
                      kParameterDeclaration);
   }
+  bool CanBeArrowParameterDeclaration() const {
+    return IsInRange(type_, kMaybeArrowParameterDeclaration,
+                     kMaybeAsyncArrowParameterDeclaration);
+  }
   bool IsCertainlyParameterDeclaration() const {
     return type_ == kParameterDeclaration;
   }
@@ -289,6 +301,7 @@ class ExpressionScope {
   ExpressionScope<Types>* parent_;
   ScopeType type_;
   bool has_possible_parameter_in_scope_chain_;
+  bool has_possible_arrow_parameter_in_scope_chain_;
 
   DISALLOW_COPY_AND_ASSIGN(ExpressionScope);
 };

src/parsing/parser-base.h

@@ -1331,6 +1331,11 @@ class ParserBase {
     return expression_scope_;
   }
 
+  bool MaybeParsingArrowhead() const {
+    return expression_scope_ != nullptr &&
+           expression_scope_->has_possible_arrow_parameter_in_scope_chain();
+  }
+
   class AcceptINScope final {
    public:
     AcceptINScope(ParserBase* parser, bool accept_IN)

src/parsing/parser.cc

@@ -2555,7 +2555,7 @@ bool Parser::SkipFunction(const AstRawString* function_name, FunctionKind kind,
       private_name_scope_iter.GetScope()->MigrateUnresolvedPrivateNameTail(
           factory(), unresolved_private_tail);
     }
-    function_scope->AnalyzePartially(this, factory());
+    function_scope->AnalyzePartially(this, factory(), MaybeParsingArrowhead());
   }
 
   return true;

test/mjsunit/regress/regress-crbug-1000094.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.
+
+// Flags: --enable-lazy-source-positions --stress-lazy-source-positions
+
+var f = (( {a: b} = {
+    a() {
+      return b;
+    }
+}) => b)()();
+
+// b should get assigned to the inner function a, which then ends up returning
+// itself.
+assertEquals(f, f());