[parser] Move MarkLoopVariableAsAssigned to 'var' proxy allocation

This unifies the code between parser and preparser, and removes more code from
the pattern rewriter.

This makes "var x" without assignment and initializer in a loop pessimistically
marked as assigned, but that seems pretty unlikely since the variable will just
always be undefined. It is also still strictly better than what we had until
very recently since any var outside of the function scope used to be marked as
assigned. Now we only mark such variables as assigned.

Change-Id: Icb37ab249b2a79c2d57a5769bdb964b435cebf62
Reviewed-on: https://chromium-review.googlesource.com/c/1405228
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58736}
diff --git a/src/parsing/expression-scope.h b/src/parsing/expression-scope.h
index 79fe7b4..19c719b 100644
--- a/src/parsing/expression-scope.h
+++ b/src/parsing/expression-scope.h
@@ -42,7 +42,28 @@
   VariableProxy* NewVariable(const AstRawString* name,
                              int pos = kNoSourcePosition) {
     VariableProxy* result = parser_->NewRawVariable(name, pos);
-    if (CanBeExpression()) AsExpressionParsingScope()->TrackVariable(result);
+    if (CanBeExpression()) {
+      AsExpressionParsingScope()->TrackVariable(result);
+    } else if (type_ == kVarDeclaration && parser_->loop_nesting_depth() > 0) {
+      // Due to hoisting, the value of a 'var'-declared variable may actually
+      // change even if the code contains only the "initial" assignment, namely
+      // when that assignment occurs inside a loop.  For example:
+      //
+      //   let i = 10;
+      //   do { var x = i } while (i--):
+      //
+      // Note that non-lexical variables include temporaries, which may also get
+      // assigned inside a loop due to the various rewritings that the parser
+      // performs.
+      //
+      // Pessimistically mark all vars in loops as assigned. This
+      // overapproximates the actual assigned vars due to unassigned var without
+      // initializer, but that's unlikely anyway.
+      //
+      // This also handles marking of loop variables in for-in and for-of loops,
+      // as determined by loop-nesting-depth.
+      result->set_is_assigned();
+    }
     return result;
   }
 
diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
index 352fbef..11119d2 100644
--- a/src/parsing/parser-base.h
+++ b/src/parsing/parser-base.h
@@ -315,6 +315,10 @@
     return default_eager_compile_hint_;
   }
 
+  int loop_nesting_depth() const {
+    return function_state_->loop_nesting_depth();
+  }
+
   int GetNextFunctionLiteralId() { return ++function_literal_id_; }
   int GetLastFunctionLiteralId() const { return function_literal_id_; }
 
@@ -1162,21 +1166,6 @@
     return true;
   }
 
-  // Due to hoisting, the value of a 'var'-declared variable may actually change
-  // even if the code contains only the "initial" assignment, namely when that
-  // assignment occurs inside a loop.  For example:
-  //
-  //   let i = 10;
-  //   do { var x = i } while (i--):
-  //
-  // Note that non-lexical variables include temporaries, which may also get
-  // assigned inside a loop due to the various rewritings that the parser
-  // performs.
-  //
-  // This also handles marking of loop variables in for-in and for-of loops,
-  // as determined by loop-nesting-depth.
-  void MarkLoopVariableAsAssigned(Variable* var);
-
   FunctionKind FunctionKindForImpl(bool is_method, ParseFunctionFlags flags) {
     static const FunctionKind kFunctionKinds[][2][2] = {
         {
@@ -5538,14 +5527,6 @@
 }
 
 template <typename Impl>
-void ParserBase<Impl>::MarkLoopVariableAsAssigned(Variable* var) {
-  if (!IsLexicalVariableMode(var->mode()) &&
-      function_state_->loop_nesting_depth() > 0) {
-    var->set_maybe_assigned();
-  }
-}
-
-template <typename Impl>
 typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
     ZonePtrList<const AstRawString>* labels,
     ZonePtrList<const AstRawString>* own_labels) {
diff --git a/src/parsing/pattern-rewriter.cc b/src/parsing/pattern-rewriter.cc
index 4f8c465..8a22ba4 100644
--- a/src/parsing/pattern-rewriter.cc
+++ b/src/parsing/pattern-rewriter.cc
@@ -212,11 +212,6 @@
   if (names_) {
     names_->Add(proxy->raw_name(), zone());
   }
-
-  // If there's no initializer, we're done.
-  if (!has_initializer_) return;
-
-  parser_->MarkLoopVariableAsAssigned(proxy->var());
 }
 
 // When an extra declaration scope needs to be inserted to account for
diff --git a/src/parsing/preparser.cc b/src/parsing/preparser.cc
index 445022a..6c22e59 100644
--- a/src/parsing/preparser.cc
+++ b/src/parsing/preparser.cc
@@ -425,7 +425,6 @@
         ReportUnidentifiableError();
         return;
       }
-      MarkLoopVariableAsAssigned(var);
       // This is only necessary if there is an initializer, but we don't have
       // that information here.  Consequently, the preparser sometimes says
       // maybe-assigned where the parser (correctly) says never-assigned.