[parser] Declarations store Variables not proxies

Storing a VariableProxy in declarations means that a declaration and
initialisation assignment are tightly coupled to use the same var. In
particular, this means that Var declarations in with scopes have to
clone the VariableProxy to split the declaration and initializer LHS
lookup.

This patch changes declarations to point directly to the Variable, not
the VariableProxy. This will allow future refactoring to decouple
declarations and initialisations.

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

src/ast/ast.h

@@ -431,14 +431,14 @@ class Declaration : public AstNode {
  public:
   typedef base::ThreadedList<Declaration> List;
 
-  VariableProxy* proxy() const { return proxy_; }
+  Variable* var() const { return var_; }
+  void set_var(Variable* var) { var_ = var; }
 
  protected:
-  Declaration(VariableProxy* proxy, int pos, NodeType type)
-      : AstNode(pos, type), proxy_(proxy), next_(nullptr) {}
+  Declaration(int pos, NodeType type) : AstNode(pos, type), next_(nullptr) {}
 
  private:
-  VariableProxy* proxy_;
+  Variable* var_;
   // Declarations list threaded through the declarations.
   Declaration** next() { return &next_; }
   Declaration* next_;
@@ -457,8 +457,8 @@ class VariableDeclaration : public Declaration {
       : public BitField<bool, Declaration::kNextBitFieldIndex, 1> {};
 
  protected:
-  VariableDeclaration(VariableProxy* proxy, int pos, bool is_nested = false)
-      : Declaration(proxy, pos, kVariableDeclaration) {
+  explicit VariableDeclaration(int pos, bool is_nested = false)
+      : Declaration(pos, kVariableDeclaration) {
     bit_field_ = IsNestedField::update(bit_field_, is_nested);
   }
 
@@ -475,8 +475,8 @@ class NestedVariableDeclaration final : public VariableDeclaration {
  private:
   friend class AstNodeFactory;
 
-  NestedVariableDeclaration(VariableProxy* proxy, Scope* scope, int pos)
-      : VariableDeclaration(proxy, pos, true), scope_(scope) {}
+  NestedVariableDeclaration(Scope* scope, int pos)
+      : VariableDeclaration(pos, true), scope_(scope) {}
 
   // Nested scope from which the declaration originated.
   Scope* scope_;
@@ -501,9 +501,9 @@ class FunctionDeclaration final : public Declaration {
   class DeclaresSloppyBlockFunction
       : public BitField<bool, Declaration::kNextBitFieldIndex, 1> {};
 
-  FunctionDeclaration(VariableProxy* proxy, FunctionLiteral* fun,
-                      bool declares_sloppy_block_function, int pos)
-      : Declaration(proxy, pos, kFunctionDeclaration), fun_(fun) {
+  FunctionDeclaration(FunctionLiteral* fun, bool declares_sloppy_block_function,
+                      int pos)
+      : Declaration(pos, kFunctionDeclaration), fun_(fun) {
     bit_field_ = DeclaresSloppyBlockFunction::update(
         bit_field_, declares_sloppy_block_function);
   }
@@ -2812,22 +2812,19 @@ class AstNodeFactory final {
   AstNodeFactory* ast_node_factory() { return this; }
   AstValueFactory* ast_value_factory() const { return ast_value_factory_; }
 
-  VariableDeclaration* NewVariableDeclaration(VariableProxy* proxy, int pos) {
-    return new (zone_) VariableDeclaration(proxy, pos);
+  VariableDeclaration* NewVariableDeclaration(int pos) {
+    return new (zone_) VariableDeclaration(pos);
   }
 
-  NestedVariableDeclaration* NewNestedVariableDeclaration(VariableProxy* proxy,
-                                                          Scope* scope,
+  NestedVariableDeclaration* NewNestedVariableDeclaration(Scope* scope,
                                                           int pos) {
-    return new (zone_) NestedVariableDeclaration(proxy, scope, pos);
+    return new (zone_) NestedVariableDeclaration(scope, pos);
   }
 
-  FunctionDeclaration* NewFunctionDeclaration(VariableProxy* proxy,
-                                              FunctionLiteral* fun,
+  FunctionDeclaration* NewFunctionDeclaration(FunctionLiteral* fun,
                                               bool is_sloppy_block_function,
                                               int pos) {
-    return new (zone_)
-        FunctionDeclaration(proxy, fun, is_sloppy_block_function, pos);
+    return new (zone_) FunctionDeclaration(fun, is_sloppy_block_function, pos);
   }
 
   Block* NewBlock(int capacity, bool ignore_completion_value) {

src/ast/prettyprinter.cc

@@ -848,15 +848,15 @@ void AstPrinter::VisitBlock(Block* node) {
 
 // TODO(svenpanne) Start with IndentedScope.
 void AstPrinter::VisitVariableDeclaration(VariableDeclaration* node) {
-  PrintLiteralWithModeIndented("VARIABLE", node->proxy()->var(),
-                               node->proxy()->raw_name());
+  PrintLiteralWithModeIndented("VARIABLE", node->var(),
+                               node->var()->raw_name());
 }
 
 
 // TODO(svenpanne) Start with IndentedScope.
 void AstPrinter::VisitFunctionDeclaration(FunctionDeclaration* node) {
   PrintIndented("FUNCTION ");
-  PrintLiteral(node->proxy()->raw_name(), true);
+  PrintLiteral(node->var()->raw_name(), true);
   Print(" = function ");
   PrintLiteral(node->fun()->raw_name(), false);
   Print("\n");

src/ast/scopes.cc

@@ -560,12 +560,11 @@ void DeclarationScope::HoistSloppyBlockFunctions(AstNodeFactory* factory) {
     if (factory) {
       DCHECK(!is_being_lazily_parsed_);
       VariableProxy* proxy = factory->NewVariableProxy(name, NORMAL_VARIABLE);
-      auto declaration =
-          factory->NewVariableDeclaration(proxy, kNoSourcePosition);
+      auto declaration = factory->NewVariableDeclaration(kNoSourcePosition);
       // Based on the preceding checks, it doesn't matter what we pass as
       // sloppy_mode_block_scope_function_redefinition.
       bool ok = true;
-      DeclareVariable(declaration, VariableMode::kVar, NORMAL_VARIABLE,
+      DeclareVariable(declaration, proxy, VariableMode::kVar, NORMAL_VARIABLE,
                       Variable::DefaultInitializationFlag(VariableMode::kVar),
                       nullptr, &ok);
       DCHECK(ok);
@@ -977,8 +976,11 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
   return Declare(zone(), name, mode, kind, init_flag);
 }
 
-void Scope::DeclareVariable(Declaration* declaration, VariableMode mode,
-                            VariableKind kind, InitializationFlag init,
+// TODO(leszeks): Avoid passing the proxy into here, passing the raw_name alone
+// instead.
+void Scope::DeclareVariable(Declaration* declaration, VariableProxy* proxy,
+                            VariableMode mode, VariableKind kind,
+                            InitializationFlag init,
                             bool* sloppy_mode_block_scope_function_redefinition,
                             bool* ok) {
   DCHECK(IsDeclaredVariableMode(mode));
@@ -988,7 +990,7 @@ void Scope::DeclareVariable(Declaration* declaration, VariableMode mode,
 
   if (mode == VariableMode::kVar && !is_declaration_scope()) {
     return GetDeclarationScope()->DeclareVariable(
-        declaration, mode, kind, init,
+        declaration, proxy, mode, kind, init,
         sloppy_mode_block_scope_function_redefinition, ok);
   }
   DCHECK(!is_catch_scope());
@@ -996,7 +998,6 @@ void Scope::DeclareVariable(Declaration* declaration, VariableMode mode,
   DCHECK(is_declaration_scope() ||
          (IsLexicalVariableMode(mode) && is_block_scope()));
 
-  VariableProxy* proxy = declaration->proxy();
   DCHECK_NOT_NULL(proxy->raw_name());
   const AstRawString* name = proxy->raw_name();
 
@@ -1073,6 +1074,7 @@ void Scope::DeclareVariable(Declaration* declaration, VariableMode mode,
   // semantic issue, but it may be a performance issue since it may
   // lead to repeated DeclareEvalVar or DeclareEvalFunction calls.
   decls_.Add(declaration);
+  declaration->set_var(var);
   proxy->BindTo(var);
 }
 
@@ -1160,13 +1162,13 @@ Declaration* Scope::CheckConflictingVarDeclarations() {
     // are lexical vs nested var.
     if (decl->IsVariableDeclaration() &&
         decl->AsVariableDeclaration()->AsNested() != nullptr) {
-      DCHECK_EQ(decl->proxy()->var()->mode(), VariableMode::kVar);
+      DCHECK_EQ(decl->var()->mode(), VariableMode::kVar);
       Scope* current = decl->AsVariableDeclaration()->AsNested()->scope();
       // Iterate through all scopes until and including the declaration scope.
       while (true) {
         // There is a conflict if there exists a non-VAR binding.
         Variable* other_var =
-            current->variables_.Lookup(decl->proxy()->raw_name());
+            current->variables_.Lookup(decl->var()->raw_name());
         if (other_var != nullptr && IsLexicalVariableMode(other_var->mode())) {
           return decl;
         }

src/ast/scopes.h

@@ -226,8 +226,9 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
                          VariableKind kind = NORMAL_VARIABLE,
                          InitializationFlag init_flag = kCreatedInitialized);
 
-  void DeclareVariable(Declaration* declaration, VariableMode mode,
-                       VariableKind kind, InitializationFlag init,
+  void DeclareVariable(Declaration* declaration, VariableProxy* proxy,
+                       VariableMode mode, VariableKind kind,
+                       InitializationFlag init,
                        bool* sloppy_mode_block_scope_function_redefinition,
                        bool* ok);
 

src/interpreter/bytecode-generator.cc

@@ -1220,7 +1220,7 @@ void BytecodeGenerator::VisitBlockDeclarationsAndStatements(Block* stmt) {
 }
 
 void BytecodeGenerator::VisitVariableDeclaration(VariableDeclaration* decl) {
-  Variable* variable = decl->proxy()->var();
+  Variable* variable = decl->var();
   switch (variable->location()) {
     case VariableLocation::UNALLOCATED: {
       DCHECK(!variable->binding_needs_init());
@@ -1271,7 +1271,7 @@ void BytecodeGenerator::VisitVariableDeclaration(VariableDeclaration* decl) {
 }
 
 void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) {
-  Variable* variable = decl->proxy()->var();
+  Variable* variable = decl->var();
   DCHECK(variable->mode() == VariableMode::kLet ||
          variable->mode() == VariableMode::kVar);
   switch (variable->location()) {

src/parsing/parser.cc

@@ -1361,34 +1361,33 @@ void Parser::DeclareVariable(VariableProxy* proxy, VariableKind kind,
   Declaration* declaration;
   if (mode == VariableMode::kVar && !scope->is_declaration_scope()) {
     DCHECK(scope->is_block_scope() || scope->is_with_scope());
-    declaration = factory()->NewNestedVariableDeclaration(proxy, scope, begin);
+    declaration = factory()->NewNestedVariableDeclaration(scope, begin);
   } else {
-    declaration = factory()->NewVariableDeclaration(proxy, begin);
+    declaration = factory()->NewVariableDeclaration(begin);
   }
-  Declare(declaration, kind, mode, init, scope, end);
+  Declare(declaration, proxy, kind, mode, init, scope, end);
 }
 
-void Parser::Declare(Declaration* declaration, VariableKind variable_kind,
-                     VariableMode mode, InitializationFlag init, Scope* scope,
-                     int var_end_pos) {
+void Parser::Declare(Declaration* declaration, VariableProxy* proxy,
+                     VariableKind variable_kind, VariableMode mode,
+                     InitializationFlag init, Scope* scope, int var_end_pos) {
   bool local_ok = true;
   bool sloppy_mode_block_scope_function_redefinition = false;
-  scope->DeclareVariable(declaration, mode, variable_kind, init,
+  scope->DeclareVariable(declaration, proxy, mode, variable_kind, init,
                          &sloppy_mode_block_scope_function_redefinition,
                          &local_ok);
   if (!local_ok) {
     // If we only have the start position of a proxy, we can't highlight the
     // whole variable name.  Pretend its length is 1 so that we highlight at
     // least the first character.
-    Scanner::Location loc(declaration->proxy()->position(),
-                          var_end_pos != kNoSourcePosition
-                              ? var_end_pos
-                              : declaration->proxy()->position() + 1);
+    Scanner::Location loc(proxy->position(), var_end_pos != kNoSourcePosition
+                                                 ? var_end_pos
+                                                 : proxy->position() + 1);
     if (variable_kind == PARAMETER_VARIABLE) {
       ReportMessageAt(loc, MessageTemplate::kParamDupe);
     } else {
       ReportMessageAt(loc, MessageTemplate::kVarRedeclaration,
-                      declaration->proxy()->raw_name());
+                      declaration->var()->raw_name());
     }
   } else if (sloppy_mode_block_scope_function_redefinition) {
     ++use_counts_[v8::Isolate::kSloppyModeBlockScopedFunctionRedefinition];
@@ -1414,8 +1413,9 @@ Statement* Parser::DeclareFunction(const AstRawString* variable_name,
   VariableProxy* proxy =
       factory()->NewVariableProxy(variable_name, NORMAL_VARIABLE, beg_pos);
   Declaration* declaration = factory()->NewFunctionDeclaration(
-      proxy, function, is_sloppy_block_function, beg_pos);
-  Declare(declaration, NORMAL_VARIABLE, mode, kCreatedInitialized, scope());
+      function, is_sloppy_block_function, beg_pos);
+  Declare(declaration, proxy, NORMAL_VARIABLE, mode, kCreatedInitialized,
+          scope());
   if (names) names->Add(variable_name, zone());
   if (is_sloppy_block_function) {
     SloppyBlockFunctionStatement* statement =
@@ -1601,8 +1601,8 @@ Block* Parser::RewriteCatchPattern(CatchInfo* catch_info) {
 
 void Parser::ReportVarRedeclarationIn(const AstRawString* name, Scope* scope) {
   for (Declaration* decl : *scope->declarations()) {
-    if (decl->proxy()->raw_name() == name) {
-      int position = decl->proxy()->position();
+    if (decl->var()->raw_name() == name) {
+      int position = decl->position();
       Scanner::Location location =
           position == kNoSourcePosition
               ? Scanner::Location::invalid()
@@ -2968,8 +2968,8 @@ void Parser::CheckConflictingVarDeclarations(Scope* scope) {
   Declaration* decl = scope->CheckConflictingVarDeclarations();
   if (decl != nullptr) {
     // In ES6, conflicting variable bindings are early errors.
-    const AstRawString* name = decl->proxy()->raw_name();
-    int position = decl->proxy()->position();
+    const AstRawString* name = decl->var()->raw_name();
+    int position = decl->position();
     Scanner::Location location =
         position == kNoSourcePosition
             ? Scanner::Location::invalid()
@@ -2997,11 +2997,11 @@ void Parser::InsertShadowingVarBindingInitializers(Block* inner_block) {
   DCHECK(function_scope->is_function_scope());
   BlockState block_state(&scope_, inner_scope);
   for (Declaration* decl : *inner_scope->declarations()) {
-    if (decl->proxy()->var()->mode() != VariableMode::kVar ||
+    if (decl->var()->mode() != VariableMode::kVar ||
         !decl->IsVariableDeclaration()) {
       continue;
     }
-    const AstRawString* name = decl->proxy()->raw_name();
+    const AstRawString* name = decl->var()->raw_name();
     Variable* parameter = function_scope->LookupLocal(name);
     if (parameter == nullptr) continue;
     VariableProxy* to = NewUnresolved(name);

src/parsing/parser.h

@@ -431,9 +431,9 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
                        VariableMode mode, InitializationFlag init,
                        Scope* declaration_scope, int begin,
                        int end = kNoSourcePosition);
-  void Declare(Declaration* declaration, VariableKind kind, VariableMode mode,
-               InitializationFlag init, Scope* declaration_scope,
-               int var_end_pos = kNoSourcePosition);
+  void Declare(Declaration* declaration, VariableProxy* proxy,
+               VariableKind kind, VariableMode mode, InitializationFlag init,
+               Scope* declaration_scope, int var_end_pos = kNoSourcePosition);
 
   bool TargetStackContainsLabel(const AstRawString* label);
   BreakableStatement* LookupBreakTarget(const AstRawString* label);

test/message/fail/param-arrow-redeclaration-as-let.out

 *%(basename)s:6: SyntaxError: Identifier 'x' has already been declared
   let x
-      ^
+  ^
 SyntaxError: Identifier 'x' has already been declared
 

test/message/fail/param-async-arrow-redeclaration-as-let.out

 *%(basename)s:6: SyntaxError: Identifier 'x' has already been declared
   let x
-      ^
+  ^
 SyntaxError: Identifier 'x' has already been declared
 

test/message/fail/param-async-function-redeclaration-as-let.out

 *%(basename)s:6: SyntaxError: Identifier 'x' has already been declared
   let x
-      ^
+  ^
 SyntaxError: Identifier 'x' has already been declared
 

test/message/fail/param-function-redeclaration-as-let.out

 *%(basename)s:6: SyntaxError: Identifier 'x' has already been declared
   let x
-      ^
+  ^
 SyntaxError: Identifier 'x' has already been declared
 

test/message/fail/try-catch-lexical-conflict-preparser.out

 *%(basename)s:8: SyntaxError: Identifier 'x' has already been declared
     let x
-        ^
+    ^
 SyntaxError: Identifier 'x' has already been declared

test/message/fail/try-catch-lexical-conflict.out

 *%(basename)s:10: SyntaxError: Identifier 'x' has already been declared
   let x;
-      ^
+  ^
 SyntaxError: Identifier 'x' has already been declared

test/message/fail/try-catch-variable-conflict.out

 *%(basename)s:9: SyntaxError: Identifier 'x' has already been declared
   var x;
-      ^
+  ^
 SyntaxError: Identifier 'x' has already been declared

test/message/fail/var-conflict-in-with.out

 *%(basename)s:5: SyntaxError: Identifier 'x' has already been declared
 { { with ({}) var x; } let x; }
-                  ^
+              ^
 SyntaxError: Identifier 'x' has already been declared