Bug 1298640 - Always give catch bodies their own lexical scope. (r=Waldo)
authorShu-yu Guo <shu@rfrn.org>
Wed, 31 Aug 2016 14:56:30 -0700
changeset 312135 d1194f91ff50d7200e5479272afb215bcb90a05f
parent 312134 3c3194673109d9e704e9c67f4318043ebf153b92
child 312136 daceebf38886f651670abe575ed70ae0673baec5
push id30632
push userryanvm@gmail.com
push dateThu, 01 Sep 2016 02:33:28 +0000
treeherdermozilla-central@b7f7ae14590a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersWaldo
bugs1298640
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1298640 - Always give catch bodies their own lexical scope. (r=Waldo)
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
js/src/tests/ecma_6/LexicalEnvironment/catch-body.js
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -165,25 +165,59 @@ ParseContext::Scope::removeVarForAnnexBL
         }
     }
 
     // Annex B semantics no longer applies to any functions with this name, as
     // an early error would have occurred.
     pc->removeInnerFunctionBoxesForAnnexB(name);
 }
 
+#ifdef DEBUG
+static bool
+DeclarationKindIsCatchParameter(DeclarationKind kind)
+{
+    return kind == DeclarationKind::SimpleCatchParameter ||
+           kind == DeclarationKind::CatchParameter;
+}
+#endif
+
+bool
+ParseContext::Scope::addCatchParameters(ParseContext* pc, Scope& catchParamScope)
+{
+    if (pc->useAsmOrInsideUseAsm())
+        return true;
+
+    for (DeclaredNameMap::Range r = catchParamScope.declared_->all(); !r.empty(); r.popFront()) {
+        DeclarationKind kind = r.front().value()->kind();
+        MOZ_ASSERT(DeclarationKindIsCatchParameter(kind));
+        JSAtom* name = r.front().key();
+        AddDeclaredNamePtr p = lookupDeclaredNameForAdd(name);
+        MOZ_ASSERT(!p);
+        if (!addDeclaredName(pc, p, name, kind))
+            return false;
+    }
+
+    return true;
+}
+
 void
