[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/1405228Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58736}

src/parsing/expression-scope.h

@@ -42,7 +42,28 @@ class ExpressionScope {
   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;
   }
 

src/parsing/parser-base.h

@@ -315,6 +315,10 @@ class ParserBase {
     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 @@ class ParserBase {
     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] = {
         {
@@ -5537,14 +5526,6 @@ typename ParserBase<Impl>::ForStatementT ParserBase<Impl>::ParseStandardForLoop(
   return loop;
 }
 
-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,

src/parsing/pattern-rewriter.cc

@@ -212,11 +212,6 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* proxy) {
   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

src/parsing/preparser.cc

@@ -425,7 +425,6 @@ void PreParser::InitializeVariables(
         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.