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 567064 d6e29a56658e1931b181eb8c6a7a18d6d1c5b152
parent 567063 0ad2e4e4cd72b84f178a7b7f1af70760493da1c9
child 567065 8f2c0ac318b10cac245519338ed8556bcfce09e6
push id55429
push userbmo:hskupin@gmail.com
push dateMon, 24 Apr 2017 10:48:57 +0000
reviewersshu
bugs1303703
milestone55.0a1
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.