Set up rewriting triggers

src/parsing/parser-base.h

Index: src/parsing/parser-base.h
diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h
index 732c7783d7eb783897930301624ba688bee999c3..35aee16b0a9b1f9b1f723d14acbe0808289dc590 100644
--- a/src/parsing/parser-base.h
+++ b/src/parsing/parser-base.h
@@ -1037,8 +1037,10 @@ typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParseIdentifier(
     ValidateBindingPattern(&classifier, ok);
     if (!*ok) return Traits::EmptyIdentifier();
   } else {
+#if 0  // TODO(nikolaos): is this needed?

[rossberg, 2016/01/12 13:47:52]

Yes, this is needed -- ParseAndClassifyIdentifier may set expression errors in
some cases (e.g. using `arguments` in strong mode).

[nickie, 2016/01/12 14:12:32]

Acknowledged.  I'm adding a test for this.

[nickie, 2016/01/12 15:16:09]

Cannot find a test for this.  This is the "else" case, when the use of
restricted identifiers is allowed.  Is there something to validate in this case?

[rossberg, 2016/01/14 13:28:40]

Yes, I don't understand why this is the else case either. Shouldn't it occur in
the then case?

[nickie, 2016/01/14 14:47:14]

In the "then" case there is some validation.  But this thing is just an
identifier and not yet found to be a non-pattern, so no rewriting must take
place there (and I believe that calling ValidateExpression would be wrong).  On
top of everything, we're talking about rewriting an identifier which will
probably never be non-trivial...

     ValidateExpression(&classifier, ok);
     if (!*ok) return Traits::EmptyIdentifier();
+#endif
   }
 
   return result;
@@ -1354,7 +1356,9 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
                                                classifier, CHECK_OK);
       Expect(Token::RPAREN, CHECK_OK);
       if (peek() != Token::ARROW) {
+#if 0  // TODO(nikolaos): is this needed?

[rossberg, 2016/01/12 13:47:52]

Isn't this needed to catch syntax errors like "(());"? Why is Rewrite not needed
here?

[caitp (gmail), 2016/01/12 13:50:39]

This is needed to catch things like CoverInitializedNames in things that didn't
turn out to be Arrow formal parameters

[nickie, 2016/01/12 14:12:32]

Acknowledged.  I'm adding a test for this.

[nickie, 2016/01/12 15:16:09]

Cannot find a test for this either.  It seems to me that errors like "(())" will
be caught when this whole thing is rewritten and validated.  This may be at the
place where somebody called ParsePrimaryExpression, or indeed at the place where
somebody called a chain that is a suffix of:

    ParseExpression
    ParseAssignmentExpression
    ParseConditionalExpression
    ParseBinaryExpression
    ParseUnaryExpression
    ParsePostfixExpression
    ParseLeftHandSideExpression
    ParseMemberExpressionWithNewPrefixesExpression
    ParseMemberExpression
    ParsePrimaryExpression

[nickie, 2016/01/13 10:26:06]

After Caitlin's patch for parenthesized patterns, I believe this is not needed
anymore.  It doesn't hurt much to have a validate-and-rewrite here, but notice
that this expression will certainly be (validated and) rewritten again, so we're
sort of violating our invariant that this non-pattern rewriting phase should be
kept as "shallow" as possible.

[rossberg, 2016/01/14 13:28:40]

Hm, I wonder if it isn't a broader shortcoming that parenthesised get rewritten
again (also in other cases). Shouldn't RewriteNonPattern stop when it hits an
expression with the is_parenthesised flag?

[nickie, 2016/01/14 14:47:14]

There's two approaches, as I understand it.  Either we remove the validation
from here and change nothing else, or we add the validation here and we have
RewriteNonPattern stop when it hits expressions with the is_parenthesised flag
set.  I have taken the first approach, see here:

https://codereview.chromium.org/1567603005/diff/100001/src/parsing/parser-bas...

but if you prefer the second, I can change it.

[caitp (gmail), 2016/01/14 15:06:25]

Just a note about why the ValidateExpression was originally added, it was
because the ParseExpression method was changed from one that did its own
ValidateExpression(), to one that doesn't (because arrow formals needed to be
validated differently).

