Issue #1465 - Implement optional catch binding.

7 files changed, 118 insertions, 76 deletions

diff --git a/js/src/builtin/ReflectParse.cpp b/js/src/builtin/ReflectParse.cpp
index 8e8bb2417..e7a84c7c3 100644
--- a/js/src/builtin/ReflectParse.cpp
+++ b/js/src/builtin/ReflectParse.cpp
@@ -1547,7 +1547,7 @@ NodeBuilder::catchClause(HandleValue var, HandleValue guard, HandleValue body, T
 {
     RootedValue cb(cx, callbacks[AST_CATCH]);
     if (!cb.isNull())
-        return callback(cb, var, opt(guard), body, pos, dst);
+        return callback(cb, opt(var), opt(guard), body, pos, dst);
 
     return newNode(AST_CATCH, pos,
                    "param", var,
@@ -1803,6 +1803,14 @@ class ASTSerializer
     bool identifier(ParseNode* pn, MutableHandleValue dst);
     bool literal(ParseNode* pn, MutableHandleValue dst);
 
+    bool optPattern(ParseNode* pn, MutableHandleValue dst) {
+        if (!pn) {
+            dst.setMagic(JS_SERIALIZE_NO_NODE);
+            return true;
+        }
+        return pattern(pn, dst);
+    }
+
     bool pattern(ParseNode* pn, MutableHandleValue dst);
     bool arrayPattern(ParseNode* pn, MutableHandleValue dst);
     bool objectPattern(ParseNode* pn, MutableHandleValue dst);
@@ -2265,13 +2273,13 @@ ASTSerializer::switchStatement(ParseNode* pn, MutableHandleValue dst)
 bool
 ASTSerializer::catchClause(ParseNode* pn, bool* isGuarded, MutableHandleValue dst)
 {
-    MOZ_ASSERT(pn->pn_pos.encloses(pn->pn_kid1->pn_pos));
+    MOZ_ASSERT_IF(pn->pn_kid1, pn->pn_pos.encloses(pn->pn_kid1->pn_pos));
     MOZ_ASSERT_IF(pn->pn_kid2, pn->pn_pos.encloses(pn->pn_kid2->pn_pos));
     MOZ_ASSERT(pn->pn_pos.encloses(pn->pn_kid3->pn_pos));
 
     RootedValue var(cx), guard(cx), body(cx);
 
-    if (!pattern(pn->pn_kid1, &var) ||
+    if (!optPattern(pn->pn_kid1, &var) ||
         !optExpression(pn->pn_kid2, &guard)) {
         return false;
     }
diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp
index 309d6c290..fd9d072b4 100644
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -3427,10 +3427,12 @@ BytecodeEmitter::checkSideEffects(ParseNode* pn, bool* answer)
 
       case PNK_CATCH:
         MOZ_ASSERT(pn->isArity(PN_TERNARY));
-        if (!checkSideEffects(pn->pn_kid1, answer))
-            return false;
-        if (*answer)
-            return true;
+        if (ParseNode* name = pn->pn_kid1) {
+            if (!checkSideEffects(name, answer))
+                return false;
+            if (*answer)
+                return true;
+        }
         if (ParseNode* cond = pn->pn_kid2) {
             if (!checkSideEffects(cond, answer))
                 return false;
@@ -6564,24 +6566,31 @@ BytecodeEmitter::emitCatch(ParseNode* pn)
         return false;
 
     ParseNode* pn2 = pn->pn_kid1;
-    switch (pn2->getKind()) {
-      case PNK_ARRAY:
-      case PNK_OBJECT:
-        if (!emitDestructuringOps(pn2, DestructuringDeclaration))
-            return false;
-        if (!emit1(JSOP_POP))
-            return false;
-        break;
-
-      case PNK_NAME:
-        if (!emitLexicalInitialization(pn2))
-            return false;
-        if (!emit1(JSOP_POP))
-            return false;
-        break;
-
-      default:
-        MOZ_ASSERT(0);
+    if (!pn2) {
+      // See ES2019 13.15.7 Runtime Semantics: CatchClauseEvaluation
+      // Catch variable was omitted: discard the exception.
+      if (!emit1(JSOP_POP))
+        return false;
+    } else {
+      switch (pn2->getKind()) {
+        case PNK_ARRAY:
+        case PNK_OBJECT:
+          if (!emitDestructuringOps(pn2, DestructuringDeclaration))
+              return false;
+          if (!emit1(JSOP_POP))
+              return false;
+          break;
+
+        case PNK_NAME:
+          if (!emitLexicalInitialization(pn2))
+              return false;
+          if (!emit1(JSOP_POP))
+              return false;
+          break;
+
+        default:
+          MOZ_ASSERT(0);
+      }
     }
 
     // If there is a guard expression, emit it and arrange to jump to the next
@@ -6825,7 +6834,9 @@ BytecodeEmitter::emitLexicalScope(ParseNode* pn)
     EmitterScope emitterScope(this);
     ScopeKind kind;
     if (body->isKind(PNK_CATCH))
-        kind = body->pn_kid1->isKind(PNK_NAME) ? ScopeKind::SimpleCatch : ScopeKind::Catch;
+        kind = (!body->pn_kid1 || body->pn_kid1->isKind(PNK_NAME)) ?
+               ScopeKind::SimpleCatch :
+               ScopeKind::Catch;
     else
         kind = ScopeKind::Lexical;
 
diff --git a/js/src/frontend/FoldConstants.cpp b/js/src/frontend/FoldConstants.cpp
index 689fa02b4..fcda44a84 100644
--- a/js/src/frontend/FoldConstants.cpp
+++ b/js/src/frontend/FoldConstants.cpp
@@ -1259,9 +1259,10 @@ FoldCatch(ExclusiveContext* cx, ParseNode* node, Parser<FullParseHandler>& parse
     MOZ_ASSERT(node->isKind(PNK_CATCH));
     MOZ_ASSERT(node->isArity(PN_TERNARY));
 
-    ParseNode*& declPattern = node->pn_kid1;
-    if (!Fold(cx, &declPattern, parser, inGenexpLambda))
-        return false;
+    if (ParseNode*& declPattern = node->pn_kid1) {
+        if (!Fold(cx, &declPattern, parser, inGenexpLambda))
+            return false;
+    }
 
     if (ParseNode*& cond = node->pn_kid2) {
         if (!FoldCondition(cx, &cond, parser, inGenexpLambda))
diff --git a/js/src/frontend/NameFunctions.cpp b/js/src/frontend/NameFunctions.cpp
index dc54d0a88..f1a29163a 100644
--- a/js/src/frontend/NameFunctions.cpp
+++ b/js/src/frontend/NameFunctions.cpp
@@ -650,8 +650,10 @@ class NameResolver
           // contain arbitrary expressions.
           case PNK_CATCH:
             MOZ_ASSERT(cur->isArity(PN_TERNARY));
-            if (!resolve(cur->pn_kid1, prefix))
-                return false;
+            if (cur->pn_kid1) {
+                if (!resolve(cur->pn_kid1, prefix))
+                    return false;
+            }
             if (cur->pn_kid2) {
                 if (!resolve(cur->pn_kid2, prefix))
                     return false;
diff --git a/js/src/frontend/ParseNode.cpp b/js/src/frontend/ParseNode.cpp
index 5fe64e3d3..603c97e57 100644
--- a/js/src/frontend/ParseNode.cpp
+++ b/js/src/frontend/ParseNode.cpp
@@ -407,13 +407,14 @@ PushNodeChildren(ParseNode* pn, NodeStack* stack)
         return PushResult::Recyclable;
       }
 
-      // A catch node has first kid as catch-variable pattern, the second kid
-      // as catch condition (which, if non-null, records the |<cond>| in
-      // SpiderMonkey's |catch (e if <cond>)| extension), and third kid as the
-      // statements in the catch block.
+      // A catch node has an (optional) first kid as catch-variable pattern,
+      // the second kid as (optional) catch condition (which, records the
+      // |<cond>| in SpiderMonkey's |catch (e if <cond>)| extension), and
+      // third kid as the statements in the catch block.
       case PNK_CATCH: {
         MOZ_ASSERT(pn->isArity(PN_TERNARY));
-        stack->push(pn->pn_kid1);
+        if (pn->pn_kid1)
+            stack->push(pn->pn_kid1);
         if (pn->pn_kid2)
             stack->push(pn->pn_kid2);
         stack->push(pn->pn_kid3);
diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp
index 0c279591f..cf1639e4c 100644
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -6915,57 +6915,67 @@ Parser<ParseHandler>::tryStatement(YieldHandling yieldHandling)
             /*
              * Legal catch forms are:
              *   catch (lhs)
-             *   catch (lhs if <boolean_expression>)
+             *   catch (lhs if <boolean_expression>)  ** non-standard **
+             *   catch                                ** ES2019 **
              * where lhs is a name or a destructuring left-hand side.
-             * (the latter is legal only #ifdef JS_HAS_CATCH_GUARD)
+             * The second is legal only #ifdef JS_HAS_CATCH_GUARD
              */
-            MUST_MATCH_TOKEN(TOK_LP, JSMSG_PAREN_BEFORE_CATCH);
-
-            if (!tokenStream.getToken(&tt))
+            bool omittedBinding;
+            if (!tokenStream.matchToken(&omittedBinding, TOK_LC))
                 return null();
+
             Node catchName;
-            switch (tt) {
-              case TOK_LB:
-              case TOK_LC:
-                catchName = destructuringDeclaration(DeclarationKind::CatchParameter,
-                                                     yieldHandling, tt);
-                if (!catchName)
-                    return null();
-                break;
+            Node catchGuard = null();
 
-              default: {
-                if (!TokenKindIsPossibleIdentifierName(tt)) {
-                    error(JSMSG_CATCH_IDENTIFIER);
-                    return null();
-                }
+            if (omittedBinding) {
+                catchName = null();
+            } else {
+                MUST_MATCH_TOKEN(TOK_LP, JSMSG_PAREN_BEFORE_CATCH);
 
-                catchName = bindingIdentifier(DeclarationKind::SimpleCatchParameter,
-                                              yieldHandling);
-                if (!catchName)
+                if (!tokenStream.getToken(&tt))
                     return null();
-                break;
-              }
-            }
+                switch (tt) {
+                  case TOK_LB:
+                  case TOK_LC:
+                    catchName = destructuringDeclaration(DeclarationKind::CatchParameter,
+                                                     yieldHandling, tt);
+                    if (!catchName)
+                        return null();
+                    break;
+
+                  default: {
+                    if (!TokenKindIsPossibleIdentifierName(tt)) {
+                        error(JSMSG_CATCH_IDENTIFIER);
+                        return null();
+                    }
+
+                    catchName = bindingIdentifier(DeclarationKind::SimpleCatchParameter,
+                                                  yieldHandling);
+                    if (!catchName)
+                        return null();
+                    break;
+                  }
+                }
 
-            Node catchGuard = null();
 #if JS_HAS_CATCH_GUARD
-            /*
-             * We use 'catch (x if x === 5)' (not 'catch (x : x === 5)')
-             * to avoid conflicting with the JS2/ECMAv4 type annotation
-             * catchguard syntax.
-             */
-            bool matched;
-            if (!tokenStream.matchToken(&matched, TOK_IF))
-                return null();
-            if (matched) {
-                catchGuard = expr(InAllowed, yieldHandling, TripledotProhibited);
-                if (!catchGuard)
+                /*
+                 * We use 'catch (x if x === 5)' (not 'catch (x : x === 5)')
+                 * to avoid conflicting with the JS2/ECMAv4 type annotation
+                 * catchguard syntax.
+                 */
+                bool matched;
+                if (!tokenStream.matchToken(&matched, TOK_IF))
                     return null();
-            }
+                if (matched) {
+                    catchGuard = expr(InAllowed, yieldHandling, TripledotProhibited);
+                    if (!catchGuard)
+                        return null();
+                }
 #endif
-            MUST_MATCH_TOKEN(TOK_RP, JSMSG_PAREN_AFTER_CATCH);
+                MUST_MATCH_TOKEN(TOK_RP, JSMSG_PAREN_AFTER_CATCH);
 
-            MUST_MATCH_TOKEN(TOK_LC, JSMSG_CURLY_BEFORE_CATCH);
+                MUST_MATCH_TOKEN(TOK_LC, JSMSG_CURLY_BEFORE_CATCH);
+            }
 
             Node catchBody = catchBlockStatement(yieldHandling, scope);
             if (!catchBody)
diff --git a/js/src/tests/js1_8_5/reflect-parse/statements.js b/js/src/tests/js1_8_5/reflect-parse/statements.js
index 1df86508b..11c1b88b4 100644
--- a/js/src/tests/js1_8_5/reflect-parse/statements.js
+++ b/js/src/tests/js1_8_5/reflect-parse/statements.js
@@ -77,7 +77,16 @@ assertStmt("try { } catch (e if foo) { } catch (e if bar) { } catch (e) { } fina
                      catchClause(ident("e"), ident("bar"), blockStmt([])) ],
                    catchClause(ident("e"), null, blockStmt([])),
                    blockStmt([])));
-
+assertStmt("try { } catch { }",
+           tryStmt(blockStmt([]),
+                   [],
+       catchClause(null, null, blockStmt([])),
+                   null));
+assertStmt("try { } catch { } finally { }",
+           tryStmt(blockStmt([]),
+                   [],
+       catchClause(null, null, blockStmt([])),
+                   blockStmt([])));
 
 // Bug 632028: yield outside of a function should throw
 (function() {