[parser] Reuse identifier variable proxy for rewritten declaration

When parsing an identifier as an expression we'll immediately create an
unresolved VariableProxy in the parsing scope. If this variable ends up
becoming a declaration, e.g., due to arrow function parameter, we'll move it
into the function scope for that arrow function. Then to actually create the
declarations we rewrite the "pattern". When we declare the variable, the proxy
is automatically resolved to the variable we create from it. That means it
can't be in the unresolved list anymore.

We tried to remove the unresolved variable. Unfortunately, if there was a
sloppy eval in a parameter context, there's an additional var-block scope
created for the parameter. Rewriting happens in *that* scope. Hence we didn't
always manage to remove the unresolved variable. I suppose as a fix an
additional variable proxy was introduced; since otherwise the implicit
resolution upon declaration would trigger a dcheck in scope resolution later.

This CL removes the initial variable proxy from the correct scope, so it can be
reused for the declaration.

Change-Id: Id917afb177aef076a2947b0fdd03b5393bd29c3f
Reviewed-on: https://chromium-review.googlesource.com/c/1261937Reviewed-by: Igor Sheludko <ishell@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56402}

src/parsing/pattern-rewriter.cc

@@ -181,7 +181,16 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   DCHECK_NOT_NULL(descriptor_);
   DCHECK_NOT_NULL(ok_);
 
-  descriptor_->scope->RemoveUnresolved(pattern);
+  Scope* outer_function_scope = nullptr;
+  bool success;
+  if (DeclaresParameterContainingSloppyEval()) {
+    outer_function_scope = scope()->outer_scope();
+    success = outer_function_scope->RemoveUnresolved(pattern);
+  } else {
+    success = scope()->RemoveUnresolved(pattern);
+  }
+  USE(success);
+  DCHECK(success);
 
   // Declare variable.
   // Note that we *always* must treat the initial value via a separate init
@@ -192,15 +201,13 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // an initial value in the declaration (because they are initialized upon
   // entering the function).
   const AstRawString* name = pattern->raw_name();
-  VariableProxy* proxy =
-      factory()->NewVariableProxy(name, NORMAL_VARIABLE, pattern->position());
+  VariableProxy* proxy = pattern;
   Declaration* declaration;
   if (descriptor_->mode == VariableMode::kVar &&
-      !descriptor_->scope->is_declaration_scope()) {
-    DCHECK(descriptor_->scope->is_block_scope() ||
-           descriptor_->scope->is_with_scope());
+      !scope()->is_declaration_scope()) {
+    DCHECK(scope()->is_block_scope() || scope()->is_with_scope());
     declaration = factory()->NewNestedVariableDeclaration(
-        proxy, descriptor_->scope, descriptor_->declaration_pos);
+        proxy, scope(), descriptor_->declaration_pos);
   } else {
     declaration =
         factory()->NewVariableDeclaration(proxy, descriptor_->declaration_pos);
@@ -210,10 +217,6 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // 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.
-  Scope* outer_function_scope = nullptr;
-  if (DeclaresParameterContainingSloppyEval()) {
-    outer_function_scope = descriptor_->scope->outer_scope();
-  }
   Variable* var = parser_->Declare(
       declaration, descriptor_->declaration_kind, descriptor_->mode,
       Variable::DefaultInitializationFlag(descriptor_->mode), ok_,
@@ -224,12 +227,11 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   DCHECK_NE(initializer_position_, kNoSourcePosition);
   var->set_initializer_position(initializer_position_);
 
-  Scope* declaration_scope =
-      outer_function_scope != nullptr
+  Scope* declaration_scope = outer_function_scope != nullptr
                                  ? outer_function_scope
                                  : (IsLexicalVariableMode(descriptor_->mode)
-                 ? descriptor_->scope
-                 : descriptor_->scope->GetDeclarationScope());
+                                        ? scope()
+                                        : scope()->GetDeclarationScope());
   if (declaration_scope->num_var() > kMaxNumFunctionLocals) {
     parser_->ReportMessage(MessageTemplate::kTooManyVariables);
     *ok_ = false;
@@ -242,7 +244,7 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   // If there's no initializer, we're done.
   if (value == nullptr) return;
 
-  Scope* var_init_scope = descriptor_->scope;
+  Scope* var_init_scope = scope();
   Parser::MarkLoopVariableAsAssigned(var_init_scope, proxy->var(),
                                      descriptor_->declaration_kind);