For whatever reason, it seems that the correct validation is happening
elsewhere. So, so long as you can't write `({ x = 10 });` or `({ x: { y = 10 }
});` without a SyntaxError, it should be fine to just remove the validation.

This is unrelated to the parenthesized flag, it's all making sure
ExpressionErrors are only emitted in an Expression context, and at this point
it's ambiguous whether it's an Expression context or not, until the `=>` is seen
or not seen

         ValidateExpression(classifier, CHECK_OK);
+#endif
         expr->set_is_parenthesized();
       }
       return expr;
@@ -1419,8 +1423,7 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseExpression(
     bool accept_IN, bool* ok) {
   ExpressionClassifier classifier;
   ExpressionT result = ParseExpression(accept_IN, &classifier, CHECK_OK);
-  ValidateExpression(&classifier, CHECK_OK);
-  result = Traits::RewriteExpression(result);
+  result = Traits::RewriteExpression(result, &classifier, CHECK_OK);
   return result;
 }
 
@@ -1586,7 +1589,8 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParsePropertyName(
       ExpressionClassifier computed_name_classifier;
       ExpressionT expression =
           ParseAssignmentExpression(true, &computed_name_classifier, CHECK_OK);
-      expression = Traits::RewriteExpression(expression);
+      expression = Traits::RewriteExpression(
+          expression, &computed_name_classifier, CHECK_OK);
       classifier->Accumulate(computed_name_classifier,
                              ExpressionClassifier::ExpressionProductions);
       Expect(Token::RBRACK, CHECK_OK);
@@ -1700,7 +1704,8 @@ ParserBase<Traits>::ParsePropertyDefinition(
         ExpressionClassifier rhs_classifier;
         ExpressionT rhs = this->ParseAssignmentExpression(
             true, &rhs_classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-        rhs = Traits::RewriteExpression(rhs);
+        rhs = Traits::RewriteExpression(
+            rhs, &rhs_classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
         classifier->Accumulate(rhs_classifier,
                                ExpressionClassifier::ExpressionProductions);
         value = factory()->NewAssignment(Token::ASSIGN, lhs, rhs,
@@ -1768,7 +1773,7 @@ ParserBase<Traits>::ParsePropertyDefinition(
     ObjectLiteralPropertyT property = ParsePropertyDefinition(
         checker, true, has_extends, true, is_computed_name, nullptr, classifier,
         name, ok);
-    property = Traits::RewriteObjectLiteralProperty(property);
+    property = Traits::RewriteObjectLiteralProperty(property, classifier, ok);
     return property;
   }
 
@@ -1908,7 +1913,8 @@ typename Traits::Type::ExpressionList ParserBase<Traits>::ParseArguments(
 
     ExpressionT argument = this->ParseAssignmentExpression(
         true, classifier, CHECK_OK_CUSTOM(NullExpressionList));
-    argument = Traits::RewriteExpression(argument);
+    argument = Traits::RewriteExpression(argument, classifier,
+                                         CHECK_OK_CUSTOM(NullExpressionList));
     if (is_spread) {
       if (!spread_arg.IsValid()) {
         spread_arg.beg_pos = start_pos;
@@ -2047,9 +2053,10 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, int flags,
       CheckDestructuringElement(expression, classifier, lhs_beg_pos,
                                 scanner()->location().end_pos);
     } else if (is_rhs && maybe_pattern) {
+#if 0  // TODO(nikolaos): is this needed?

[rossberg, 2016/01/12 13:47:52]

Hm, why is it not needed? Don't you even have to do RewriteExpression here?
Can't this arise as "x = ()", for example?

[nickie, 2016/01/12 14:12:32]

I have a RewriteExpression at the place where the RHS of the assignment is
parsed, so now it will be validated there.  Probably the rhs flag is not needed
any more, @caitp?

[caitp (gmail), 2016/01/12 14:25:55]

It looks like you can remove this block, and the flag, but it's possible that
I've missed something

[nickie, 2016/01/12 15:16:09]

Acknowledged.

       ValidateExpression(classifier, CHECK_OK);
+#endif
     }
-
     return expression;
   }
 
@@ -2094,7 +2101,7 @@ ParserBase<Traits>::ParseAssignmentExpression(bool accept_IN, int flags,
   rhs_flags |= kIsRightHandSide;
   ExpressionT right = this->ParseAssignmentExpression(
       accept_IN, rhs_flags, &rhs_classifier, CHECK_OK);
-  right = Traits::RewriteExpression(right);
+  right = Traits::RewriteExpression(right, &rhs_classifier, CHECK_OK);
   classifier->Accumulate(
       rhs_classifier, ExpressionClassifier::ExpressionProductions |
                           ExpressionClassifier::CoverInitializedNameProduction);
@@ -2166,7 +2173,8 @@ ParserBase<Traits>::ParseYieldExpression(ExpressionClassifier* classifier,
         // Delegating yields require an RHS; fall through.
       default:
         expression = ParseAssignmentExpression(false, classifier, CHECK_OK);
-        expression = Traits::RewriteExpression(expression);
+        expression =
+            Traits::RewriteExpression(expression, classifier, CHECK_OK);
         break;
     }
   }
@@ -2199,7 +2207,7 @@ ParserBase<Traits>::ParseConditionalExpression(bool accept_IN,
   ExpressionT expression =
       this->ParseBinaryExpression(4, accept_IN, classifier, CHECK_OK);
   if (peek() != Token::CONDITIONAL) return expression;
-  expression = Traits::RewriteExpression(expression);
+  expression = Traits::RewriteExpression(expression, classifier, CHECK_OK);
   ArrowFormalParametersUnexpectedToken(classifier);
   BindingPatternUnexpectedToken(classifier);
   Consume(Token::CONDITIONAL);
@@ -2207,11 +2215,11 @@ ParserBase<Traits>::ParseConditionalExpression(bool accept_IN,
   // expressions we always accept the 'in' keyword; see ECMA-262,
   // section 11.12, page 58.
   ExpressionT left = ParseAssignmentExpression(true, classifier, CHECK_OK);
-  left = Traits::RewriteExpression(left);
+  left = Traits::RewriteExpression(left, classifier, CHECK_OK);
   Expect(Token::COLON, CHECK_OK);
   ExpressionT right =
       ParseAssignmentExpression(accept_IN, classifier, CHECK_OK);
-  right = Traits::RewriteExpression(right);
+  right = Traits::RewriteExpression(right, classifier, CHECK_OK);
   return factory()->NewConditional(expression, left, right, pos);
 }
 
@@ -2227,7 +2235,7 @@ ParserBase<Traits>::ParseBinaryExpression(int prec, bool accept_IN,
   for (int prec1 = Precedence(peek(), accept_IN); prec1 >= prec; prec1--) {
     // prec1 >= 4
     while (Precedence(peek(), accept_IN) == prec1) {
-      x = Traits::RewriteExpression(x);
+      x = Traits::RewriteExpression(x, classifier, CHECK_OK);
       BindingPatternUnexpectedToken(classifier);
       ArrowFormalParametersUnexpectedToken(classifier);
       Token::Value op = Next();
@@ -2235,7 +2243,7 @@ ParserBase<Traits>::ParseBinaryExpression(int prec, bool accept_IN,
       int pos = position();
       ExpressionT y =
           ParseBinaryExpression(prec1 + 1, accept_IN, classifier, CHECK_OK);
-      y = Traits::RewriteExpression(y);
+      y = Traits::RewriteExpression(y, classifier, CHECK_OK);
 
       if (this->ShortcutNumericLiteralBinaryExpression(&x, y, op, pos,
                                                        factory())) {
@@ -2298,7 +2306,7 @@ ParserBase<Traits>::ParseUnaryExpression(ExpressionClassifier* classifier,
     op = Next();
     int pos = position();
     ExpressionT expression = ParseUnaryExpression(classifier, CHECK_OK);
-    expression = Traits::RewriteExpression(expression);
+    expression = Traits::RewriteExpression(expression, classifier, CHECK_OK);
 
     if (op == Token::DELETE && is_strict(language_mode())) {
       if (is_strong(language_mode())) {
@@ -2325,7 +2333,7 @@ ParserBase<Traits>::ParseUnaryExpression(ExpressionClassifier* classifier,
         expression, beg_pos, scanner()->location().end_pos,
         MessageTemplate::kInvalidLhsInPrefixOp, CHECK_OK);
     this->MarkExpressionAsAssigned(expression);
-    expression = Traits::RewriteExpression(expression);
+    expression = Traits::RewriteExpression(expression, classifier, CHECK_OK);
 
     return factory()->NewCountOperation(op,
                                         true /* prefix */,
@@ -2357,6 +2365,7 @@ ParserBase<Traits>::ParsePostfixExpression(ExpressionClassifier* classifier,
         expression, lhs_beg_pos, scanner()->location().end_pos,
         MessageTemplate::kInvalidLhsInPostfixOp, CHECK_OK);
     expression = this->MarkExpressionAsAssigned(expression);
+    expression = Traits::RewriteExpression(expression, classifier, CHECK_OK);
 
     Token::Value next = Next();
     expression =
@@ -2387,14 +2396,14 @@ ParserBase<Traits>::ParseLeftHandSideExpression(
         Consume(Token::LBRACK);
         int pos = position();
         ExpressionT index = ParseExpression(true, classifier, CHECK_OK);
-        index = Traits::RewriteExpression(index);
+        index = Traits::RewriteExpression(index, classifier, CHECK_OK);
         result = factory()->NewProperty(result, index, pos);
         Expect(Token::RBRACK, CHECK_OK);
         break;
       }
 
       case Token::LPAREN: {
-        result = Traits::RewriteExpression(result);
+        result = Traits::RewriteExpression(result, classifier, CHECK_OK);
         BindingPatternUnexpectedToken(classifier);
         ArrowFormalParametersUnexpectedToken(classifier);
 
@@ -2522,7 +2531,7 @@ ParserBase<Traits>::ParseMemberWithNewPrefixesExpression(
     } else {
       result = this->ParseMemberWithNewPrefixesExpression(classifier, CHECK_OK);
     }
-    result = Traits::RewriteExpression(result);
+    result = Traits::RewriteExpression(result, classifier, CHECK_OK);
     if (peek() == Token::LPAREN) {
       // NewExpression with arguments.
       Scanner::Location spread_pos;
@@ -2622,7 +2631,7 @@ ParserBase<Traits>::ParseStrongInitializationExpression(
       Consume(Token::LBRACK);
       int pos = position();
       ExpressionT index = this->ParseExpression(true, classifier, CHECK_OK);
-      index = Traits::RewriteExpression(index);
+      index = Traits::RewriteExpression(index, classifier, CHECK_OK);
       left = factory()->NewProperty(this_expr, index, pos);
       if (fni_ != NULL) {
         this->PushPropertyName(fni_, index);
@@ -2658,7 +2667,7 @@ ParserBase<Traits>::ParseStrongInitializationExpression(
 
   ExpressionT right =
       this->ParseAssignmentExpression(true, classifier, CHECK_OK);
-  right = Traits::RewriteExpression(right);
+  right = Traits::RewriteExpression(right, classifier, CHECK_OK);
   this->CheckAssigningFunctionLiteralToProperty(left, right);
   function_state_->AddProperty();
   if (fni_ != NULL) {
@@ -2815,7 +2824,7 @@ ParserBase<Traits>::ParseMemberExpressionContinuation(
         Consume(Token::LBRACK);
         int pos = position();
         ExpressionT index = this->ParseExpression(true, classifier, CHECK_OK);
-        index = Traits::RewriteExpression(index);
+        index = Traits::RewriteExpression(index, classifier, CHECK_OK);
         expression = factory()->NewProperty(expression, index, pos);
         if (fni_ != NULL) {
           this->PushPropertyName(fni_, index);
@@ -2896,10 +2905,9 @@ void ParserBase<Traits>::ParseFormalParameter(
     ExpressionClassifier init_classifier;
     initializer = ParseAssignmentExpression(true, &init_classifier, ok);
     if (!*ok) return;
-    ValidateExpression(&init_classifier, ok);
+    initializer = Traits::RewriteExpression(initializer, &init_classifier, ok);
     ValidateFormalParameterInitializer(&init_classifier, ok);
     if (!*ok) return;
-    initializer = Traits::RewriteExpression(initializer);
     parameters->is_simple = false;
     classifier->RecordNonSimpleParameter();
   }
@@ -3071,8 +3079,7 @@ ParserBase<Traits>::ParseArrowFunctionLiteral(
       ExpressionClassifier classifier;
       ExpressionT expression =
           ParseAssignmentExpression(accept_IN, &classifier, CHECK_OK);
-      ValidateExpression(&classifier, CHECK_OK);
-      expression = Traits::RewriteExpression(expression);
+      expression = Traits::RewriteExpression(expression, &classifier, CHECK_OK);
       body = this->NewStatementList(1, zone());
       this->AddParameterInitializationBlock(formal_parameters, body, CHECK_OK);
       body->Add(factory()->NewReturnStatement(expression, pos), zone());
@@ -3178,7 +3185,7 @@ ParserBase<Traits>::ParseTemplateLiteral(ExpressionT tag, int start,
 
     int expr_pos = peek_position();
     ExpressionT expression = this->ParseExpression(true, classifier, CHECK_OK);
-    expression = Traits::RewriteExpression(expression);
+    expression = Traits::RewriteExpression(expression, classifier, CHECK_OK);
     Traits::AddTemplateExpression(&ts, expression);
 
     if (peek() != Token::RBRACE) {