[parser] Clean up Parser::DeclareVariable and use in the pattern rewriter

Change-Id: I9446a73bb47b11e2d161a4678638b7618ce52b9a
Reviewed-on: https://chromium-review.googlesource.com/c/1387490Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58427}

src/ast/scopes.cc

@@ -1028,9 +1028,10 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
   return Declare(zone(), name, mode, NORMAL_VARIABLE, init_flag);
 }
 
-Variable* Scope::DeclareVariable(
-    Declaration* declaration, VariableMode mode, InitializationFlag init,
-    bool* sloppy_mode_block_scope_function_redefinition, bool* ok) {
+void Scope::DeclareVariable(Declaration* declaration, VariableMode mode,
+                            InitializationFlag init,
+                            bool* sloppy_mode_block_scope_function_redefinition,
+                            bool* ok) {
   DCHECK(IsDeclaredVariableMode(mode));
   DCHECK(!already_resolved_);
   DCHECK(!GetDeclarationScope()->is_being_lazily_parsed());
@@ -1124,7 +1125,6 @@ Variable* Scope::DeclareVariable(
   // lead to repeated DeclareEvalVar or DeclareEvalFunction calls.
   decls_.Add(declaration);
   proxy->BindTo(var);
-  return var;
 }
 
 Variable* Scope::DeclareVariableName(const AstRawString* name,

src/ast/scopes.h

@@ -233,10 +233,10 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   Variable* DeclareLocal(const AstRawString* name, VariableMode mode,
                          InitializationFlag init_flag = kCreatedInitialized);
 
-  Variable* DeclareVariable(Declaration* declaration, VariableMode mode,
-                            InitializationFlag init,
-                            bool* sloppy_mode_block_scope_function_redefinition,
-                            bool* ok);
+  void DeclareVariable(Declaration* declaration, VariableMode mode,
+                       InitializationFlag init,
+                       bool* sloppy_mode_block_scope_function_redefinition,
+                       bool* ok);
 
   // The return value is meaningful only if FLAG_preparser_scope_analysis is on.
   Variable* DeclareVariableName(const AstRawString* name, VariableMode mode);

src/parsing/parser-base.h

@@ -5358,7 +5358,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
     // The initializer contains lexical declarations,
     // so create an in-between scope.
     BlockState for_state(zone(), &scope_);
-    scope()->set_start_position(scanner()->location().beg_pos);
+    scope()->set_start_position(position());
 
     // Also record whether inner functions or evals are found inside
     // this loop, as this information is used to simplify the desugaring
@@ -5375,7 +5375,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
                                 nullptr);
     }
     DCHECK(IsLexicalVariableMode(for_info.parsing_result.descriptor.mode));
-    for_info.position = scanner()->location().beg_pos;
+    for_info.position = position();
 
     if (CheckInOrOf(&for_info.mode)) {
       scope()->set_is_hidden();

src/parsing/parser.h

@@ -440,15 +440,18 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
   VariableProxy* NewUnresolved(const AstRawString* name, int begin_pos,
                                VariableKind kind = NORMAL_VARIABLE);
   VariableProxy* NewUnresolved(const AstRawString* name);
-  Variable* Declare(Declaration* declaration,
-                    DeclarationDescriptor::Kind declaration_kind,
-                    VariableMode mode, InitializationFlag init,
-                    Scope* declaration_scope = nullptr,
-                    int var_end_pos = kNoSourcePosition);
-  Declaration* DeclareVariable(const AstRawString* name, VariableMode mode,
-                               int pos);
-  Declaration* DeclareVariable(const AstRawString* name, VariableMode mode,
-                               InitializationFlag init, int pos);
+  VariableProxy* DeclareVariable(const AstRawString* name, VariableMode mode,
+                                 int pos);
+  VariableProxy* DeclareVariable(const AstRawString* name, VariableMode mode,
+                                 InitializationFlag init, int pos);
+  void DeclareVariable(VariableProxy* proxy, DeclarationDescriptor::Kind,
+                       VariableMode mode, InitializationFlag init,
+                       Scope* declaration_scope, int begin,
+                       int end = kNoSourcePosition);
+  void Declare(Declaration* declaration,
+               DeclarationDescriptor::Kind declaration_kind, VariableMode mode,
+               InitializationFlag init, Scope* declaration_scope,
+               int var_end_pos = kNoSourcePosition);
 
   bool TargetStackContainsLabel(const AstRawString* label);
   BreakableStatement* LookupBreakTarget(const AstRawString* label);

src/parsing/pattern-rewriter.cc

@@ -183,15 +183,15 @@ Expression* PatternRewriter::RewriteDestructuringAssignment(
   return rewriter.Rewrite(to_rewrite);
 }
 
-void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
+void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
   Expression* value = current_value_;
 
   if (IsAssignmentContext()) {
     // In an assignment context, simply perform the assignment
-    Assignment* assignment = factory()->NewAssignment(
-        Token::ASSIGN, pattern, value, pattern->position());
+    Assignment* assignment = factory()->NewAssignment(Token::ASSIGN, proxy,
+                                                      value, proxy->position());
     block_->statements()->Add(
-        factory()->NewExpressionStatement(assignment, pattern->position()),
+        factory()->NewExpressionStatement(assignment, proxy->position()),
         zone());
     return;
   }
@@ -199,12 +199,15 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   DCHECK_NOT_NULL(block_);
   DCHECK_NOT_NULL(descriptor_);
 
-  Scope* outer_function_scope = nullptr;
+  // scope() isn't guaranteed to be the same as parser_->scope(). scope() is
+  // where the pattern was parsed, whereas parser_->scope() is where the pattern
+  // is rewritten into as a declaration.
+  Scope* target_scope = parser_->scope();
   if (declares_parameter_containing_sloppy_eval_) {
-    outer_function_scope = scope()->outer_scope();
-    outer_function_scope->DeleteUnresolved(pattern);
+    target_scope = scope()->outer_scope();
+    target_scope->DeleteUnresolved(proxy);
   } else {
-    scope()->DeleteUnresolved(pattern);
+    scope()->DeleteUnresolved(proxy);
   }
 
   // Declare variable.
@@ -215,44 +218,34 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // which the variable or constant is declared. Only function variables have
   // an initial value in the declaration (because they are initialized upon
   // entering the function).
-  const AstRawString* name = pattern->raw_name();
-  VariableProxy* proxy = pattern;
-  Declaration* declaration;
-  if (descriptor_->mode == VariableMode::kVar &&
-      !scope()->is_declaration_scope()) {
-    DCHECK(scope()->is_block_scope() || scope()->is_with_scope());
-    declaration = factory()->NewNestedVariableDeclaration(
-        proxy, scope(), descriptor_->declaration_pos);
-  } else {
-    declaration =
-        factory()->NewVariableDeclaration(proxy, descriptor_->declaration_pos);
-  }
-
   // When an extra declaration scope needs to be inserted to account for
   // a sloppy eval in a default parameter or function body, the parameter
   // needs to be declared in the function's scope, not in the varblock
   // scope which will be used for the initializer expression.
-  Variable* var = parser_->Declare(
-      declaration, descriptor_->declaration_kind, descriptor_->mode,
-      Variable::DefaultInitializationFlag(descriptor_->mode),
-      outer_function_scope);
+  parser_->DeclareVariable(
+      proxy, descriptor_->declaration_kind, descriptor_->mode,
+      Variable::DefaultInitializationFlag(descriptor_->mode), target_scope,
+      descriptor_->declaration_pos);
+
   if (parser_->has_error()) return;
+  Variable* var = proxy->var();
   DCHECK_NOT_NULL(var);
   DCHECK(proxy->is_resolved());
   DCHECK_NE(initializer_position_, kNoSourcePosition);
   var->set_initializer_position(initializer_position_);
-
-  Scope* declaration_scope = outer_function_scope != nullptr
-                                 ? outer_function_scope
-                                 : (IsLexicalVariableMode(descriptor_->mode)
-                                        ? scope()
-                                        : scope()->GetDeclarationScope());
+  Scope* declaration_scope = var->scope();
+  DCHECK_EQ(declaration_scope,
+            declares_parameter_containing_sloppy_eval_
+                ? scope()->outer_scope()
+                : (IsLexicalVariableMode(descriptor_->mode)
+                       ? parser_->scope()
+                       : parser_->scope()->GetDeclarationScope()));
   if (declaration_scope->num_var() > kMaxNumFunctionLocals) {
     parser_->ReportMessage(MessageTemplate::kTooManyVariables);
     return;
   }
   if (names_) {
-    names_->Add(name, zone());
+    names_->Add(proxy->raw_name(), zone());
   }
 
   // If there's no initializer, we're done.
@@ -279,7 +272,7 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // scope we need to do a new lookup.
   if (descriptor_->mode == VariableMode::kVar &&
       var_init_scope != declaration_scope) {
-    proxy = var_init_scope->NewUnresolved(factory(), name);
+    proxy = var_init_scope->NewUnresolved(factory(), proxy->raw_name());
   } else {
     DCHECK_NOT_NULL(proxy);
     DCHECK_NOT_NULL(proxy->var());
@@ -287,7 +280,7 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // Add break location for destructured sub-pattern.
   int pos = value_beg_position_;
   if (pos == kNoSourcePosition) {
-    pos = IsSubPattern() ? pattern->position() : value->position();
+    pos = IsSubPattern() ? proxy->position() : value->position();
   }
   Assignment* assignment =
       factory()->NewAssignment(Token::INIT, proxy, value, pos);