[es6] strict eval/arguments and strong undefined in AssignmentPattern

src/parsing/parser-base.h

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_PARSING_PARSER_BASE_H
 #define V8_PARSING_PARSER_BASE_H
 
 #include "src/ast/scopes.h"
 #include "src/bailout-reason.h"
 #include "src/hashmap.h"
@@ skipping 794 matching lines @@
   // Checks if the expression is a valid reference expression (e.g., on the
   // left-hand side of assignments). Although ruled out by ECMA as early errors,
   // we allow calls for web compatibility and rewrite them to a runtime throw.
   ExpressionT CheckAndRewriteReferenceExpression(
       ExpressionT expression, int beg_pos, int end_pos,
       MessageTemplate::Template message, bool* ok);
   ExpressionT ClassifyAndRewriteReferenceExpression(
       ExpressionClassifier* classifier, ExpressionT expression, int beg_pos,
       int end_pos, MessageTemplate::Template message,
       ParseErrorType type = kSyntaxError);
+  void ClassifyReferenceExpression(ExpressionClassifier* classifier,
+                                   ExpressionT expression, int beg_pos,
+                                   int end_pos);
   ExpressionT CheckAndRewriteReferenceExpression(
       ExpressionT expression, int beg_pos, int end_pos,
       MessageTemplate::Template message, ParseErrorType type, bool* ok);
 
   bool IsValidReferenceExpression(ExpressionT expression);
 
   bool IsAssignableIdentifier(ExpressionT expression) {
     if (!Traits::IsIdentifier(expression)) return false;
     if (is_strict(language_mode()) &&
         Traits::IsEvalOrArguments(Traits::AsIdentifier(expression))) {
@@ skipping 685 matching lines @@
                         MessageTemplate::kStrongEllision);
         *ok = false;
         return this->EmptyExpression();
       }
       elem = this->GetLiteralTheHole(peek_position(), factory());
     } else if (peek() == Token::ELLIPSIS) {
       int start_pos = peek_position();
       Consume(Token::ELLIPSIS);
       ExpressionT argument =
           this->ParseAssignmentExpression(true, classifier, CHECK_OK);
+      ClassifyReferenceExpression(classifier, argument, start_pos,
+                                  scanner()->location().end_pos);
       elem = factory()->NewSpread(argument, start_pos);
 
       if (first_spread_index < 0) {
         first_spread_index = values->length();
       }
 
       CheckDestructuringElement(argument, classifier, start_pos,

[adamk, 2015/12/12 01:03:35]

I think CheckDestructuringElement is doing the work here already (testing
locally, "[...eval] = []" already gives me an error without your patch). It
calls IsAssignableIdentifier(), which does the proper checks
eval/arguments/undefined checks.

Maybe you just need to make sure to call CheckDestructuringElement in more
places?

[caitp (gmail), 2015/12/12 01:08:14]

Yes, there is some redundant stuff here --- as mentioned, this change matches
the messages used in other situations where assignment to illegal variables is
performed, but it can be cheapened a bit if we don't care which message is used

[adamk, 2015/12/12 01:10:44]

Would much prefer less duplication to message clarity (at least as a first cut).

                                 scanner()->location().end_pos);
 
       if (peek() == Token::COMMA) {
         classifier->RecordPatternError(
             Scanner::Location(start_pos, scanner()->location().end_pos),
             MessageTemplate::kElementAfterRest);
       }
     } else {
       elem = this->ParseAssignmentExpression(true, kIsPatternElement,
                                              classifier, CHECK_OK);
+      ClassifyReferenceExpression(classifier, elem, pos,
+                                  scanner()->location().end_pos);
       if (!this->IsValidReferenceExpression(elem) &&
           !classifier->is_valid_assignment_pattern()) {
         classifier->RecordPatternError(
             Scanner::Location(pos, scanner()->location().end_pos),
             MessageTemplate::kInvalidDestructuringTarget);
       }
     }
     values->Add(elem, zone_);
     if (peek() != Token::RBRACK) {
       Expect(Token::COMMA, CHECK_OK);
@@ skipping 116 matching lines @@
       //    PropertyName ':' AssignmentExpression
       if (!*is_computed_name) {
         checker->CheckProperty(name_token, kValueProperty, false, false,
                                CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
       }
       Consume(Token::COLON);
       int pos = peek_position();
       value = this->ParseAssignmentExpression(
           true, kIsPatternElement, classifier,
           CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
-
+      ClassifyReferenceExpression(classifier, value, pos,
+                                  scanner()->location().end_pos);
       if (!this->IsValidReferenceExpression(value) &&
           !classifier->is_valid_assignment_pattern()) {
         classifier->RecordPatternError(
             Scanner::Location(pos, scanner()->location().end_pos),
             MessageTemplate::kInvalidDestructuringTarget);
       }
 
       return factory()->NewObjectLiteralProperty(name_expression, value, false,
                                                  *is_computed_name);
     }
@@ skipping 20 matching lines @@
         classifier->RecordDuplicateFormalParameterError(scanner()->location());
       }
       if (name_token == Token::LET) {
         classifier->RecordLetPatternError(
             scanner()->location(), MessageTemplate::kLetInLexicalBinding);
       }
 
       ExpressionT lhs = this->ExpressionFromIdentifier(
           name, next_beg_pos, next_end_pos, scope_, factory());
 
+      ClassifyReferenceExpression(classifier, lhs, next_beg_pos, next_end_pos);
+
       if (peek() == Token::ASSIGN) {
         Consume(Token::ASSIGN);
         ExpressionClassifier rhs_classifier;
         ExpressionT rhs = this->ParseAssignmentExpression(
             true, &rhs_classifier, CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
         classifier->Accumulate(rhs_classifier,
                                ExpressionClassifier::ExpressionProductions);
         value = factory()->NewAssignment(Token::ASSIGN, lhs, rhs,
                                          RelocInfo::kNoPosition);
         classifier->RecordCoverInitializedNameError(
@@ skipping 319 matching lines @@
           ExpressionClassifier::FormalParametersProductions |
           ExpressionClassifier::CoverInitializedNameProduction);
 
   bool maybe_pattern =
       expression->IsObjectLiteral() || expression->IsArrayLiteral();
   // bool binding_pattern =
   //    allow_harmony_destructuring_bind() && maybe_pattern && !is_rhs;
 
   if (!Token::IsAssignmentOp(peek())) {
     // Parsed conditional expression only (no assignment).
+    if (is_pattern_element && !maybe_pattern) {
+      ClassifyReferenceExpression(classifier, expression, lhs_beg_pos,
+                                  scanner()->location().end_pos);
+    }
     if (is_pattern_element && !this->IsValidReferenceExpression(expression) &&
         !maybe_pattern) {
       classifier->RecordPatternError(
           Scanner::Location(lhs_beg_pos, scanner()->location().end_pos),
           MessageTemplate::kInvalidDestructuringTarget);
     } else if (is_rhs && maybe_pattern) {
       ValidateExpression(classifier, CHECK_OK);
     }
 
     return expression;
@@ skipping 1168 matching lines @@
   ExpressionClassifier classifier;
   ExpressionT result = ClassifyAndRewriteReferenceExpression(
       &classifier, expression, beg_pos, end_pos, message, type);
   ValidateExpression(&classifier, ok);
   if (!*ok) return this->EmptyExpression();
   return result;
 }
 
 
 template <typename Traits>
+void ParserBase<Traits>::ClassifyReferenceExpression(

[adamk, 2015/12/12 01:03:35]

Indeed, this code is very close to the existing CheckDestructuringElement...

+    ExpressionClassifier* classifier, ExpressionT expression, int beg_pos,
+    int end_pos) {
+  if (!this->IsIdentifier(expression)) return;
+  IdentifierT ref = this->AsIdentifier(expression);
+  if (is_strict(language_mode()) && this->IsEvalOrArguments(ref)) {
+    classifier->RecordAssignmentPatternError(
+        Scanner::Location(beg_pos, end_pos),
+        MessageTemplate::kStrictEvalArguments);
+  } else if (is_strong(language_mode()) && this->IsUndefined(ref)) {
+    classifier->RecordAssignmentPatternError(
+        Scanner::Location(beg_pos, end_pos), MessageTemplate::kStrongUndefined);
+  }
+}
+
+
+template <typename Traits>
 typename ParserBase<Traits>::ExpressionT
 ParserBase<Traits>::ClassifyAndRewriteReferenceExpression(
     ExpressionClassifier* classifier, ExpressionT expression, int beg_pos,
     int end_pos, MessageTemplate::Template message, ParseErrorType type) {
   Scanner::Location location(beg_pos, end_pos);
   if (this->IsIdentifier(expression)) {
     if (is_strict(language_mode()) &&
         this->IsEvalOrArguments(this->AsIdentifier(expression))) {
       classifier->RecordExpressionError(
           location, MessageTemplate::kStrictEvalArguments, kSyntaxError);
+      classifier->RecordAssignmentPatternError(
+          location, MessageTemplate::kStrictEvalArguments);
       return expression;
     }
     if (is_strong(language_mode()) &&
         this->IsUndefined(this->AsIdentifier(expression))) {
       classifier->RecordExpressionError(
           location, MessageTemplate::kStrongUndefined, kSyntaxError);
+      classifier->RecordAssignmentPatternError(
+          location, MessageTemplate::kStrongUndefined);
       return expression;
     }
   }
   if (expression->IsValidReferenceExpression()) {
     return expression;
   } else if (expression->IsCall()) {
     // If it is a call, make it a runtime error for legacy web compatibility.
     // Rewrite `expr' to `expr[throw ReferenceError]'.
     int pos = location.beg_pos;
     ExpressionT error = this->NewThrowReferenceError(message, pos);
@@ skipping 82 matching lines @@
       return;
     }
     has_seen_constructor_ = true;
     return;
   }
 }
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_PARSING_PARSER_BASE_H