-ParseContext::Scope::removeSimpleCatchParameter(ParseContext* pc, JSAtom* name)
+ParseContext::Scope::removeCatchParameters(ParseContext* pc, Scope& catchParamScope)
 {
     if (pc->useAsmOrInsideUseAsm())
         return;
 
-    DeclaredNamePtr p = declared_->lookup(name);
-    MOZ_ASSERT(p && p->value()->kind() == DeclarationKind::SimpleCatchParameter);
-    declared_->remove(p);
+    for (DeclaredNameMap::Range r = catchParamScope.declared_->all(); !r.empty(); r.popFront()) {
+        DeclaredNamePtr p = declared_->lookup(r.front().key());
+        MOZ_ASSERT(p);
+
+        // This check is needed because the catch body could have declared
+        // vars, which would have been added to catchParamScope.
+        if (DeclarationKindIsCatchParameter(r.front().value()->kind()))
+            declared_->remove(p);
+    }
 }
 
 void
 SharedContext::computeAllowSyntax(Scope* scope)
 {
     for (ScopeIter si(scope); si; si++) {
         if (si.kind() == ScopeKind::Function) {
             JSFunction* fun = si.scope()->as<FunctionScope>().canonicalFunction();
@@ -5969,18 +6003,16 @@ Parser<ParseHandler>::tryStatement(Yield
              * Legal catch forms are:
              *   catch (lhs)
              *   catch (lhs if <boolean_expression>)
              * where lhs is a name or a destructuring left-hand side.
              * (the latter is legal only #ifdef JS_HAS_CATCH_GUARD)
              */
             MUST_MATCH_TOKEN(TOK_LP, JSMSG_PAREN_BEFORE_CATCH);
 
-            RootedPropertyName simpleCatchParam(context);
-
             if (!tokenStream.getToken(&tt))
                 return null();
             Node catchName;
             switch (tt) {
               case TOK_LB:
               case TOK_LC:
                 catchName = destructuringDeclaration(DeclarationKind::CatchParameter,
                                                      yieldHandling, tt);
@@ -5993,27 +6025,25 @@ Parser<ParseHandler>::tryStatement(Yield
                     return null();
                 }
 
                 // Even if yield is *not* necessarily a keyword, we still must
                 // check its validity for legacy generators.
                 if (!checkYieldNameValidity())
                     return null();
                 MOZ_FALLTHROUGH;
-              case TOK_NAME:
-                simpleCatchParam = tokenStream.currentName();
-                catchName = newName(simpleCatchParam);
+              case TOK_NAME: {
+                RootedPropertyName param(context, tokenStream.currentName());
+                catchName = newName(param);
                 if (!catchName)
                     return null();
-                if (!noteDeclaredName(simpleCatchParam, DeclarationKind::SimpleCatchParameter,
-                                      pos()))
-                {
+                if (!noteDeclaredName(param, DeclarationKind::SimpleCatchParameter, pos()))
                     return null();
-                }
                 break;
+              }
 
               default:
                 report(ParseError, false, null(), JSMSG_CATCH_IDENTIFIER);
                 return null();
             }
 
             Node catchGuard = null();
 #if JS_HAS_CATCH_GUARD
@@ -6030,17 +6060,17 @@ Parser<ParseHandler>::tryStatement(Yield
                 if (!catchGuard)
                     return null();
             }
 #endif
             MUST_MATCH_TOKEN(TOK_RP, JSMSG_PAREN_AFTER_CATCH);
 
             MUST_MATCH_TOKEN(TOK_LC, JSMSG_CURLY_BEFORE_CATCH);
 
-            Node catchBody = catchBlockStatement(yieldHandling, simpleCatchParam);
+            Node catchBody = catchBlockStatement(yieldHandling, scope);
             if (!catchBody)
                 return null();
 
             if (!catchGuard)
                 hasUnconditionalCatch = true;
 
             pnblock = finishLexicalScope(scope, catchBody);
             if (!pnblock)
@@ -6084,53 +6114,43 @@ Parser<ParseHandler>::tryStatement(Yield
     }
 
     return handler.newTryStatement(begin, innerBlock, catchList, finallyBlock);
 }
 
 template <typename ParseHandler>
 typename ParseHandler::Node
 Parser<ParseHandler>::catchBlockStatement(YieldHandling yieldHandling,
-                                          HandlePropertyName simpleCatchParam)
+                                          ParseContext::Scope& catchParamScope)
 {
     ParseContext::Statement stmt(pc, StatementKind::Block);
 
-    // Annex B.3.5 requires that vars be allowed to redeclare a simple
-    // (non-destructured) catch parameter (including via a direct eval), so
-    // the catch parameter needs to live in its own scope. So if we have a
-    // simple catch parameter, make a new scope.
-    Node body;
-    if (simpleCatchParam) {
-        ParseContext::Scope scope(this);
-        if (!scope.init(pc))
-            return null();
-
-        // The catch parameter name cannot be redeclared inside the catch
-        // block, so declare the name in the inner scope.
-        if (!noteDeclaredName(simpleCatchParam, DeclarationKind::SimpleCatchParameter, pos()))
-            return null();
-
-        Node list = statementList(yieldHandling);
-        if (!list)
-            return null();
-
-        // The catch parameter name is not bound in this scope, so remove it
-        // before generating bindings.
-        scope.removeSimpleCatchParameter(pc, simpleCatchParam);
-
-        body = finishLexicalScope(scope, list);
-    } else {
-        body = statementList(yieldHandling);
-    }
-    if (!body)
+    // ES 13.15.7 CatchClauseEvaluation
+    //
+    // Step 8 means that the body of a catch block always has an additional
+    // lexical scope.
+    ParseContext::Scope scope(this);
+    if (!scope.init(pc))
+        return null();
+
+    // The catch parameter names cannot be redeclared inside the catch
+    // block, so declare the name in the inner scope.
+    if (!scope.addCatchParameters(pc, catchParamScope))
+        return null();
+
+    Node list = statementList(yieldHandling);
+    if (!list)
         return null();
 
     MUST_MATCH_TOKEN_MOD(TOK_RC, TokenStream::Operand, JSMSG_CURLY_AFTER_CATCH);
 
-    return body;
+    // The catch parameter names are not bound in the body scope, so remove
+    // them before generating bindings.
+    scope.removeCatchParameters(pc, catchParamScope);
+    return finishLexicalScope(scope, list);
 }
 
 template <typename ParseHandler>
 typename ParseHandler::Node
 Parser<ParseHandler>::debuggerStatement()
 {
     TokenPos p;
     p.begin = pos().begin;
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -149,19 +149,20 @@ class ParseContext : public Nestable<Par
         {
             return maybeReportOOM(pc, declared_->add(p, name, DeclaredNameInfo(kind)));
         }
 
         // Remove all VarForAnnexBLexicalFunction declarations of a certain
         // name from all scopes in pc's scope stack.
         static void removeVarForAnnexBLexicalFunction(ParseContext* pc, JSAtom* name);
 
-        // Remove a simple catch parameter name. Used to implement the odd
-        // semantics of Annex B.3.5.
-        void removeSimpleCatchParameter(ParseContext* pc, JSAtom* name);
+        // Add and remove catch parameter names. Used to implement the odd
+        // semantics of catch bodies.
+        bool addCatchParameters(ParseContext* pc, Scope& catchParamScope);
+        void removeCatchParameters(ParseContext* pc, Scope& catchParamScope);
 
         void useAsVarScope(ParseContext* pc) {
             MOZ_ASSERT(!pc->varScope_);
             pc->varScope_ = this;
         }
 
         // An iterator for the set of names a scope binds: the set of all
         // declared names for 'var' scopes, and the set of lexically declared
@@ -1037,17 +1038,17 @@ class Parser final : private JS::AutoGCR
 
     Node switchStatement(YieldHandling yieldHandling);
     Node continueStatement(YieldHandling yieldHandling);
     Node breakStatement(YieldHandling yieldHandling);
     Node returnStatement(YieldHandling yieldHandling);
     Node withStatement(YieldHandling yieldHandling);
     Node throwStatement(YieldHandling yieldHandling);
     Node tryStatement(YieldHandling yieldHandling);
-    Node catchBlockStatement(YieldHandling yieldHandling, HandlePropertyName simpleCatchParam);
+    Node catchBlockStatement(YieldHandling yieldHandling, ParseContext::Scope& catchParamScope);
     Node debuggerStatement();
 
     Node variableStatement(YieldHandling yieldHandling);
 
     Node labeledStatement(YieldHandling yieldHandling);
     Node labeledItem(YieldHandling yieldHandling);
 
     Node ifStatement(YieldHandling yieldHandling);
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/LexicalEnvironment/catch-body.js
@@ -0,0 +1,19 @@
+function f() {
+  var probeParam, probeBlock;
+  let x = 'outside';
+
+  try {
+    throw [];
+  } catch ([_ = probeParam = function() { return x; }]) {
+    probeBlock = function() { return x; };
+    let x = 'inside';
+  }
+
+  assertEq(probeBlock(), 'inside');
+  assertEq(probeParam(), 'outside');
+}
+
+f();
+
+if (typeof reportCompare === 'function')
+  reportCompare(true, true);