[parser] Allow try {} catch (e) { for (var e of x) {} }

This patch changes the parser to allow for-of initializer
var-redeclaration of non-destructured catch parameters.

Previously, the spec allowed var-redeclaration of a
non-destructured catch parameter…

    try {} catch (e) { var e; }

…except in the particular case where the var declaration is
a for-of initializer:

    try {} catch (e) { for (var e of whatever) {} }

https://github.com/tc39/ecma262/pull/1393 removes this strange
exceptional case. This patch implements that change.

BUG=v8:8759

Change-Id: Ia4e33ac1eab89085f8a5fdb547f479cfa38bbee5
Reviewed-on: https://chromium-review.googlesource.com/c/1444954
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Mathias Bynens <mathias@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59209}

src/parsing/parser-base.h

@@ -1183,40 +1183,6 @@ class ParserBase {
     return identifier == ast_value_factory()->let_string();
   }
 
-  void DesugarBindingInForEachStatement(ForInfo* for_info, BlockT* body_block,
-                                        ExpressionT* each_variable) {
-    // Annex B.3.5 prohibits the form
-    // `try {} catch(e) { for (var e of {}); }`
-    // So if we are parsing a statement like `for (var ... of ...)`
-    // we need to walk up the scope chain and look for catch scopes
-    // which have a simple binding, then compare their binding against
-    // all of the names declared in the init of the for-of we're
-    // parsing.
-    bool is_for_var_of =
-        for_info->mode == ForEachStatement::ITERATE &&
-        for_info->parsing_result.descriptor.mode == VariableMode::kVar;
-
-    if (is_for_var_of) {
-      Scope* scope = this->scope();
-      while (!scope->is_declaration_scope() ||
-             (scope->is_eval_scope() && is_sloppy(scope->language_mode()))) {
-        if (scope->is_catch_scope()) {
-          auto name = scope->catch_variable()->raw_name();
-          // If it's a simple binding and the name is declared in the for loop.
-          if (name != ast_value_factory()->dot_catch_string() &&
-              for_info->bound_names.Contains(name)) {
-            impl()->ReportMessageAt(for_info->parsing_result.bindings_loc,
-                                    MessageTemplate::kVarRedeclaration, name);
-          }
-        }
-        scope = scope->outer_scope();
-      }
-    }
-
-    impl()->DesugarBindingInForEachStatement(for_info, body_block,
-                                             each_variable);
-  }
-
   bool IsNextLetKeyword();
 
   // Checks if the expression is a valid reference expression (e.g., on the
@@ -5531,7 +5497,8 @@ ParserBase<Impl>::ParseForEachStatementWithDeclarations(
     }
     impl()->RecordIterationStatementSourceRange(loop, body_range);
 
