Bug 1303703 - Part 2: Clean-up bits of destructuring parsing which are no longer needed. r=shu
authorAndré Bargull <andre.bargull@gmail.com>
Sat, 22 Apr 2017 02:09:23 -0700
changeset 402710 d6e29a56658e1931b181eb8c6a7a18d6d1c5b152
parent 402709 0ad2e4e4cd72b84f178a7b7f1af70760493da1c9
child 402711 8f2c0ac318b10cac245519338ed8556bcfce09e6
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1303703
milestone55.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 1303703 - Part 2: Clean-up bits of destructuring parsing which are no longer needed. r=shu
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -4409,30 +4409,16 @@ Parser<FullParseHandler, char16_t>::chec
  * - binding-like: |var| and |let| declarations, functions' formal
  *   parameter lists, |catch| clauses, and comprehension tails.  In
  *   these cases, the patterns' property value positions must be
  *   simple names; the destructuring defines them as new variables.
  *
  * In the first case, other code parses the pattern as an arbitrary
  * primaryExpr, and then, here in checkDestructuringAssignmentPattern, verify
  * that the tree is a valid AssignmentPattern.
- *
- * In assignment-like contexts, we parse the pattern with
- * pc->inDestructuringDecl clear, so the lvalue expressions in the pattern are
- * parsed normally. identifierReference() links variable references into the
- * appropriate use chains; creates placeholder definitions; and so on.
- * checkDestructuringAssignmentPattern won't bind any new names and we
- * specialize lvalues as appropriate.
- *
- * In declaration-like contexts, the normal variable reference processing
- * would just be an obstruction, because we're going to define the names that
- * appear in the property value positions as new variables anyway. In this
- * case, we parse the pattern in destructuringDeclaration() with
- * pc->inDestructuringDecl set, which directs identifierReference() to leave
- * whatever name nodes it creates unconnected.
  */
 template <>
 bool
 Parser<FullParseHandler, char16_t>::checkDestructuringAssignmentPattern(ParseNode* pattern,
                                                                         PossibleError* possibleError /* = nullptr */)
 {
     if (pattern->isKind(PNK_ARRAYCOMP)) {
         errorAt(pattern->pn_pos.begin, JSMSG_ARRAY_COMP_LEFTSIDE);
@@ -4445,49 +4431,29 @@ Parser<FullParseHandler, char16_t>::chec
 
     // Report any pending destructuring error.
     if (isDestructuring && possibleError && !possibleError->checkForDestructuringError())
         return false;
 
     return isDestructuring;
 }
 
-class AutoClearInDestructuringDecl
-{
-    ParseContext* pc_;
-    Maybe<DeclarationKind> saved_;
-
-  public:
-    explicit AutoClearInDestructuringDecl(ParseContext* pc)
-      : pc_(pc),
-        saved_(pc->inDestructuringDecl)
-    {
-        pc->inDestructuringDecl = Nothing();
-        if (saved_ && *saved_ == DeclarationKind::FormalParameter)
-            pc->functionBox()->hasParameterExprs = true;
-    }
-
-    ~AutoClearInDestructuringDecl() {
-        pc_->inDestructuringDecl = saved_;
-    }
-};
-
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
-Parser<ParseHandler, CharT>::bindingInitializer(Node lhs, YieldHandling yieldHandling)
+Parser<ParseHandler, CharT>::bindingInitializer(Node lhs, DeclarationKind kind,
+                                                YieldHandling yieldHandling)
 {
     MOZ_ASSERT(tokenStream.isCurrentTokenType(TOK_ASSIGN));
 
-    Node rhs;
-    {
-        AutoClearInDestructuringDecl autoClear(pc);
-        rhs = assignExpr(InAllowed, yieldHandling, TripledotProhibited);
-        if (!rhs)
-            return null();
-    }
+    if (kind == DeclarationKind::FormalParameter)
+        pc->functionBox()->hasParameterExprs = true;
+
+    Node rhs = assignExpr(InAllowed, yieldHandling, TripledotProhibited);
+    if (!rhs)
+        return null();
 
     handler.checkAndSetIsDirectRHSAnonFunction(rhs);
 
     Node assign = handler.newAssignment(PNK_ASSIGN, lhs, rhs, JSOP_NOP);
     if (!assign)
         return null();
 
     if (foldConstants && !FoldConstants(context, &assign, this))
@@ -4495,17 +4461,17 @@ Parser<ParseHandler, CharT>::bindingInit
 
     return assign;
 }
 
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
 Parser<ParseHandler, CharT>::bindingIdentifier(DeclarationKind kind, YieldHandling yieldHandling)
 {
-    Rooted<PropertyName*> name(context, bindingIdentifier(yieldHandling));
+    RootedPropertyName name(context, bindingIdentifier(yieldHandling));
     if (!name)
         return null();
 
     Node binding = newName(name);
     if (!binding || !noteDeclaredName(name, kind, pos()))
         return null();
 
     return binding;
@@ -4540,30 +4506,31 @@ Parser<ParseHandler, CharT>::objectBindi
     if (!CheckRecursionLimit(context))
         return null();
 
     uint32_t begin = pos().begin;
     Node literal = handler.newObjectLiteral(begin);
     if (!literal)
         return null();
 
+    Maybe<DeclarationKind> declKind = Some(kind);
     RootedAtom propAtom(context);
     for (;;) {
         TokenKind tt;
         if (!tokenStream.getToken(&tt))
             return null();
         if (tt == TOK_RC)
             break;
 
         TokenPos namePos = pos();
 
         tokenStream.ungetToken();
 
         PropertyType propType;
-        Node propName = propertyName(yieldHandling, literal, &propType, &propAtom);
+        Node propName = propertyName(yieldHandling, declKind, literal, &propType, &propAtom);
         if (!propName)
             return null();
 
         if (propType == PropertyType::Normal) {
             // Handle e.g., |var {p: x} = o| and |var {p: x=0} = o|.
 
             if (!tokenStream.getToken(&tt, TokenStream::Operand))
                 return null();
@@ -4572,17 +4539,17 @@ Parser<ParseHandler, CharT>::objectBindi
             if (!binding)
                 return null();
 
             bool hasInitializer;
             if (!tokenStream.matchToken(&hasInitializer, TOK_ASSIGN))
                 return null();
 
             Node bindingExpr = hasInitializer
-                               ? bindingInitializer(binding, yieldHandling)
+                               ? bindingInitializer(binding, kind, yieldHandling)
                                : binding;
             if (!bindingExpr)
                 return null();
 
             if (!handler.addPropertyDefinition(literal, propName, bindingExpr))
                 return null();
         } else if (propType == PropertyType::Shorthand) {
             // Handle e.g., |var {x, y} = o| as destructuring shorthand
@@ -4601,17 +4568,17 @@ Parser<ParseHandler, CharT>::objectBindi
             MOZ_ASSERT(TokenKindIsPossibleIdentifierName(tt));
 
             Node binding = bindingIdentifier(kind, yieldHandling);
             if (!binding)
                 return null();
 
             tokenStream.consumeKnownToken(TOK_ASSIGN);
 
-            Node bindingExpr = bindingInitializer(binding, yieldHandling);
+            Node bindingExpr = bindingInitializer(binding, kind, yieldHandling);
             if (!bindingExpr)
                 return null();
 
             if (!handler.addPropertyDefinition(literal, propName, bindingExpr))
                 return null();
         } else {
             errorAt(namePos.begin, JSMSG_NO_VARIABLE_NAME);
             return null();
@@ -4682,17 +4649,19 @@ Parser<ParseHandler, CharT>::arrayBindin
              Node binding = bindingIdentifierOrPattern(kind, yieldHandling, tt);
              if (!binding)
                  return null();
 
              bool hasInitializer;
              if (!tokenStream.matchToken(&hasInitializer, TOK_ASSIGN))
                  return null();
 
-             Node element = hasInitializer ? bindingInitializer(binding, yieldHandling) : binding;
+             Node element = hasInitializer
+                            ? bindingInitializer(binding, kind, yieldHandling)
+                            : binding;
              if (!element)
                  return null();
 
              handler.addArrayElement(literal, element);
          }
 
          if (tt != TOK_COMMA) {
              // If we didn't already match TOK_COMMA in above case.
@@ -4722,27 +4691,19 @@ template <template <typename CharT> clas
 typename ParseHandler<CharT>::Node
 Parser<ParseHandler, CharT>::destructuringDeclaration(DeclarationKind kind,
                                                       YieldHandling yieldHandling,
                                                       TokenKind tt)
 {
     MOZ_ASSERT(tokenStream.isCurrentTokenType(tt));
     MOZ_ASSERT(tt == TOK_LB || tt == TOK_LC);
 
-    Node pattern;
-    {
-        pc->inDestructuringDecl = Some(kind);
-        if (tt == TOK_LB)
-            pattern = arrayBindingPattern(kind, yieldHandling);
-        else
-            pattern = objectBindingPattern(kind, yieldHandling);
-        pc->inDestructuringDecl = Nothing();
-    }
-
-    return pattern;
+    return tt == TOK_LB
+           ? arrayBindingPattern(kind, yieldHandling)
+           : objectBindingPattern(kind, yieldHandling);
 }
 
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
 Parser<ParseHandler, CharT>::destructuringDeclarationWithoutYieldOrAwait(DeclarationKind kind,
                                                                          YieldHandling yieldHandling,
                                                                          TokenKind tt)
 {
@@ -7123,24 +7084,20 @@ Parser<ParseHandler, CharT>::tryStatemen
                 break;
 
               default: {
                 if (!TokenKindIsPossibleIdentifierName(tt)) {
                     error(JSMSG_CATCH_IDENTIFIER);
                     return null();
                 }
 
-                RootedPropertyName param(context, bindingIdentifier(yieldHandling));
-                if (!param)
-                    return null();
-                catchName = newName(param);
+                catchName = bindingIdentifier(DeclarationKind::SimpleCatchParameter,
+                                              yieldHandling);
                 if (!catchName)
                     return null();
-                if (!noteDeclaredName(param, DeclarationKind::SimpleCatchParameter, pos()))
-                    return null();
                 break;
               }
             }
 
             Node catchGuard = null();
 #if JS_HAS_CATCH_GUARD
             /*
              * We use 'catch (x if x === 5)' (not 'catch (x : x === 5)')
@@ -7359,16 +7316,17 @@ Parser<ParseHandler, CharT>::classDefini
     }
 
     MUST_MATCH_TOKEN(TOK_LC, JSMSG_CURLY_BEFORE_CLASS);
 
     Node classMethods = handler.newClassMethodList(pos().begin);
     if (!classMethods)
         return null();
 
+    Maybe<DeclarationKind> declKind = Nothing();
     for (;;) {
         TokenKind tt;
         if (!tokenStream.getToken(&tt))
             return null();
         if (tt == TOK_RC)
             break;
 
         if (tt == TOK_SEMI)
@@ -7392,17 +7350,17 @@ Parser<ParseHandler, CharT>::classDefini
             tokenStream.ungetToken();
         }
 
         uint32_t nameOffset;
         if (!tokenStream.peekOffset(&nameOffset))
             return null();
 
         PropertyType propType;
-        Node propName = propertyName(yieldHandling, classMethods, &propType, &propAtom);
+        Node propName = propertyName(yieldHandling, declKind, classMethods, &propType, &propAtom);
         if (!propName)
             return null();
 
         if (propType != PropertyType::Getter && propType != PropertyType::Setter &&
             propType != PropertyType::Method && propType != PropertyType::GeneratorMethod &&
             propType != PropertyType::AsyncMethod &&
             propType != PropertyType::AsyncGeneratorMethod)
         {
@@ -8485,23 +8443,19 @@ Parser<ParseHandler, CharT>::assignExpr(
     } else {
         errorAt(exprOffset, JSMSG_BAD_LEFTSIDE_OF_ASS);
         return null();
     }
 
     if (!possibleErrorInner.checkForExpressionError())
         return null();
 
-    Node rhs;
-    {
-        AutoClearInDestructuringDecl autoClear(pc);
-        rhs = assignExpr(inHandling, yieldHandling, TripledotProhibited);
-        if (!rhs)
-            return null();
-    }
+    Node rhs = assignExpr(inHandling, yieldHandling, TripledotProhibited);
+    if (!rhs)
+        return null();
 
     if (kind == PNK_ASSIGN)
         handler.checkAndSetIsDirectRHSAnonFunction(rhs);
 
     return handler.newAssignment(kind, lhs, rhs, op);
 }
 
 template <template <typename CharT> class ParseHandler, typename CharT>
@@ -9455,17 +9409,17 @@ Parser<ParseHandler, CharT>::bindingIden
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
 Parser<ParseHandler, CharT>::identifierReference(Handle<PropertyName*> name)
 {
     Node pn = newName(name);
     if (!pn)
         return null();
 
-    if (!pc->inDestructuringDecl && !noteUsedName(name))
+    if (!noteUsedName(name))
         return null();
 
     return pn;
 }
 
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
 Parser<ParseHandler, CharT>::stringLiteral()
@@ -9606,17 +9560,18 @@ DoubleToAtom(JSContext* cx, double value
 {
     // This is safe because doubles can not be moved.
     Value tmp = DoubleValue(value);
     return ToAtom<CanGC>(cx, HandleValue::fromMarkedLocation(&tmp));
 }
 
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
-Parser<ParseHandler, CharT>::propertyName(YieldHandling yieldHandling, Node propList,
+Parser<ParseHandler, CharT>::propertyName(YieldHandling yieldHandling,
+                                          const Maybe<DeclarationKind>& maybeDecl, Node propList,
                                           PropertyType* propType, MutableHandleAtom propAtom)
 {
     TokenKind ltok;
     if (!tokenStream.getToken(&ltok))
         return null();
 
     MOZ_ASSERT(ltok != TOK_RC, "caller should have handled TOK_RC");
 
@@ -9673,17 +9628,17 @@ Parser<ParseHandler, CharT>::propertyNam
         if (!propAtom.get())
             return null();
         propName = newNumber(tokenStream.currentToken());
         if (!propName)
             return null();
         break;
 
       case TOK_LB:
-        propName = computedPropertyName(yieldHandling, propList);
+        propName = computedPropertyName(yieldHandling, maybeDecl, propList);
         if (!propName)
             return null();
         break;
 
       default: {
         if (!TokenKindIsPossibleIdentifierName(ltok)) {
             error(JSMSG_UNEXPECTED_TOKEN, "property name", TokenKindToDesc(ltok));
             return null();
@@ -9731,17 +9686,17 @@ Parser<ParseHandler, CharT>::propertyNam
             propAtom.set(DoubleToAtom(context, tokenStream.currentToken().number()));
             if (!propAtom.get())
                 return null();
             return newNumber(tokenStream.currentToken());
         }
         if (tt == TOK_LB) {
             tokenStream.consumeKnownToken(TOK_LB);
 
-            return computedPropertyName(yieldHandling, propList);
+            return computedPropertyName(yieldHandling, maybeDecl, propList);
         }
 
         // Not an accessor property after all.
         propName = handler.newObjectLiteralPropertyName(propAtom.get(), pos());
         if (!propName)
             return null();
         break;
       }
@@ -9803,38 +9758,35 @@ Parser<ParseHandler, CharT>::propertyNam
     }
 
     error(JSMSG_COLON_AFTER_ID);
     return null();
 }
 
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
-Parser<ParseHandler, CharT>::computedPropertyName(YieldHandling yieldHandling, Node literal)
+Parser<ParseHandler, CharT>::computedPropertyName(YieldHandling yieldHandling,
+                                                  const Maybe<DeclarationKind>& maybeDecl,
+                                                  Node literal)
 {
     uint32_t begin = pos().begin;
 
-    Node assignNode;
-    {
-        // Turn off the inDestructuringDecl flag when parsing computed property
-        // names. In short, when parsing 'let {[x + y]: z} = obj;', noteUsedName()
-        // should be called on x and y, but not on z. See the comment on
-        // Parser<>::checkDestructuringAssignmentPattern() for details.
-        AutoClearInDestructuringDecl autoClear(pc);
-        assignNode = assignExpr(InAllowed, yieldHandling, TripledotProhibited);
-        if (!assignNode)
-            return null();
-    }
+    if (maybeDecl) {
+        if (*maybeDecl == DeclarationKind::FormalParameter)
+            pc->functionBox()->hasParameterExprs = true;
+    } else {
+        handler.setListFlag(literal, PNX_NONCONST);
+    }
+
+    Node assignNode = assignExpr(InAllowed, yieldHandling, TripledotProhibited);
+    if (!assignNode)
+        return null();
 
     MUST_MATCH_TOKEN(TOK_RB, JSMSG_COMP_PROP_UNTERM_EXPR);
-    Node propname = handler.newComputedName(assignNode, begin, pos().end);
-    if (!propname)
-        return null();
-    handler.setListFlag(literal, PNX_NONCONST);
-    return propname;
+    return handler.newComputedName(assignNode, begin, pos().end);
 }
 
 template <template <typename CharT> class ParseHandler, typename CharT>
 typename ParseHandler<CharT>::Node
 Parser<ParseHandler, CharT>::objectLiteral(YieldHandling yieldHandling,
                                            PossibleError* possibleError)
 {
     MOZ_ASSERT(tokenStream.isCurrentTokenType(TOK_LC));
@@ -9842,30 +9794,31 @@ Parser<ParseHandler, CharT>::objectLiter
     uint32_t openedPos = pos().begin;
 
     Node literal = handler.newObjectLiteral(pos().begin);
     if (!literal)
         return null();
 
     bool seenPrototypeMutation = false;
     bool seenCoverInitializedName = false;
+    Maybe<DeclarationKind> declKind = Nothing();
     RootedAtom propAtom(context);
     for (;;) {
         TokenKind tt;
         if (!tokenStream.getToken(&tt))
             return null();
         if (tt == TOK_RC)
             break;
 
         TokenPos namePos = pos();
 
         tokenStream.ungetToken();
 
         PropertyType propType;
-        Node propName = propertyName(yieldHandling, literal, &propType, &propAtom);
+        Node propName = propertyName(yieldHandling, declKind, literal, &propType, &propAtom);
         if (!propName)
             return null();
 
         if (propType == PropertyType::Normal) {
             Node propExpr = assignExpr(InAllowed, yieldHandling, TripledotProhibited,
                                        possibleError);
             if (!propExpr)
                 return null();
@@ -9949,25 +9902,19 @@ Parser<ParseHandler, CharT>::objectLiter
                 }
 
                 // Here we set a pending error so that later in the parse, once we've
                 // determined whether or not we're destructuring, the error can be
                 // reported or ignored appropriately.
                 possibleError->setPendingExpressionErrorAt(pos(), JSMSG_COLON_AFTER_ID);
             }
 
-            Node rhs;
-            {
-                // Clearing `inDestructuringDecl` allows name use to be noted
-                // in Parser::identifierReference. See bug 1255167.
-                AutoClearInDestructuringDecl autoClear(pc);
-                rhs = assignExpr(InAllowed, yieldHandling, TripledotProhibited);
-                if (!rhs)
-                    return null();
-            }
+            Node rhs = assignExpr(InAllowed, yieldHandling, TripledotProhibited);
+            if (!rhs)
+                return null();
 
             handler.checkAndSetIsDirectRHSAnonFunction(rhs);
 
             Node propExpr = handler.newAssignment(PNK_ASSIGN, lhs, rhs, JSOP_NOP);
             if (!propExpr)
                 return null();
 
             if (!handler.addPropertyDefinition(literal, propName, propExpr))
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -335,27 +335,16 @@ class ParseContext : public Nestable<Par
 
     // In a function context, points to a Directive struct that can be updated
     // to reflect new directives encountered in the Directive Prologue that
     // require reparsing the function. In global/module/generator-tail contexts,
     // we don't need to reparse when encountering a DirectivePrologue so this
     // pointer may be nullptr.
     Directives* newDirectives;
 
-    // Set when parsing a declaration-like destructuring pattern.  This flag
-    // causes PrimaryExpr to create PN_NAME parse nodes for variable references
-    // which are not hooked into any definition's use chain, added to any tree
-    // context's AtomList, etc. etc.  checkDestructuring will do that work
-    // later.
-    //
-    // The comments atop checkDestructuring explain the distinction between
-    // assignment-like and declaration-like destructuring patterns, and why
-    // they need to be treated differently.
-    mozilla::Maybe<DeclarationKind> inDestructuringDecl;
-
     // Set when parsing a function and it has 'return <expr>;'
     bool funHasReturnExpr;
 
     // Set when parsing a function and it has 'return;'
     bool funHasReturnVoid;
 
   public:
     template <template <typename CharT> class ParseHandler, typename CharT>
@@ -1513,25 +1502,27 @@ class Parser final : public ParserBase, 
     bool noteUsedName(HandlePropertyName name);
     bool hasUsedName(HandlePropertyName name);
 
     // Required on Scope exit.
     bool propagateFreeNamesAndMarkClosedOverBindings(ParseContext::Scope& scope);
 
     Node finishLexicalScope(ParseContext::Scope& scope, Node body);
 
-    Node propertyName(YieldHandling yieldHandling, Node propList,
+    Node propertyName(YieldHandling yieldHandling,
+                      const mozilla::Maybe<DeclarationKind>& maybeDecl, Node propList,
                       PropertyType* propType, MutableHandleAtom propAtom);
-    Node computedPropertyName(YieldHandling yieldHandling, Node literal);
+    Node computedPropertyName(YieldHandling yieldHandling,
+                              const mozilla::Maybe<DeclarationKind>& maybeDecl, Node literal);
     Node arrayInitializer(YieldHandling yieldHandling, PossibleError* possibleError);
     Node newRegExp();
 
     Node objectLiteral(YieldHandling yieldHandling, PossibleError* possibleError);
 
-    Node bindingInitializer(Node lhs, YieldHandling yieldHandling);
+    Node bindingInitializer(Node lhs, DeclarationKind kind, YieldHandling yieldHandling);
     Node bindingIdentifier(DeclarationKind kind, YieldHandling yieldHandling);
     Node bindingIdentifierOrPattern(DeclarationKind kind, YieldHandling yieldHandling,
                                     TokenKind tt);
     Node objectBindingPattern(DeclarationKind kind, YieldHandling yieldHandling);
     Node arrayBindingPattern(DeclarationKind kind, YieldHandling yieldHandling);
 
     // Top-level entrypoint into destructuring assignment pattern checking and
     // name-analyzing.