Bug 1523720: Allow for-of var-redeclarations within a catch-block. r=jorendorff

js/src/frontend/NameAnalysisTypes.h

@@ -72,7 +72,6 @@ enum class DeclarationKind : uint8_t {
   FormalParameter,
   CoverArrowParameter,
   Var,
-  ForOfVar,
   Let,
   Const,
   Class,  // Handled as same as `let` after parsing.
@@ -97,7 +96,6 @@ static inline BindingKind DeclarationKindToBindingKind(DeclarationKind kind) {
     case DeclarationKind::BodyLevelFunction:
     case DeclarationKind::ModuleBodyLevelFunction:
     case DeclarationKind::VarForAnnexBLexicalFunction:
-    case DeclarationKind::ForOfVar:
       return BindingKind::Var;
 
     case DeclarationKind::Let:

js/src/frontend/ParseContext.cpp

@@ -40,8 +40,6 @@ const char* DeclarationKindString(DeclarationKind kind) {
       return "function";
     case DeclarationKind::VarForAnnexBLexicalFunction:
       return "annex b var";
-    case DeclarationKind::ForOfVar:
-      return "var in for-of";
     case DeclarationKind::SimpleCatchParameter:
     case DeclarationKind::CatchParameter:
       return "catch parameter";
@@ -53,8 +51,7 @@ const char* DeclarationKindString(DeclarationKind kind) {
 bool DeclarationKindIsVar(DeclarationKind kind) {
   return kind == DeclarationKind::Var ||
          kind == DeclarationKind::BodyLevelFunction ||
-         kind == DeclarationKind::VarForAnnexBLexicalFunction ||
-         kind == DeclarationKind::ForOfVar;
+         kind == DeclarationKind::VarForAnnexBLexicalFunction;
 }
 
 bool DeclarationKindIsParameter(DeclarationKind kind) {
@@ -359,10 +356,8 @@ Maybe<DeclarationKind> ParseContext::isVarRedeclaredInEval(
       switch (bi.kind()) {
         case BindingKind::Let: {
           // Annex B.3.5 allows redeclaring simple (non-destructured)
-          // catch parameters with var declarations, except when it
-          // appears in a for-of.
-          bool annexB35Allowance = si.kind() == ScopeKind::SimpleCatch &&
-                                   kind != DeclarationKind::ForOfVar;
+          // catch parameters with var declarations.
+          bool annexB35Allowance = si.kind() == ScopeKind::SimpleCatch;
           if (!annexB35Allowance) {
             return Some(ScopeKindIsCatch(si.kind())
                             ? DeclarationKind::CatchParameter
@@ -434,13 +429,6 @@ bool ParseContext::tryDeclareVarHelper(HandlePropertyName name,
         // restrictive kind. These semantics are implemented in
         // CheckCanDeclareGlobalBinding.
         //
-        // For a var previously declared as ForOfVar, this previous
-        // DeclarationKind is used only to check for if the
-        // 'arguments' binding should be declared. Since body-level
-        // functions shadow 'arguments' [5], it is correct to alter
-        // the kind to BodyLevelFunction. See
-        // declareFunctionArgumentsObject.
-        //
         // VarForAnnexBLexicalFunction declarations are declared when
         // the var scope exits. It is not possible for a var to be
         // previously declared as VarForAnnexBLexicalFunction and
@@ -450,7 +438,6 @@ bool ParseContext::tryDeclareVarHelper(HandlePropertyName name,
         // [2] ES 18.2.1.3
         // [3] ES 8.1.1.4.15
         // [4] ES 8.1.1.4.16
-        // [5] ES 9.2.12
         if (dryRunOption == NotDryRun &&
             kind == DeclarationKind::BodyLevelFunction) {
           MOZ_ASSERT(declaredKind !=
@@ -459,11 +446,9 @@ bool ParseContext::tryDeclareVarHelper(HandlePropertyName name,
         }
       } else if (!DeclarationKindIsParameter(declaredKind)) {
         // Annex B.3.5 allows redeclaring simple (non-destructured)
-        // catch parameters with var declarations, except when it
-        // appears in a for-of.
+        // catch parameters with var declarations.
         bool annexB35Allowance =
-            declaredKind == DeclarationKind::SimpleCatchParameter &&
-            kind != DeclarationKind::ForOfVar;
+            declaredKind == DeclarationKind::SimpleCatchParameter;
 
         // Annex B.3.3 allows redeclaring functions in the same block.
         bool annexB33Allowance =

js/src/frontend/Parser.cpp

@@ -608,8 +608,7 @@ bool GeneralParser<ParseHandler, Unit>::noteDeclaredName(
 
   switch (kind) {
     case DeclarationKind::Var:
-    case DeclarationKind::BodyLevelFunction:
-    case DeclarationKind::ForOfVar: {
+    case DeclarationKind::BodyLevelFunction: {
       Maybe<DeclarationKind> redeclaredKind;
       uint32_t prevPos;
       if (!pc->tryDeclareVar(name, kind, pos.begin, &redeclaredKind,
@@ -1882,8 +1881,7 @@ bool PerHandlerParser<ParseHandler>::declareFunctionArgumentsObject() {
   // declared 'var arguments', we still need to declare 'arguments' in the
   // function scope.
   DeclaredNamePtr p = varScope.lookupDeclaredName(argumentsName);
-  if (p && (p->value()->kind() == DeclarationKind::Var ||
-            p->value()->kind() == DeclarationKind::ForOfVar)) {
+  if (p && p->value()->kind() == DeclarationKind::Var) {
     if (hasExtraBodyVarScope) {
       tryDeclareArguments = true;
     } else {
@@ -4154,11 +4152,6 @@ GeneralParser<ParseHandler, Unit>::declarationPattern(
       *forHeadKind = ParseNodeKind::ForIn;
     } else if (isForOf) {
       *forHeadKind = ParseNodeKind::ForOf;
-
-      // Annex B.3.5 has different early errors for vars in for-of loops.
-      if (declKind == DeclarationKind::Var) {
-        declKind = DeclarationKind::ForOfVar;
-      }
     } else {
       *forHeadKind = ParseNodeKind::ForHead;
     }
@@ -4306,11 +4299,6 @@ GeneralParser<ParseHandler, Unit>::declarationName(DeclarationKind declKind,
         *forHeadKind = ParseNodeKind::ForIn;
       } else if (isForOf) {
         *forHeadKind = ParseNodeKind::ForOf;
-
-        // Annex B.3.5 has different early errors for vars in for-of loops.
-        if (declKind == DeclarationKind::Var) {
-          declKind = DeclarationKind::ForOfVar;
-        }
       } else {
         *forHeadKind = ParseNodeKind::ForHead;
       }

js/src/tests/jstests.list

@@ -1260,3 +1260,7 @@ skip script test262/harness/detachArrayBuffer.js
 # https://github.com/tc39/test262/pull/1965
 skip script test262/language/expressions/dynamic-import/indirect-resolution.js
 skip script test262/language/expressions/dynamic-import/namespace/default-property-not-set-own.js
+
+# https://github.com/tc39/test262/pull/2023
+skip script test262/language/statements/try/early-catch-var.js
+skip script test262/language/eval-code/direct/var-env-lower-lex-catch-non-strict.js

js/src/tests/non262/lexical-environment/var-in-catch-body-annex-b-eval-for-of.js

@@ -1,12 +1,12 @@
 // |reftest| skip-if(!xulRuntime.shell)
 
-assertThrowsInstanceOf(() => evaluate(`
+evaluate(`
     try { throw null; } catch (e) { eval("for (var e of []) {}") }
-`), SyntaxError);
+`);
 
-assertThrowsInstanceOf(new Function(`
+new Function(`
     try { throw null; } catch (e) { eval("for (var e of []) {}") }
-`), SyntaxError);
+`)();
 
 if (typeof reportCompare === "function")
     reportCompare(true, true);

js/src/tests/non262/lexical-environment/var-in-catch-body-annex-b.js

@@ -82,6 +82,14 @@ function h1() {
 }
 h1();
 
+// Tests that var declaration is allowed in for-of head.
+function h2() {
+  try {} catch (e) {
+    for (var e of {});
+  }
+}
+h2();
+
 // Tests that redeclaring a var inside the catch is allowed.
 function h3() {
   var e;
@@ -91,16 +99,6 @@ function h3() {
 }
 h3();
 
-// Tests that var declaration is not allowed in for-of head.
-assertThrowsInstanceOf(function () {
-  eval(`
-       function h2() {
-         try {} catch (e) { for (var e of {}); }
-       }
-       log += 'unreached';
-       `);
-}, SyntaxError);
-
 if (typeof evaluate === "function") {
   assertThrowsInstanceOf(function () {
     evaluate(`