-    DesugarBindingInForEachStatement(for_info, &body_block, &each_variable);
+    impl()->DesugarBindingInForEachStatement(for_info, &body_block,
+                                             &each_variable);
     body_block->statements()->Add(body, zone());
 
     if (IsLexicalVariableMode(for_info->parsing_result.descriptor.mode)) {
@@ -5787,7 +5754,8 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
 
     if (has_declarations) {
       BlockT body_block = impl()->NullBlock();
-      DesugarBindingInForEachStatement(&for_info, &body_block, &each_variable);
+      impl()->DesugarBindingInForEachStatement(&for_info, &body_block,
+                                               &each_variable);
       body_block->statements()->Add(body, zone());
       body_block->set_scope(scope()->FinalizeBlockScope());
       body = body_block;

test/mjsunit/es6/for-each-in-catch.js

@@ -5,25 +5,25 @@
 function checkIsRedeclarationError(code) {
   try {
     eval(`
-checkIsRedeclarationError : {
-  break checkIsRedeclarationError;
-${code}
-}
-`);
+      checkIsRedeclarationError: {
+        break checkIsRedeclarationError;
+        ${code}
+      }
+    `);
     assertUnreachable();
-  } catch(e) {
-    assertInstanceof(e, SyntaxError );
-    assertTrue( e.toString().indexOf("has already been declared") >= 0 );
+  } catch (e) {
+    assertInstanceof(e, SyntaxError);
+    assertTrue(e.toString().includes("has already been declared"));
   }
 }
 
 function checkIsNotRedeclarationError(code) {
-  assertDoesNotThrow(()=>eval(`
-checkIsNotRedeclarationError_label : {
-  break checkIsNotRedeclarationError_label;
-${code}
-}
-`));
+  assertDoesNotThrow(() => eval(`
+    checkIsNotRedeclarationError_label: {
+      break checkIsNotRedeclarationError_label;
+      ${code}
+    }
+  `));
 }
 
 
@@ -52,143 +52,145 @@ let not_var_e = [
   'const {f:e}'
 ];
 
-// Check that `for (var ... of ...)` cannot redeclare a simple catch variable
-// but `for (var ... in ...)` can.
+// Check that both `for (var ... of ...)` and `for (var ... in ...)`
+// can redeclare a simple catch variable.
 for (let binding of var_e) {
-  checkIsRedeclarationError(`
-try {
-  throw 0;
-} catch(e) {
-  for (${binding} of []);
-}
-`);
-
   checkIsNotRedeclarationError(`
-try {
-  throw 0;
-} catch(e) {
-  for (${binding} in []);
-}
-`);
-}
-
-// Check that the above error occurs even for nested catches.
-for (let binding of var_e) {
-  checkIsRedeclarationError(`
-try {
-  throw 0;
-} catch(e) {
-  try {
-    throw 1;
-  } catch(f) {
     try {
-      throw 2;
-    } catch({}) {
+      throw 0;
+    } catch (e) {
       for (${binding} of []);
     }
-  }
-}
-`);
+  `);
 
   checkIsNotRedeclarationError(`
-try {
-  throw 0;
-} catch(e) {
-  try {
-    throw 1;
-  } catch(f) {
     try {
-      throw 2;
-    } catch({}) {
+      throw 0;
+    } catch (e) {
       for (${binding} in []);
     }
-  }
-}
-`);
+  `);
 }
 
-// Check that the above error does not occur if a declaration scope is between
-// the catch and the loop.
+// Check that the above applies even for nested catches.
 for (let binding of var_e) {
   checkIsNotRedeclarationError(`
-try {
-  throw 0;
-} catch(e) {
-  (()=>{for (${binding} of []);})();
-}
-`);
+    try {
+      throw 0;
+    } catch (e) {
+      try {
+        throw 1;
+      } catch (f) {
+        try {
+          throw 2;
+        } catch ({}) {
+          for (${binding} of []);
+        }
+      }
+    }
+  `);
 
   checkIsNotRedeclarationError(`
-try {
-  throw 0;
-} catch(e) {
-  (function(){for (${binding} of []);})();
+    try {
+      throw 0;
+    } catch (e) {
+      try {
+        throw 1;
+      } catch (f) {
+        try {
+          throw 2;
+        } catch ({}) {
+          for (${binding} in []);
+        }
+      }
+    }
+  `);
 }
-`);
+
+// Check that the above applies if a declaration scope is between the
+// catch and the loop.
+for (let binding of var_e) {
+  checkIsNotRedeclarationError(`
+    try {
+      throw 0;
+    } catch (e) {
+      (()=>{for (${binding} of []);})();
+    }
+  `);
+
+  checkIsNotRedeclarationError(`
+    try {
+      throw 0;
+    } catch (e) {
+      (function() {
+        for (${binding} of []);
+      })();
+    }
+  `);
 }
 
 // Check that there is no error when not declaring a var named e.
 for (let binding of not_var_e) {
   checkIsNotRedeclarationError(`
-try {
-  throw 0;
-} catch(e) {
-  for (${binding} of []);
-}
-`);
+    try {
+      throw 0;
+    } catch (e) {
+      for (${binding} of []);
+    }
+  `);
 }
 
 // Check that there is an error for both for-in and for-of when redeclaring
-// a non-simple catch parameter
+// a non-simple catch parameter.
 for (let binding of var_e) {
   checkIsRedeclarationError(`
-try {
-  throw 0;
-} catch({e}) {
-  for (${binding} of []);
-}
-`);
+    try {
+      throw 0;
+    } catch ({e}) {
+      for (${binding} of []);
+    }
+  `);
 
   checkIsRedeclarationError(`
-try {
-  throw 0;
-} catch({e}) {
-  for (${binding} in []);
-}
-`);
+    try {
+      throw 0;
+    } catch ({e}) {
+      for (${binding} in []);
+    }
+  `);
 }
 
 // Check that the above error occurs even for nested catches.
 for (let binding of var_e) {
   checkIsRedeclarationError(`
-try {
-  throw 0;
-} catch({e}) {
-  try {
-    throw 1;
-  } catch(f) {
     try {
-      throw 2;
-    } catch({}) {
-      for (${binding} of []);
+      throw 0;
+    } catch ({e}) {
+      try {
+        throw 1;
+      } catch (f) {
+        try {
+          throw 2;
+        } catch ({}) {
+          for (${binding} of []);
+        }
+      }
     }
-  }
-}
-`);
+  `);
 
   checkIsRedeclarationError(`
-try {
-  throw 0;
-} catch({e}) {
-  try {
-    throw 1;
-  } catch(f) {
     try {
-      throw 2;
-    } catch({}) {
-      for (${binding} in []);
+      throw 0;
+    } catch ({e}) {
+      try {
+        throw 1;
+      } catch (f) {
+        try {
+          throw 2;
+        } catch ({}) {
+          for (${binding} in []);
+        }
+      }
     }
-  }
-}
-`);
+  `);
 }

test/mjsunit/for-of-in-catch-duplicate-decl.js

@@ -2,4 +2,4 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-assertThrows("try { } catch (e) { var e; for (var e of []) {} }")
+assertDoesNotThrow("try { } catch (e) { var e; for (var e of []) {} }")

test/test262/test262.status

@@ -690,6 +690,10 @@
   # https://bugs.chromium.org/p/v8/issues/detail?id=7187
   'built-ins/Function/prototype/toString/line-terminator-normalisation-CR': [SKIP],
 
+  # https://bugs.chromium.org/p/v8/issues/detail?id=8759
+  'language/eval-code/direct/var-env-lower-lex-catch-non-strict': [SKIP],
+  'language/statements/try/early-catch-var': [SKIP],
+
   ############################ SLOW TESTS #############################
 
   'annexB/built-ins/RegExp/RegExp-leading-escape-BMP': [PASS, SLOW],