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 318457 2821a9fdfd25
parent 318456 29f57d48fdf4
child 318458 5c23428eec9b
push id82943
push userryanvm@gmail.com
push dateWed, 19 Oct 2016 02:17:19 +0000
treeherdermozilla-inbound@3b4965bb11c0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai, Waldo
bugs1041341
milestone52.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 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;