Centralize and standardize logic for ExpressionClassifier accumulation

Previously the calls to ExpressionClassifier::Accumulate() each chose
slightly different sets of productions to accumulate, and it turned
out that these were in some cases broader than needed and in some
cases less broad.

The existence of some grab-bag production bitmasks like
ExpressionClassifier::ExpressionProductions made this situation more
error-prone (for example, that production was missing AsyncArrowFormalParametersProduction).

This patch removes all "grab-bags" besides AllProductions. In some of
the places where code was using those grab-bags for convenience, it
switches them to use negation of AllProductions. In other, specifically
those having to do with expressions that are disallowed anywhere in
a sub-expression of a parameter list, I've added a new method on
ExpressionClassifier to centralize the logic.

The aforementioned centralization/addition of
AsyncArrowFormalParametersProduction fixes several cases where we were
failing to report an error for 'await' in some contexts; I've added
those test cases.

The patch also narrows all cases to exactly the set or productions
necessary, with a comment on each explaining the choice.

BUG=v8:4483

Review-Url: https://codereview.chromium.org/2271063002
Cr-Commit-Position: refs/heads/master@{#38918}

src/parsing/expression-classifier.h

@@ -57,23 +57,17 @@ class ExpressionClassifier {
     const char* arg;
   };
 
+  // clang-format off
   enum TargetProduction : unsigned {
 #define DEFINE_PRODUCTION(NAME, CODE) NAME = 1 << CODE,
     ERROR_CODES(DEFINE_PRODUCTION)
 #undef DEFINE_PRODUCTION
 
-        ExpressionProductions =
-            (ExpressionProduction | FormalParameterInitializerProduction |
-             TailCallExpressionProduction),
-    PatternProductions = (BindingPatternProduction |
-                          AssignmentPatternProduction | LetPatternProduction),
-    FormalParametersProductions = (DistinctFormalParametersProduction |
-                                   StrictModeFormalParametersProduction),
-    AllProductions =
-        (ExpressionProductions | PatternProductions |
-         FormalParametersProductions | ArrowFormalParametersProduction |
-         ObjectLiteralProduction | AsyncArrowFormalParametersProduction)
+#define DEFINE_ALL_PRODUCTIONS(NAME, CODE) NAME |
+    AllProductions = ERROR_CODES(DEFINE_ALL_PRODUCTIONS) /* | */ 0
+#undef DEFINE_ALL_PRODUCTIONS
   };
+  // clang-format on
 
   enum FunctionProperties : unsigned {
     NonSimpleParameter = 1 << 0
@@ -373,6 +367,20 @@ class ExpressionClassifier {
         reported_errors_end_;
   }
 
+  // Accumulate errors that can be arbitrarily deep in an expression.
+  // These correspond to the ECMAScript spec's 'Contains' operation
+  // on productions. This includes:
+  //
+  // - YieldExpression is disallowed in arrow parameters in a generator.
+  // - AwaitExpression is disallowed in arrow parameters in an async function.
+  // - AwaitExpression is disallowed in async arrow parameters.
+  //
+  V8_INLINE void AccumulateFormalParameterContainmentErrors(
+      ExpressionClassifier* inner) {
+    Accumulate(inner, FormalParameterInitializerProduction |
+                          AsyncArrowFormalParametersProduction);
+  }
+
   V8_INLINE int GetNonPatternBegin() const { return non_pattern_begin_; }
 
   V8_INLINE void Discard() {

src/parsing/parser-base.h

@@ -1643,10 +1643,16 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePrimaryExpression(
                                           Token::String(Token::ELLIPSIS));
         classifier->RecordNonSimpleParameter();
         ExpressionClassifier binding_classifier(this);
+        // TODO(adamk): The grammar for this is
+        // BindingIdentifier | BindingPattern, so
+        // ParseAssignmentExpression is overkill.
         ExpressionT expr =
             ParseAssignmentExpression(true, &binding_classifier, CHECK_OK);
-        classifier->Accumulate(&binding_classifier,
-                               ExpressionClassifier::AllProductions);
+        // This already couldn't be an expression or a pattern, so the
+        // only remaining possibility is an arrow formal parameter list.
+        classifier->Accumulate(
+            &binding_classifier,
+            ExpressionClassifier::ArrowFormalParametersProduction);
         if (!impl()->IsIdentifier(expr) && !IsValidPattern(expr)) {
           classifier->RecordArrowFormalParametersError(
               Scanner::Location(ellipsis_pos, scanner()->location().end_pos),
@@ -1733,12 +1739,18 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseExpression(
   //   Expression ',' AssignmentExpression
 
   ExpressionT result;
+
+  // No need to accumulate binding pattern-related errors, since
+  // an Expression can't be a binding pattern anyway.
+  static const unsigned kExpressionProductions =
+      ExpressionClassifier::AllProductions &
+      ~(ExpressionClassifier::BindingPatternProduction |
+        ExpressionClassifier::LetPatternProduction);
   {
     ExpressionClassifier binding_classifier(this);
     result =
         ParseAssignmentExpression(accept_IN, &binding_classifier, CHECK_OK);
-    classifier->Accumulate(&binding_classifier,
-                           ExpressionClassifier::AllProductions);
+    classifier->Accumulate(&binding_classifier, kExpressionProductions);
   }
   bool is_simple_parameter_list = impl()->IsIdentifier(result);
   bool seen_rest = false;
@@ -1763,14 +1775,15 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParseExpression(
       ExpressionUnexpectedToken(classifier);
       BindingPatternUnexpectedToken(classifier);
       Consume(Token::ELLIPSIS);
+      // TODO(adamk): If is_rest is true we don't need to parse an assignment
+      // expression, the grammar is BindingIdentifier | BindingPattern.
       seen_rest = is_rest = true;
     }
     int pos = position(), expr_pos = peek_position();
     ExpressionClassifier binding_classifier(this);
     ExpressionT right =
         ParseAssignmentExpression(accept_IN, &binding_classifier, CHECK_OK);
-    classifier->Accumulate(&binding_classifier,
-                           ExpressionClassifier::AllProductions);
+    classifier->Accumulate(&binding_classifier, kExpressionProductions);
     if (is_rest) {
       if (!impl()->IsIdentifier(right) && !IsValidPattern(right)) {
         classifier->RecordArrowFormalParametersError(
@@ -1904,8 +1917,8 @@ typename ParserBase<Impl>::ExpressionT ParserBase<Impl>::ParsePropertyName(
       ExpressionT expression =
           ParseAssignmentExpression(true, &computed_name_classifier, CHECK_OK);
       impl()->RewriteNonPattern(&computed_name_classifier, CHECK_OK);
-      classifier->Accumulate(&computed_name_classifier,
-                             ExpressionClassifier::ExpressionProductions);
+      classifier->AccumulateFormalParameterContainmentErrors(
+          &computed_name_classifier);
       Expect(Token::RBRACK, CHECK_OK);
       return expression;
     }
@@ -2019,8 +2032,7 @@ ParserBase<Impl>::ParsePropertyDefinition(
             true, &rhs_classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
         impl()->RewriteNonPattern(&rhs_classifier,
                                   CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-        classifier->Accumulate(&rhs_classifier,
-                               ExpressionClassifier::ExpressionProductions);
+        classifier->AccumulateFormalParameterContainmentErrors(&rhs_classifier);
         value = factory()->NewAssignment(Token::ASSIGN, lhs, rhs,
                                          kNoSourcePosition);
         classifier->RecordObjectLiteralError(
@@ -2387,29 +2399,25 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN,
   // (including those for binding patterns, since formal parameters can
   // themselves contain binding patterns).
   // Do not merge pending non-pattern expressions yet!
-  unsigned productions =
-      ExpressionClassifier::FormalParametersProductions |
-      ExpressionClassifier::AsyncArrowFormalParametersProduction |
-      ExpressionClassifier::FormalParameterInitializerProduction;
+  unsigned productions = ExpressionClassifier::AllProductions &
+                         ~ExpressionClassifier::ArrowFormalParametersProduction;
 
   // Parenthesized identifiers and property references are allowed as part
-  // of a larger binding pattern, even though parenthesized patterns
+  // of a larger assignment pattern, even though parenthesized patterns
   // themselves are not allowed, e.g., "[(x)] = []". Only accumulate
   // assignment pattern errors if the parsed expression is more complex.
   if (IsValidReferenceExpression(expression)) {
-    productions |= ExpressionClassifier::PatternProductions &
-                   ~ExpressionClassifier::AssignmentPatternProduction;
-  } else {
-    productions |= ExpressionClassifier::PatternProductions;
+    productions &= ~ExpressionClassifier::AssignmentPatternProduction;
   }
 
   const bool is_destructuring_assignment =
       IsValidPattern(expression) && peek() == Token::ASSIGN;
-  if (!is_destructuring_assignment) {
-    // This may be an expression or a pattern, so we must continue to
-    // accumulate expression-related errors.
-    productions |= ExpressionClassifier::ExpressionProductions |
-                   ExpressionClassifier::ObjectLiteralProduction;
+  if (is_destructuring_assignment) {
+    // This is definitely not an expression so don't accumulate
+    // expression-related errors.
+    productions &= ~(ExpressionClassifier::ExpressionProduction |
+                     ExpressionClassifier::ObjectLiteralProduction |
+                     ExpressionClassifier::TailCallExpressionProduction);
   }
 
   classifier->Accumulate(&arrow_formals_classifier, productions, false);
@@ -2450,11 +2458,7 @@ ParserBase<Impl>::ParseAssignmentExpression(bool accept_IN,
       ParseAssignmentExpression(accept_IN, &rhs_classifier, CHECK_OK);
   CheckNoTailCallExpressions(&rhs_classifier, CHECK_OK);
   impl()->RewriteNonPattern(&rhs_classifier, CHECK_OK);
-  classifier->Accumulate(
-      &rhs_classifier,
-      ExpressionClassifier::ExpressionProductions |
-          ExpressionClassifier::ObjectLiteralProduction |
-          ExpressionClassifier::AsyncArrowFormalParametersProduction);
+  classifier->AccumulateFormalParameterContainmentErrors(&rhs_classifier);
 
   // TODO(1231235): We try to estimate the set of properties set by
   // constructors. We define a new property whenever there is an
@@ -2889,8 +2893,8 @@ ParserBase<Impl>::ParseLeftHandSideExpression(ExpressionClassifier* classifier,
             // async () => ...
             return factory()->NewEmptyParentheses(pos);
           } else {
-            classifier->Accumulate(&async_classifier,
-                                   ExpressionClassifier::AllProductions);
+            classifier->AccumulateFormalParameterContainmentErrors(
+                &async_classifier);
           }
         } else {
           args = ParseArguments(&spread_pos, false, classifier, CHECK_OK);

src/parsing/parser.cc

@@ -4677,8 +4677,8 @@ Expression* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
     CheckNoTailCallExpressions(&extends_classifier, CHECK_OK);
     RewriteNonPattern(&extends_classifier, CHECK_OK);
     if (classifier != nullptr) {
-      classifier->Accumulate(&extends_classifier,
-                             ExpressionClassifier::ExpressionProductions);
+      classifier->AccumulateFormalParameterContainmentErrors(
+          &extends_classifier);
     }
   } else {
     block_state.set_start_position(scanner()->location().end_pos);
@@ -4706,8 +4706,8 @@ Expression* Parser::ParseClassLiteral(ExpressionClassifier* classifier,
         &has_seen_constructor, &property_classifier, &property_name, CHECK_OK);
     RewriteNonPattern(&property_classifier, CHECK_OK);
     if (classifier != nullptr) {
-      classifier->Accumulate(&property_classifier,
-                             ExpressionClassifier::ExpressionProductions);
+      classifier->AccumulateFormalParameterContainmentErrors(
+          &property_classifier);
     }
 
     if (has_seen_constructor && constructor == nullptr) {

src/parsing/preparser.cc

@@ -1168,8 +1168,8 @@ PreParserExpression PreParser::ParseClassLiteral(
     CheckNoTailCallExpressions(&extends_classifier, CHECK_OK);
     ValidateExpression(&extends_classifier, CHECK_OK);
     if (classifier != nullptr) {
-      classifier->Accumulate(&extends_classifier,
-                             ExpressionClassifier::ExpressionProductions);
+      classifier->AccumulateFormalParameterContainmentErrors(
+          &extends_classifier);
     }
   }
 
@@ -1189,8 +1189,8 @@ PreParserExpression PreParser::ParseClassLiteral(
         &has_seen_constructor, &property_classifier, &name, CHECK_OK);
     ValidateExpression(&property_classifier, CHECK_OK);
     if (classifier != nullptr) {
-      classifier->Accumulate(&property_classifier,
-                             ExpressionClassifier::ExpressionProductions);
+      classifier->AccumulateFormalParameterContainmentErrors(
+          &property_classifier);
     }
   }
 

test/cctest/test-parsing.cc

@@ -7817,8 +7817,6 @@ TEST(AsyncAwaitErrors) {
     "var f = async(x = await 1) => x;",
     "var O = { async method(x = await 1) { return x; } };",
 
-    "var f = async(x = await) => 1;",
-
     "function* g() { var f = async yield => 1; }",
     "function* g() { var f = async(yield) => 1; }",
     "function* g() { var f = async(x = yield) => 1; }",
@@ -7895,6 +7893,7 @@ TEST(AsyncAwaitErrors) {
     "var f = async(await) => 1;",
     "var f = async(await = 1) => 1;",
     "var f = async(...[await]) => 1;",
+    "var f = async(x = await) => 1;",
 
     // v8:5190
     "var f = async(1) => 1",
@@ -7903,6 +7902,12 @@ TEST(AsyncAwaitErrors) {
     "var f = async({ foo = async(1) => 1 }) => 1",
     "var f = async({ foo = async(a) => 1 })",
 
+    "var f = async(x = async(await)) => 1;",
+    "var f = async(x = { [await]: 1 }) => 1;",
+    "var f = async(x = class extends (await) { }) => 1;",
+    "var f = async(x = class { static [await]() {} }) => 1;",
+    "var f = async({ x = await }) => 1;",
+
     NULL
   };
   // clang-format on