[parser] Validate patterns non-parenthesized in ValidatePattern

No-Tree-Checks: true
No-Try: true
Change-Id: I8b8651e499c2d862ae9f81a18af11d610214c0cd
Reviewed-on: https://chromium-review.googlesource.com/c/1350118
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57818}

src/ast/ast.h

@@ -250,7 +250,7 @@ class Expression : public AstNode {
 
   bool IsCompileTimeValue();
 
-  bool IsValidPattern() {
+  bool IsPattern() {
     STATIC_ASSERT(kObjectLiteral + 1 == kArrayLiteral);
     return IsInRange(node_type(), kObjectLiteral, kArrayLiteral);
   }

src/parsing/parser-base.h

@@ -901,6 +901,15 @@ class ParserBase {
     }
   }
 
+  void ValidatePattern(ExpressionT expression) {
+    if (expression->is_parenthesized()) {
+      impl()->ReportMessageAt(
+          Scanner::Location(expression->position(), end_position()),
+          MessageTemplate::kInvalidDestructuringTarget);
+    }
+    ValidatePattern();
+  }
+
   void ValidateFormalParameters(LanguageMode language_mode,
                                 const FormalParametersT& parameters,
                                 bool allow_duplicates) {
@@ -2635,13 +2644,8 @@ ParserBase<Impl>::ParseAssignmentExpression() {
   }
 
   // Destructuring assignmment.
-  if (V8_UNLIKELY(expression->IsValidPattern() && op == Token::ASSIGN)) {
-    if (expression->is_parenthesized()) {
-      impl()->ReportMessageAt(
-          Scanner::Location(expression->position(), end_position()),
-          MessageTemplate::kInvalidDestructuringTarget);
-    }
-    ValidatePattern();
+  if (V8_UNLIKELY(expression->IsPattern() && op == Token::ASSIGN)) {
+    ValidatePattern(expression);
 
     // This is definitely not an expression so don't accumulate
     // expression-related errors.
@@ -4381,7 +4385,7 @@ ParserBase<Impl>::RewriteInvalidReferenceExpression(ExpressionT expression,
 template <typename Impl>
 void ParserBase<Impl>::CheckArrowFormalParameter(ExpressionT formal) {
   if (formal->is_parenthesized() ||
-      !(impl()->IsIdentifier(formal) || formal->IsValidPattern() ||
+      !(impl()->IsIdentifier(formal) || formal->IsPattern() ||
         formal->IsAssignment())) {
     classifier()->RecordBindingPatternError(
         Scanner::Location(formal->position(), end_position()),
@@ -4413,7 +4417,7 @@ void ParserBase<Impl>::CheckDestructuringElement(ExpressionT expression,
     return;
   }
   if (expression->is_parenthesized() ||
-      (!expression->IsValidPattern() && !expression->IsAssignment())) {
+      (!expression->IsPattern() && !expression->IsAssignment())) {
     classifier()->RecordPatternError(
         Scanner::Location(begin, end),
         MessageTemplate::kInvalidDestructuringTarget);
@@ -5361,10 +5365,10 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForStatement(
     int lhs_end_pos = end_position();
 
     bool is_for_each = CheckInOrOf(&for_info.mode);
-    bool is_destructuring = is_for_each && expression->IsValidPattern();
+    bool is_destructuring = is_for_each && expression->IsPattern();
 
     if (is_destructuring) {
-      ValidatePattern();
+      ValidatePattern(expression);
     } else {
       ValidateExpression();
     }
@@ -5496,8 +5500,13 @@ ParserBase<Impl>::ParseForEachStatementWithoutDeclarations(
     ForInfo* for_info, ZonePtrList<const AstRawString>* labels,
     ZonePtrList<const AstRawString>* own_labels) {
   // Initializer is reference followed by in/of.
-  if (V8_UNLIKELY(!expression->IsValidPattern() &&
-                  !IsValidReferenceExpression(expression))) {
+  if (expression->IsPattern()) {
+    if (expression->is_parenthesized()) {
+      impl()->ReportMessageAt(
+          Scanner::Location(expression->position(), end_position()),
+          MessageTemplate::kInvalidDestructuringTarget);
+    }
+  } else if (V8_UNLIKELY(!IsValidReferenceExpression(expression))) {
     expression = RewriteInvalidReferenceExpression(
         expression, lhs_beg_pos, lhs_end_pos, MessageTemplate::kInvalidLhsInFor,
         kSyntaxError);
@@ -5698,8 +5707,8 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseForAwaitStatement(
     ExpressionT lhs = each_variable = ParseLeftHandSideExpression();
     int lhs_end_pos = end_position();
 
-    if (lhs->IsValidPattern()) {
-      ValidatePattern();
+    if (lhs->IsPattern()) {
+      ValidatePattern(lhs);
     } else {
       ValidateExpression();
       if (V8_UNLIKELY(!IsValidReferenceExpression(lhs))) {

src/parsing/parser.cc

@@ -1816,7 +1816,7 @@ Statement* Parser::InitializeForEachStatement(ForEachStatement* stmt,
     return InitializeForOfStatement(for_of, each, subject, body, finalize,
                                     IteratorType::kNormal, each->position());
   } else {
-    if (each->IsValidPattern()) {
+    if (each->IsPattern()) {
       Variable* temp = NewTemporary(ast_value_factory()->empty_string());
       VariableProxy* temp_proxy = factory()->NewVariableProxy(temp);
       Expression* assign_each =
@@ -2053,7 +2053,7 @@ Statement* Parser::InitializeForOfStatement(
   {
     assign_each =
         factory()->NewAssignment(Token::ASSIGN, each, result_value, nopos);
-    if (each->IsValidPattern()) {
+    if (each->IsPattern()) {
       assign_each = RewriteDestructuringAssignment(assign_each->AsAssignment());
     }
   }

src/parsing/preparser.h

@@ -239,7 +239,7 @@ class PreParserExpression {
     return TypeField::decode(code_) == kArrayLiteralExpression;
   }
 
-  bool IsValidPattern() const {
+  bool IsPattern() const {
     STATIC_ASSERT(kObjectLiteralExpression + 1 == kArrayLiteralExpression);
     return IsInRange(TypeField::decode(code_), kObjectLiteralExpression,
                      kArrayLiteralExpression);

test/mjsunit/es6/destructuring.js

@@ -1191,6 +1191,12 @@
   assertEquals(undefined, eval('try {throw [1, 2, 3]} catch([x]) {}'));
 })();
 
+// Property access as declaration target.
 assertThrows("let [o.x=1]=[]", SyntaxError);
 assertThrows("let {x:o.f=1}={x:1}", SyntaxError);
 assertThrows("(o.f=1)=>0", SyntaxError);
+
+// Invalidly parenthesized declaration targets.
+assertThrows("for (({x}) of [{x:1}]) {}", SyntaxError);
+assertThrows("for (var ({x}) of [{x:1}]) {}", SyntaxError);
+assertThrows("for await (({x}) of [{x:1}]) {}", SyntaxError);