[parser] Ensure for-in/of loop variables are marked maybe_assigned

The code used to rely on all such loops having a block scope around
them, but that is no longer the case for loops whose loop variables
are VAR-declared.

This patch introduces a new DeclarationDescriptor::Kind for such
variables, and sets it during parsing, allowing the variable
declaration code to note them as assigned appropriately.

Bug: chromium:768158
Change-Id: I0cd60e8c8c735681be9dbb9344a93156af09c952
Reviewed-on: https://chromium-review.googlesource.com/701624Reviewed-by: Marja Hölttä <marja@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48320}

src/parsing/parser-base.h

@@ -486,7 +486,7 @@ class ParserBase {
   };
 
   struct DeclarationDescriptor {
-    enum Kind { NORMAL, PARAMETER };
+    enum Kind { NORMAL, PARAMETER, FOR_EACH };
     Scope* scope;
     VariableMode mode;
     int declaration_pos;
@@ -1285,7 +1285,12 @@ class ParserBase {
   // assigned inside a loop due to the various rewritings that the parser
   // performs.
   //
-  static void MarkLoopVariableAsAssigned(Scope* scope, Variable* var);
+  // This also handles marking of loop variables in for-in and for-of loops,
+  // as determined by declaration_kind.
+  //
+  static void MarkLoopVariableAsAssigned(
+      Scope* scope, Variable* var,
+      typename DeclarationDescriptor::Kind declaration_kind);
 
   FunctionKind FunctionKindForImpl(bool is_method, bool is_generator,
                                    bool is_async) {
@@ -5657,6 +5662,10 @@ ParserBase<Impl>::ParseForEachStatementWithDeclarations(
     return impl()->NullStatement();
   }
 
+  // Reset the declaration_kind to ensure proper processing during declaration.
+  for_info->parsing_result.descriptor.declaration_kind =
+      DeclarationDescriptor::FOR_EACH;
+
   BlockT init_block = impl()->RewriteForVarInLegacy(*for_info);
 
   auto loop = factory()->NewForEachStatement(for_info->mode, labels, stmt_pos);
@@ -5846,8 +5855,12 @@ typename ParserBase<Impl>::ForStatementT ParserBase<Impl>::ParseStandardForLoop(
 }
 
 template <typename Impl>
-void ParserBase<Impl>::MarkLoopVariableAsAssigned(Scope* scope, Variable* var) {
-  if (!IsLexicalVariableMode(var->mode()) && !scope->is_function_scope()) {
+void ParserBase<Impl>::MarkLoopVariableAsAssigned(
+    Scope* scope, Variable* var,
+    typename DeclarationDescriptor::Kind declaration_kind) {
+  if (!IsLexicalVariableMode(var->mode()) &&
+      (!scope->is_function_scope() ||
+       declaration_kind == DeclarationDescriptor::FOR_EACH)) {
     var->set_maybe_assigned();
   }
 }

src/parsing/parser.cc

@@ -1399,11 +1399,11 @@ Variable* Parser::Declare(Declaration* declaration,
                           var_end_pos != kNoSourcePosition
                               ? var_end_pos
                               : declaration->proxy()->position() + 1);
-    if (declaration_kind == DeclarationDescriptor::NORMAL) {
+    if (declaration_kind == DeclarationDescriptor::PARAMETER) {
+      ReportMessageAt(loc, MessageTemplate::kParamDupe);
+    } else {
       ReportMessageAt(loc, MessageTemplate::kVarRedeclaration,
                       declaration->proxy()->raw_name());
-    } else {
-      ReportMessageAt(loc, MessageTemplate::kParamDupe);
     }
     return nullptr;
   }

src/parsing/pattern-rewriter.cc

@@ -242,7 +242,8 @@ void PatternRewriter::VisitVariableProxy(VariableProxy* pattern) {
   if (value == nullptr) return;
 
   Scope* var_init_scope = descriptor_->scope;
-  Parser::MarkLoopVariableAsAssigned(var_init_scope, proxy->var());
+  Parser::MarkLoopVariableAsAssigned(var_init_scope, proxy->var(),
+                                     descriptor_->declaration_kind);
 
   // A declaration of the form:
   //

src/parsing/preparser.cc

@@ -418,7 +418,8 @@ void PreParser::DeclareAndInitializeVariables(
       Variable* var = scope()->DeclareVariableName(
           variable->raw_name(), declaration_descriptor->mode);
       if (FLAG_preparser_scope_analysis) {
-        MarkLoopVariableAsAssigned(declaration_descriptor->scope, var);
+        MarkLoopVariableAsAssigned(declaration_descriptor->scope, var,
+                                   declaration_descriptor->declaration_kind);
         // 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.

test/mjsunit/regress/regress-crbug-768158.js

+// Copyright 2017 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Flags: --always-opt
+
+(function testOriginalRepro() {
+  var result;
+  var dict = { toString() { result = v;} };
+  for (var v of ['fontsize', 'sup']) {
+    String.prototype[v].call(dict);
+    assertEquals(v, result);
+  }
+})();
+
+(function testSimpler() {
+  var result;
+  function setResult() { result = v; }
+  for (var v of ['hello', 'world']) {
+    setResult();
+    assertEquals(v, result);
+  }
+})();