Widen --harmony-for-in flag to throw errors in PreParser

The first version of --harmony-for-in avoided throwing PreParser
errors in order to retain use counting. This patch threads
use_counts_ through to the PreParser to allow use counting in
the PreParser while also throwing errors for this case.

Also slightly refactored the Parser code to do a little less
code duplication.

BUG=v8:4942
LOG=y

Review URL: https://codereview.chromium.org/1913203002

Cr-Commit-Position: refs/heads/master@{#35780}

src/parsing/parser-base.h

@@ -114,6 +114,7 @@ class ParserBase : public Traits {
         allow_tailcalls_(false),
         allow_harmony_restrictive_declarations_(false),
         allow_harmony_do_expressions_(false),
+        allow_harmony_for_in_(false),
         allow_harmony_function_name_(false),
         allow_harmony_function_sent_(false) {}
 
@@ -132,6 +133,7 @@ class ParserBase : public Traits {
   ALLOW_ACCESSORS(tailcalls);
   ALLOW_ACCESSORS(harmony_restrictive_declarations);
   ALLOW_ACCESSORS(harmony_do_expressions);
+  ALLOW_ACCESSORS(harmony_for_in);
   ALLOW_ACCESSORS(harmony_function_name);
   ALLOW_ACCESSORS(harmony_function_sent);
   SCANNER_ACCESSORS(harmony_exponentiation_operator);
@@ -923,6 +925,7 @@ class ParserBase : public Traits {
   bool allow_tailcalls_;
   bool allow_harmony_restrictive_declarations_;
   bool allow_harmony_do_expressions_;
+  bool allow_harmony_for_in_;
   bool allow_harmony_function_name_;
   bool allow_harmony_function_sent_;
 };

src/parsing/parser.cc

@@ -785,6 +785,7 @@ Parser::Parser(ParseInfo* info)
   set_allow_tailcalls(FLAG_harmony_tailcalls && !info->is_native() &&
                       info->isolate()->is_tail_call_elimination_enabled());
   set_allow_harmony_do_expressions(FLAG_harmony_do_expressions);
+  set_allow_harmony_for_in(FLAG_harmony_for_in);
   set_allow_harmony_function_name(FLAG_harmony_function_name);
   set_allow_harmony_function_sent(FLAG_harmony_function_sent);
   set_allow_harmony_restrictive_declarations(
@@ -3478,7 +3479,12 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         if (parsing_result.first_initializer_loc.IsValid() &&
             (is_strict(language_mode()) || mode == ForEachStatement::ITERATE ||
              IsLexicalVariableMode(parsing_result.descriptor.mode) ||
-             !decl.pattern->IsVariableProxy())) {
+             !decl.pattern->IsVariableProxy() || allow_harmony_for_in())) {
+          // Only increment the use count if we would have let this through
+          // without the flag.
+          if (allow_harmony_for_in()) {
+            ++use_counts_[v8::Isolate::kForInInitializer];
+          }
           ParserTraits::ReportMessageAt(
               parsing_result.first_initializer_loc,
               MessageTemplate::kForInOfLoopInitializer,
@@ -3492,19 +3498,8 @@ Statement* Parser::ParseForStatement(ZoneList<const AstRawString*>* labels,
         // special case for legacy for (var/const x =.... in)
         if (!IsLexicalVariableMode(parsing_result.descriptor.mode) &&
             decl.pattern->IsVariableProxy() && decl.initializer != nullptr) {
+          DCHECK(!allow_harmony_for_in());
           ++use_counts_[v8::Isolate::kForInInitializer];
-          if (FLAG_harmony_for_in) {
-            // TODO(rossberg): This error is not currently generated in the
-            // preparser, because that would lose some of the use counts
-            // recorded above. Once either the use counter or the flag is
-            // removed, the preparser should be adjusted.
-            ParserTraits::ReportMessageAt(
-                parsing_result.first_initializer_loc,
-                MessageTemplate::kForInOfLoopInitializer,
-                ForEachStatement::VisitModeString(mode));
-            *ok = false;
-            return nullptr;
-          }
           const AstRawString* name =
               decl.pattern->AsVariableProxy()->raw_name();
           VariableProxy* single_var = scope_->NewUnresolved(
@@ -4602,6 +4597,7 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
 #define SET_ALLOW(name) reusable_preparser_->set_allow_##name(allow_##name());
     SET_ALLOW(natives);
     SET_ALLOW(harmony_do_expressions);
+    SET_ALLOW(harmony_for_in);
     SET_ALLOW(harmony_function_name);
     SET_ALLOW(harmony_function_sent);
     SET_ALLOW(harmony_exponentiation_operator);
@@ -4610,7 +4606,7 @@ PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
   }
   PreParser::PreParseResult result = reusable_preparser_->PreParseLazyFunction(
       language_mode(), function_state_->kind(), scope_->has_simple_parameters(),
-      logger, bookmark);
+      logger, bookmark, use_counts_);
   if (pre_parse_timer_ != NULL) {
     pre_parse_timer_->Stop();
   }

src/parsing/preparser.cc

@@ -98,11 +98,11 @@ PreParserExpression PreParserTraits::ParseFunctionLiteral(
       function_token_position, type, language_mode, ok);
 }
 
-
 PreParser::PreParseResult PreParser::PreParseLazyFunction(
     LanguageMode language_mode, FunctionKind kind, bool has_simple_parameters,
-    ParserRecorder* log, Scanner::BookmarkScope* bookmark) {
+    ParserRecorder* log, Scanner::BookmarkScope* bookmark, int* use_counts) {
   log_ = log;
+  use_counts_ = use_counts;
   // Lazy functions always have trivial outer scopes (no with/catch scopes).
   Scope* top_scope = NewScope(scope_, SCRIPT_SCOPE);
   PreParserFactory top_factory(NULL);
@@ -118,6 +118,7 @@ PreParser::PreParseResult PreParser::PreParseLazyFunction(
   bool ok = true;
   int start_position = peek_position();
   ParseLazyFunctionLiteralBody(&ok, bookmark);
+  use_counts_ = nullptr;
   if (bookmark && bookmark->HasBeenReset()) {
     // Do nothing, as we've just aborted scanning this function.
   } else if (stack_overflow()) {
@@ -811,7 +812,12 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
         }
         if (first_initializer_loc.IsValid() &&
             (is_strict(language_mode()) || mode == ForEachStatement::ITERATE ||
-             is_lexical || is_binding_pattern)) {
+             is_lexical || is_binding_pattern || allow_harmony_for_in())) {
+          // Only increment the use count if we would have let this through
+          // without the flag.
+          if (use_counts_ != nullptr && allow_harmony_for_in()) {
+            ++use_counts_[v8::Isolate::kForInInitializer];
+          }
           PreParserTraits::ReportMessageAt(
               first_initializer_loc, MessageTemplate::kForInOfLoopInitializer,
               ForEachStatement::VisitModeString(mode));

src/parsing/preparser.h

@@ -963,7 +963,8 @@ class PreParser : public ParserBase<PreParserTraits> {
   PreParser(Zone* zone, Scanner* scanner, AstValueFactory* ast_value_factory,
             ParserRecorder* log, uintptr_t stack_limit)
       : ParserBase<PreParserTraits>(zone, scanner, stack_limit, NULL,
-                                    ast_value_factory, log, this) {}
+                                    ast_value_factory, log, this),
+        use_counts_(nullptr) {}
 
   // Pre-parse the program from the character stream; returns true on
   // success (even if parsing failed, the pre-parse data successfully
@@ -1001,7 +1002,7 @@ class PreParser : public ParserBase<PreParserTraits> {
   // the final '}'.
   PreParseResult PreParseLazyFunction(
       LanguageMode language_mode, FunctionKind kind, bool has_simple_parameters,
-      ParserRecorder* log, Scanner::BookmarkScope* bookmark = nullptr);
+      ParserRecorder* log, Scanner::BookmarkScope* bookmark, int* use_counts);
 
  private:
   friend class PreParserTraits;
@@ -1074,6 +1075,8 @@ class PreParser : public ParserBase<PreParserTraits> {
                                         Scanner::Location class_name_location,
                                         bool name_is_strict_reserved, int pos,
                                         bool* ok);
+
+  int* use_counts_;
 };
 
 

test/cctest/test-parsing.cc

@@ -1508,7 +1508,8 @@ enum ParserFlag {
   kAllowHarmonyNewTarget,
   kAllowHarmonyFunctionSent,
   kAllowHarmonyRestrictiveDeclarations,
-  kAllowHarmonyExponentiationOperator
+  kAllowHarmonyExponentiationOperator,
+  kAllowHarmonyForIn
 };
 
 enum ParserSyncTestResult {
@@ -1528,6 +1529,7 @@ void SetParserFlags(i::ParserBase<Traits>* parser,
       flags.Contains(kAllowHarmonyRestrictiveDeclarations));
   parser->set_allow_harmony_exponentiation_operator(
       flags.Contains(kAllowHarmonyExponentiationOperator));
+  parser->set_allow_harmony_for_in(flags.Contains(kAllowHarmonyForIn));
 }
 
 
@@ -7267,3 +7269,23 @@ TEST(ExponentiationOperatorErrors) {
   RunParserSyncTest(context_data, error_data, kError, NULL, 0, always_flags,
                     arraysize(always_flags));
 }
+
+TEST(RestrictiveForInErrors) {
+  // clang-format off
+  const char* context_data[][2] = {
+    { "'use strict'", "" },
+    { "", "" },
+    { NULL, NULL }
+  };
+  const char* error_data[] = {
+    "for (var x = 0 in {});",
+    "for (const x = 0 in {});",
+    "for (let x = 0 in {});",
+    NULL
+  };
+  // clang-format on
+
+  static const ParserFlag always_flags[] = {kAllowHarmonyForIn};
+  RunParserSyncTest(context_data, error_data, kError, nullptr, 0, always_flags,
+                    arraysize(always_flags));
+}