Bug 1041341 - Part 1: Add support to store a pending destructuring error in PossibleError. r=arai, r=Waldo
authorAndré Bargull <andre.bargull@gmail.com>
Tue, 18 Oct 2016 17:27:47 -0700
changeset 318559 2821a9fdfd258ef73a74608ec4056c186e68b170
parent 318558 29f57d48fdf4789847ec80dfeb47535dc643de4c
child 318560 5c23428eec9b050e4e000dbccb91b6683009adf6
push id7
push usermaklebus@msu.edu
push dateWed, 19 Oct 2016 22:03:57 +0000
reviewersarai, Waldo
bugs1041341
milestone52.0a1
Bug 1041341 - Part 1: Add support to store a pending destructuring error in PossibleError. r=arai, r=Waldo
js/src/frontend/Parser.cpp
js/src/frontend/Parser.h
--- a/js/src/frontend/Parser.cpp
+++ b/js/src/frontend/Parser.cpp
@@ -3726,74 +3726,133 @@ Parser<ParseHandler>::matchLabel(YieldHa
     } else {
         label.set(nullptr);
     }
     return true;
 }
 
 template <typename ParseHandler>
 Parser<ParseHandler>::PossibleError::PossibleError(Parser<ParseHandler>& parser)
-  : parser_(parser),
-    state_(ErrorState::None)
+  : parser_(parser)
 {}
 
 template <typename ParseHandler>
+typename Parser<ParseHandler>::PossibleError::Error&
+Parser<ParseHandler>::PossibleError::error(ErrorKind kind)
+{
+    if (kind == ErrorKind::Expression)
+        return exprError_;
+    MOZ_ASSERT(kind == ErrorKind::Destructuring);
+    return destructuringError_;
+}
+
+template <typename ParseHandler>
 void
-Parser<ParseHandler>::PossibleError::setPending(Node pn, unsigned errorNumber)
+Parser<ParseHandler>::PossibleError::setResolved(ErrorKind kind)
+{
+    error(kind).state_ = ErrorState::None;
+}
+
+template <typename ParseHandler>
+bool
+Parser<ParseHandler>::PossibleError::hasError(ErrorKind kind)
+{
+    return error(kind).state_ == ErrorState::Pending;
+}
+
+template <typename ParseHandler>
+void
+Parser<ParseHandler>::PossibleError::setPending(ErrorKind kind, Node pn, unsigned errorNumber)
 {
     // Don't overwrite a previously recorded error.
-    if (hasError())
+    if (hasError(kind))
         return;
 
     // If we report an error later, we'll do it from the position where we set
     // the state to pending.
-    offset_      = (pn ? parser_.handler.getPosition(pn) : parser_.pos()).begin;
-    errorNumber_ = errorNumber;
-    state_       = ErrorState::Pending;
+    Error& err = error(kind);
+    err.offset_ = (pn ? parser_.handler.getPosition(pn) : parser_.pos()).begin;
+    err.errorNumber_ = errorNumber;
+    err.state_ = ErrorState::Pending;
+}
+
+template <typename ParseHandler>
+void
+Parser<ParseHandler>::PossibleError::setPendingDestructuringError(Node pn, unsigned errorNumber)
+{
+    setPending(ErrorKind::Destructuring, pn, errorNumber);
 }
 
 template <typename ParseHandler>
 void
-Parser<ParseHandler>::PossibleError::setResolved()
-{
-    state_ = ErrorState::None;
+Parser<ParseHandler>::PossibleError::setPendingExpressionError(Node pn, unsigned errorNumber)
+{
+    setPending(ErrorKind::Expression, pn, errorNumber);
+}
+
+template <typename ParseHandler>
+bool
+Parser<ParseHandler>::PossibleError::checkForError(ErrorKind kind)
+{
+    if (!hasError(kind))
+        return true;
+
+    Error& err = error(kind);
+    parser_.reportWithOffset(ParseError, false, err.offset_, err.errorNumber_);
+    return false;
 }
 
 template <typename ParseHandler>
 bool
-Parser<ParseHandler>::PossibleError::hasError()
-{
-    return state_ == ErrorState::Pending;
+Parser<ParseHandler>::PossibleError::checkForDestructuringError()
+{
+    // Clear pending expression error, because we're definitely not in an
+    // expression context.
+    setResolved(ErrorKind::Expression);
+
+    // Report any pending destructuring error.
+    return checkForError(ErrorKind::Destructuring);
 }
 
 template <typename ParseHandler>
 bool
-Parser<ParseHandler>::PossibleError::checkForExprErrors()
-{
-    if (hasError()) {
-        parser_.reportWithOffset(ParseError, false, offset_, errorNumber_);
-        return false;
-    }
-    return true;
+Parser<ParseHandler>::PossibleError::checkForExpressionError()
+{
+    // Clear pending destructuring error, because we're definitely not in a
+    // destructuring context.
+    setResolved(ErrorKind::Destructuring);
+
+    // Report any pending expression error.
+    return checkForError(ErrorKind::Expression);
 }
 
 template <typename ParseHandler>
 void
-Parser<ParseHandler>::PossibleError::transferErrorTo(PossibleError* other)
+Parser<ParseHandler>::PossibleError::transferErrorTo(ErrorKind kind, PossibleError* other)
+{
+    if (hasError(kind) && !other->hasError(kind)) {
+        Error& err = error(kind);
+        Error& otherErr = other->error(kind);
+        otherErr.offset_ = err.offset_;
+        otherErr.errorNumber_ = err.errorNumber_;
+        otherErr.state_ = err.state_;
+    }
+}
+
+template <typename ParseHandler>
+void
+Parser<ParseHandler>::PossibleError::transferErrorsTo(PossibleError* other)
 {
     MOZ_ASSERT(other);
     MOZ_ASSERT(this != other);
     MOZ_ASSERT(&parser_ == &other->parser_,
                "Can't transfer fields to an instance which belongs to a different parser");
 
-    if (hasError() && !other->hasError()) {
-        other->offset_        = offset_;
-        other->errorNumber_   = errorNumber_;
-        other->state_         = state_;
-    }
+    transferErrorTo(ErrorKind::Destructuring, other);
+    transferErrorTo(ErrorKind::Expression, other);
 }
 
 template <typename ParseHandler>
 bool
 Parser<ParseHandler>::checkAssignmentToCall(Node target, unsigned msg)
 {
     MOZ_ASSERT(handler.isFunctionCall(target));
 
@@ -3972,20 +4031,19 @@ Parser<FullParseHandler>::checkDestructu
         report(ParseError, false, pattern, JSMSG_ARRAY_COMP_LEFTSIDE);
         return false;
     }
 
     bool isDestructuring = pattern->isKind(PNK_ARRAY)
                            ? checkDestructuringArray(pattern, maybeDecl)
                            : checkDestructuringObject(pattern, maybeDecl);
 
-    // Resolve asap instead of checking since we already know that we are
-    // destructuring.
-    if (isDestructuring && possibleError)
-        possibleError->setResolved();
+    // Report any pending destructuring error.
+    if (isDestructuring && possibleError && !possibleError->checkForDestructuringError())
+        return false;
 
     return isDestructuring;
 }
 
 template <>
 bool
 Parser<SyntaxParseHandler>::checkDestructuringPattern(Node pattern,
                                                       Maybe<DeclarationKind> maybeDecl,
@@ -5234,17 +5292,17 @@ Parser<ParseHandler>::forHeadStart(Yield
     if (!matchInOrOf(&isForIn, &isForOf))
         return false;
 
     // If we don't encounter 'in'/'of', we have a for(;;) loop.  We've handled
     // the init expression; the caller handles the rest.  Allow the Operand
     // modifier when regetting: Operand must be used to examine the ';' in
     // |for (;|, and our caller handles this case and that.
     if (!isForIn && !isForOf) {
-        if (!possibleError.checkForExprErrors())
+        if (!possibleError.checkForExpressionError())
             return false;
         *forHeadKind = PNK_FORHEAD;
         tokenStream.addModifierException(TokenStream::OperandIsNone);
         return true;
     }
 
     MOZ_ASSERT(isForIn != isForOf);
 
@@ -5262,17 +5320,17 @@ Parser<ParseHandler>::forHeadStart(Yield
         report(ParseError, false, *forInitialPart, JSMSG_LET_STARTING_FOROF_LHS);
         return false;
     }
 
     *forHeadKind = isForIn ? PNK_FORIN : PNK_FOROF;
 
     if (!validateForInOrOfLHSExpression(*forInitialPart, &possibleError))
         return false;
-    if (!possibleError.checkForExprErrors())
+    if (!possibleError.checkForExpressionError())
         return false;
 
     // Finally, parse the iterated expression, making the for-loop's closing
     // ')' the next token.
     *forInOrOfExpression = expressionAfterForInOrOf(*forHeadKind, yieldHandling);
     return *forInOrOfExpression != null();
 }
 
@@ -6984,20 +7042,20 @@ Parser<ParseHandler>::expr(InHandling in
         PossibleError possibleErrorInner(*this);
         pn = assignExpr(inHandling, yieldHandling, tripledotHandling,
                         &possibleErrorInner);
         if (!pn)
             return null();
 
         if (!possibleError) {
             // Report any pending expression error.
-            if (!possibleErrorInner.checkForExprErrors())
+            if (!possibleErrorInner.checkForExpressionError())
                 return null();
         } else {
-            possibleErrorInner.transferErrorTo(possibleError);
+            possibleErrorInner.transferErrorsTo(possibleError);
         }
 
         handler.addList(seq, pn);
 
         if (!tokenStream.matchToken(&matched, TOK_COMMA))
             return null();
         if (!matched)
             break;
@@ -7115,17 +7173,17 @@ Parser<ParseHandler>::orExpr1(InHandling
         TokenKind tok;
         if (!tokenStream.getToken(&tok))
             return null();
 
         ParseNodeKind pnk;
         if (tok == TOK_IN ? inHandling == InAllowed : TokenKindIsBinaryOp(tok)) {
             // We're definitely not in a destructuring context, so report any
             // pending expression error now.
-            if (possibleError && !possibleError->checkForExprErrors())
+            if (possibleError && !possibleError->checkForExpressionError())
                 return null();
             // Report an error for unary expressions on the LHS of **.
             if (tok == TOK_POW && handler.isUnparenthesizedUnaryExpression(pn)) {
                 report(ParseError, false, null(), JSMSG_BAD_POW_LEFTSIDE);
                 return null();
             }
             pnk = BinaryOpTokenKindToParseNodeKind(tok);
         } else {
@@ -7399,29 +7457,29 @@ Parser<ParseHandler>::assignExpr(InHandl
             tokenStream.addModifierException(TokenStream::NoneIsOperand);
         }
         return arrowFunc;
       }
 
       default:
         MOZ_ASSERT(!tokenStream.isCurrentTokenAssignment());
         if (!possibleError) {
-            if (!possibleErrorInner.checkForExprErrors())
+            if (!possibleErrorInner.checkForExpressionError())
                 return null();
         } else {
-            possibleErrorInner.transferErrorTo(possibleError);
+            possibleErrorInner.transferErrorsTo(possibleError);
         }
         tokenStream.ungetToken();
         return lhs;
     }
 
     AssignmentFlavor flavor = kind == PNK_ASSIGN ? PlainAssignment : CompoundAssignment;
     if (!checkAndMarkAsAssignmentLhs(lhs, flavor, &possibleErrorInner))
         return null();
-    if (!possibleErrorInner.checkForExprErrors())
+    if (!possibleErrorInner.checkForExpressionError())
         return null();
 
     Node rhs;
     {
         AutoClearInDestructuringDecl autoClear(pc);
         rhs = assignExpr(inHandling, yieldHandling, TripledotProhibited);
         if (!rhs)
             return null();
@@ -8764,17 +8822,18 @@ Parser<ParseHandler>::objectLiteral(Yiel
                     // destructuring context.
                     if (!possibleError) {
                         report(ParseError, false, propName, JSMSG_DUPLICATE_PROTO_PROPERTY);
                         return null();
                     }
 
                     // Otherwise delay error reporting until we've determined
                     // whether or not we're destructuring.
-                    possibleError->setPending(propName, JSMSG_DUPLICATE_PROTO_PROPERTY);
+                    possibleError->setPendingExpressionError(propName,
+                                                             JSMSG_DUPLICATE_PROTO_PROPERTY);
                 }
                 seenPrototypeMutation = true;
 
                 // Note: this occurs *only* if we observe TOK_COLON!  Only
                 // __proto__: v mutates [[Prototype]].  Getters, setters,
                 // method/generator definitions, computed property name
                 // versions of all of these, and shorthands do not.
                 uint32_t begin = handler.getPosition(propName).begin;
@@ -8849,17 +8908,17 @@ Parser<ParseHandler>::objectLiteral(Yiel
                     // For example, maybe the preceding token is an operator: `x + {y=z}`.
                     report(ParseError, false, null(), JSMSG_COLON_AFTER_ID);
                     return null();
                 }
 
                 // 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->setPending(null(), JSMSG_COLON_AFTER_ID);
+                possibleError->setPendingExpressionError(null(), 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);
--- a/js/src/frontend/Parser.h
+++ b/js/src/frontend/Parser.h
@@ -731,70 +731,115 @@ class Parser final : private JS::AutoGCR
      * The ability to stash an error is useful for handling situations where we
      * aren't able to verify that an error has occurred until later in the parse.
      * For instance | ({x=1}) | is always parsed as an object literal with
      * a SyntaxError, however, in the case where it is followed by '=>' we rewind
      * and reparse it as a valid arrow function. Here a PossibleError would be
      * set to 'pending' when the initial SyntaxError was encountered then 'resolved'
      * just before rewinding the parser.
      *
+     * There are currently two kinds of PossibleErrors: Expression and
+     * Destructuring errors. Expression errors are used to mark a possible
+     * syntax error when a grammar production is used in an expression context.
+     * For example in |{x = 1}|, we mark the CoverInitializedName |x = 1| as a
+     * possible expression error, because CoverInitializedName productions
+     * are disallowed when an actual ObjectLiteral is expected.
+     * Destructuring errors are used to record possible syntax errors in
+     * destructuring contexts. For example in |[...rest, ] = []|, we initially
+     * mark the trailing comma after the spread expression as a possible
+     * destructuring error, because the ArrayAssignmentPattern grammar
+     * production doesn't allow a trailing comma after the rest element.
+     *
      * When using PossibleError one should set a pending error at the location
      * where an error occurs. From that point, the error may be resolved
      * (invalidated) or left until the PossibleError is checked.
      *
      * Ex:
      *   PossibleError possibleError(*this);
-     *   possibleError.setPending(pn, JSMSG_BAD_PROP_ID);
+     *   possibleError.setPendingExpressionError(pn, JSMSG_BAD_PROP_ID);
      *   // A JSMSG_BAD_PROP_ID ParseError is reported, returns false.
-     *   possibleError.checkForExprErrors();
+     *   if (!possibleError.checkForExpressionError())
+     *       return false; // we reach this point with a pending exception
      *
      *   PossibleError possibleError(*this);
-     *   possibleError.setPending(pn, JSMSG_BAD_PROP_ID);
-     *   possibleError.setResolved();
+     *   possibleError.setPendingExpressionError(pn, JSMSG_BAD_PROP_ID);
      *   // Returns true, no error is reported.
-     *   possibleError.checkForExprErrors();
+     *   if (!possibleError.checkForDestructuringError())
+     *       return false; // not reached, no pending exception
      *
      *   PossibleError possibleError(*this);
      *   // Returns true, no error is reported.
-     *   possibleError.checkForExprErrors();
+     *   if (!possibleError.checkForExpressionError())
+     *       return false; // not reached, no pending exception
      */
     class MOZ_STACK_CLASS PossibleError
     {
-      protected:
+      private:
+        enum class ErrorKind { Expression, Destructuring };
+
+        enum class ErrorState { None, Pending };
+
+        struct Error {
+            ErrorState state_ = ErrorState::None;
+
+            // Error reporting fields.
+            uint32_t offset_;
+            unsigned errorNumber_;
+        };
+
         Parser<ParseHandler>& parser_;
+        Error exprError_;
+        Error destructuringError_;
 
-        enum ErrorState { None, Pending };
-        ErrorState state_;
+        // Returns the error report.
+        Error& error(ErrorKind kind);
+
+        // Return true if an error is pending without reporting
+        bool hasError(ErrorKind kind);
+
+        // Resolve any pending error.
+        void setResolved(ErrorKind kind);
 
-        // Error reporting fields.
-        uint32_t offset_;
-        unsigned errorNumber_;
+        // Set a pending error. Only a single error may be set per instance and
+        // error kind.
+        void setPending(ErrorKind kind, Node pn, unsigned errorNumber);
+
+        // If there is a pending error, report it and return false, otherwise
+        // return true.
+        bool checkForError(ErrorKind kind);
+
+        // Transfer an existing error to another instance.
+        void transferErrorTo(ErrorKind kind, PossibleError* other);
 
       public:
         explicit PossibleError(Parser<ParseHandler>& parser);
 
-        // Set a pending error. Only a single error may be set per instance.
-        // Returns true on success or false on failure.
-        void setPending(Node pn, unsigned errorNumber);
-
-        // Resolve any pending error.
-        void setResolved();
+        // Set a pending destructuring error. Only a single error may be set
+        // per instance, i.e. subsequent calls to this method are ignored and
+        // won't overwrite the existing pending error.
+        void setPendingDestructuringError(Node pn, unsigned errorNumber);
 
-        // Return true if an error is pending without reporting
-        bool hasError();
+        // Set a pending expression error. Only a single error may be set per
+        // instance, i.e. subsequent calls to this method are ignored and won't
+        // overwrite the existing pending error.
+        void setPendingExpressionError(Node pn, unsigned errorNumber);
 
-        // If there is a pending error report it and return false, otherwise return
-        // true.
-        bool checkForExprErrors();
+        // If there is a pending destructuring error, report it and return
+        // false, otherwise return true. Clears any pending expression error.
+        bool checkForDestructuringError();
+
+        // If there is a pending expression error, report it and return false,
+        // otherwise return true. Clears any pending destructuring error.
+        bool checkForExpressionError();
 
         // Pass pending errors between possible error instances. This is useful
         // for extending the lifetime of a pending error beyond the scope of
         // the PossibleError where it was initially set (keeping in mind that
         // PossibleError is a MOZ_STACK_CLASS).
-        void transferErrorTo(PossibleError* other);
+        void transferErrorsTo(PossibleError* other);
     };
 
   public:
     ExclusiveContext* const context;
 
     LifoAlloc& alloc;
 
     TokenStream tokenStream;