Use ExpressionClassifier for bindings.

Just a refactoring, real pattern parsing comes in a later CL.

R=rossberg@chromium.org,marja@chromium.org
BUG=v8:811
LOG=N

Review URL: https://codereview.chromium.org/1066933005

Cr-Commit-Position: refs/heads/master@{#28084}

diff --git a/src/parser.cc b/src/parser.cc
index 87d347d..fe9ed10 100644 (file)
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -1146,7 +1146,7 @@ FunctionLiteral* Parser::ParseLazy(Isolate* isolate, ParseInfo* info,
         ExpressionClassifier classifier;
         Expression* expression = ParseArrowFunctionLiteral(
             scope, error_locs, has_rest, &classifier, &ok);
-        // TODO(dslomov): report error if not a valid expression.
+        ValidateExpression(&classifier, &ok);
         if (ok) {
           // Scanning must end at the same position that was recorded
           // previously. If not, parsing has been interrupted due to a stack
@@ -1616,7 +1616,7 @@ Statement* Parser::ParseExportDefault(bool* ok) {
       int pos = peek_position();
       ExpressionClassifier classifier;
       Expression* expr = ParseAssignmentExpression(true, &classifier, CHECK_OK);
-      // TODO(dslomov): report error if not a valid expression.
+      ValidateExpression(&classifier, CHECK_OK);
 
       ExpectSemicolon(CHECK_OK);
       result = factory()->NewExpressionStatement(expr, pos);
@@ -2376,7 +2376,24 @@ Block* Parser::ParseVariableDeclarations(
 
     // Parse variable name.
     if (nvars > 0) Consume(Token::COMMA);
-    name = ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK);
+
+    {
+      ExpressionClassifier pattern_classifier;
+      Token::Value next = peek();
+      Expression* pattern =
+          ParsePrimaryExpression(&pattern_classifier, CHECK_OK);
+      ValidateBindingPattern(&pattern_classifier, CHECK_OK);
+      if (pattern->IsVariableProxy() &&
+          pattern->AsVariableProxy()->IsValidReferenceExpression()) {
+        scope_->RemoveUnresolved(pattern->AsVariableProxy());
+        name = pattern->AsVariableProxy()->raw_name();
+      } else {
+        ReportUnexpectedToken(next);
+        *ok = false;
+        return nullptr;
+      }
+    }
+
     if (!first_name) first_name = name;
     Scanner::Location variable_loc = scanner()->location();
     if (fni_ != NULL) fni_->PushVariableName(name);
@@ -2455,7 +2472,7 @@ Block* Parser::ParseVariableDeclarations(
       ExpressionClassifier classifier;
       value = ParseAssignmentExpression(var_context != kForStatement,
                                         &classifier, CHECK_OK);
-      // TODO(dslomov): check that expression is valid.
+      ValidateExpression(&classifier, CHECK_OK);
       variable_loc.end_pos = scanner()->location().end_pos;
 
       if (first_initializer_loc && !first_initializer_loc->IsValid()) {
@@ -2646,7 +2663,7 @@ Statement* Parser::ParseExpressionOrLabelledStatement(
         } else {
           expr = ParseStrongSuperCallExpression(&classifier, CHECK_OK);
         }
-        // TODO(dslomov): report error if not a valid expression.
+        ValidateExpression(&classifier, CHECK_OK);
         switch (peek()) {
           case Token::SEMICOLON:
             Consume(Token::SEMICOLON);
@@ -4375,7 +4392,7 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
     block_scope->set_start_position(scanner()->location().end_pos);
     ExpressionClassifier classifier;
     extends = ParseLeftHandSideExpression(&classifier, CHECK_OK);
-    // TODO(dslomov): report error if not a valid expression.
+    ValidateExpression(&classifier, CHECK_OK);
   } else {
     block_scope->set_start_position(scanner()->location().end_pos);
   }
@@ -4400,7 +4417,7 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
     ObjectLiteral::Property* property = ParsePropertyDefinition(
         &checker, in_class, has_extends, is_static, &is_computed_name,
         &has_seen_constructor, &classifier, CHECK_OK);
-    // TODO(dslomov): report error if not a valid expression.
+    ValidateExpression(&classifier, CHECK_OK);
 
     if (has_seen_constructor && constructor == NULL) {
       constructor = GetPropertyValue(property)->AsFunctionLiteral();
@@ -4452,7 +4469,7 @@ Expression* Parser::ParseV8Intrinsic(bool* ok) {
   ExpressionClassifier classifier;
   ZoneList<Expression*>* args =
       ParseArguments(&spread_pos, &classifier, CHECK_OK);
-  // TODO(dslomov): report error if not a valid expression.
+  ValidateExpression(&classifier, CHECK_OK);
 
   DCHECK(!spread_pos.IsValid());
 

diff --git a/src/preparser.cc b/src/preparser.cc
index 950f7b5..d8c8426 100644 (file)
--- a/src/preparser.cc
+++ b/src/preparser.cc
@@ -513,7 +513,19 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
   do {
     // Parse variable name.
     if (nvars > 0) Consume(Token::COMMA);
-    ParseIdentifier(kDontAllowRestrictedIdentifiers, CHECK_OK);
+    {
+      ExpressionClassifier pattern_classifier;
+      Token::Value next = peek();
+      PreParserExpression pattern =
+          ParsePrimaryExpression(&pattern_classifier, CHECK_OK);
+      ValidateBindingPattern(&pattern_classifier, CHECK_OK);
+
+      if (!pattern.IsIdentifier()) {
+        ReportUnexpectedToken(next);
+        *ok = false;
+        return Statement::Default();
+      }
+    }
     Scanner::Location variable_loc = scanner()->location();
     nvars++;
     if (peek() == Token::ASSIGN || require_initializer ||
@@ -523,7 +535,7 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
       ExpressionClassifier classifier;
       ParseAssignmentExpression(var_context != kForStatement, &classifier,
                                 CHECK_OK);
-      // TODO(dslomov): report error if not valid expression.
+      ValidateExpression(&classifier, CHECK_OK);
 
       variable_loc.end_pos = scanner()->location().end_pos;
       if (first_initializer_loc && !first_initializer_loc->IsValid()) {
@@ -568,7 +580,7 @@ PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(bool* ok) {
         } else {
           expr = ParseStrongSuperCallExpression(&classifier, CHECK_OK);
         }
-        // TODO(dslomov): report error if not a valid expression.
+        ValidateExpression(&classifier, CHECK_OK);
         switch (peek()) {
           case Token::SEMICOLON:
             Consume(Token::SEMICOLON);
@@ -599,7 +611,7 @@ PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(bool* ok) {
   bool starts_with_identifier = peek_any_identifier();
   ExpressionClassifier classifier;
   Expression expr = ParseExpression(true, &classifier, CHECK_OK);
-  // TODO(dslomov): report error if not a valid expression.
+  ValidateExpression(&classifier, CHECK_OK);
 
   // Even if the expression starts with an identifier, it is not necessarily an
   // identifier. For example, "foo + bar" starts with an identifier but is not
@@ -1091,7 +1103,7 @@ PreParserExpression PreParser::ParseClassLiteral(
   if (has_extends) {
     ExpressionClassifier classifier;
     ParseLeftHandSideExpression(&classifier, CHECK_OK);
-    // TODO(dslomov): report error if not a valid expression.
+    ValidateExpression(&classifier, CHECK_OK);
   }
 
   ClassLiteralChecker checker(this);
@@ -1108,7 +1120,7 @@ PreParserExpression PreParser::ParseClassLiteral(
     ParsePropertyDefinition(&checker, in_class, has_extends, is_static,
                             &is_computed_name, &has_seen_constructor,
                             &classifier, CHECK_OK);
-    // TODO(dslomov): report error if not a valid expression.
+    ValidateExpression(&classifier, CHECK_OK);
   }
 
   Expect(Token::RBRACE, CHECK_OK);
@@ -1130,7 +1142,7 @@ PreParser::Expression PreParser::ParseV8Intrinsic(bool* ok) {
   Scanner::Location spread_pos;
   ExpressionClassifier classifier;
   ParseArguments(&spread_pos, &classifier, ok);
-  // TODO(dslomov): report error if not a valid expression.
+  ValidateExpression(&classifier, CHECK_OK);
 
   DCHECK(!spread_pos.IsValid());
 

diff --git a/src/preparser.h b/src/preparser.h
index c60318b..571fbf0 100644 (file)
--- a/src/preparser.h
+++ b/src/preparser.h
@@ -581,66 +581,125 @@ class ParserBase : public Traits {
   void ReportUnexpectedToken(Token::Value token);
   void ReportUnexpectedTokenAt(Scanner::Location location, Token::Value token);
 
-  // Recursive descent functions:
+  class ExpressionClassifier {
+   public:
+    struct Error {
+      Error()
+          : location(Scanner::Location::invalid()),
+            message(nullptr),
+            arg(nullptr) {}
 
-  // Parses an identifier that is valid for the current scope, in particular it
-  // fails on strict mode future reserved keywords in a strict scope. If
-  // allow_eval_or_arguments is kAllowEvalOrArguments, we allow "eval" or
-  // "arguments" as identifier even in strict mode (this is needed in cases like
-  // "var foo = eval;").
-  IdentifierT ParseIdentifier(AllowRestrictedIdentifiers, bool* ok);
-  // Parses an identifier or a strict mode future reserved word, and indicate
-  // whether it is strict mode future reserved.
-  IdentifierT ParseIdentifierOrStrictReservedWord(
-      bool* is_strict_reserved,
-      bool* ok);
-  IdentifierT ParseIdentifierName(bool* ok);
-  // Parses an identifier and determines whether or not it is 'get' or 'set'.
-  IdentifierT ParseIdentifierNameOrGetOrSet(bool* is_get,
-                                            bool* is_set,
-                                            bool* ok);
+      Scanner::Location location;
+      const char* message;
+      const char* arg;
 
+      bool HasError() const { return location.IsValid(); }
+    };
 
-  class ExpressionClassifier {
-   public:
-    ExpressionClassifier()
-        : expression_error_(Scanner::Location::invalid()),
-          binding_pattern_error_(Scanner::Location::invalid()),
-          assignment_pattern_error_(Scanner::Location::invalid()) {}
+    ExpressionClassifier() {}
 
-    bool is_valid_expression() const {
-      return expression_error_ == Scanner::Location::invalid();
-    }
+    bool is_valid_expression() const { return !expression_error_.HasError(); }
 
     bool is_valid_binding_pattern() const {
-      return binding_pattern_error_ == Scanner::Location::invalid();
+      return !binding_pattern_error_.HasError();
     }
 
-    bool is_valid_assignmnent_pattern() const {
-      return assignment_pattern_error_ == Scanner::Location::invalid();
+    bool is_valid_assignment_pattern() const {
+      return !assignment_pattern_error_.HasError();
     }
 
-    void RecordExpressionError(const Scanner::Location& loc) {
+    const Error& expression_error() const { return expression_error_; }
+
+    const Error& binding_pattern_error() const {
+      return binding_pattern_error_;
+    }
+
+    const Error& assignment_pattern_error() const {
+      return assignment_pattern_error_;
+    }
+
+    void RecordExpressionError(const Scanner::Location& loc,
+                               const char* message, const char* arg = nullptr) {
       if (!is_valid_expression()) return;
-      expression_error_ = loc;
+      expression_error_.location = loc;
+      expression_error_.message = message;
+      expression_error_.arg = arg;
     }
 
-    void RecordBindingPatternError(const Scanner::Location& loc) {
+    void RecordBindingPatternError(const Scanner::Location& loc,
+                                   const char* message,
+                                   const char* arg = nullptr) {
       if (!is_valid_binding_pattern()) return;
-      binding_pattern_error_ = loc;
+      binding_pattern_error_.location = loc;
+      binding_pattern_error_.message = message;
+      binding_pattern_error_.arg = arg;
     }
 
-    void RecordAssignmentPatternError(const Scanner::Location& loc) {
-      if (!is_valid_assignmnent_pattern()) return;
-      assignment_pattern_error_ = loc;
+    void RecordAssignmentPatternError(const Scanner::Location& loc,
+                                      const char* message,
+                                      const char* arg = nullptr) {
+      if (!is_valid_assignment_pattern()) return;
+      assignment_pattern_error_.location = loc;
+      assignment_pattern_error_.message = message;
+      assignment_pattern_error_.arg = arg;
     }
 
    private:
-    Scanner::Location expression_error_;
-    Scanner::Location binding_pattern_error_;
-    Scanner::Location assignment_pattern_error_;
+    Error expression_error_;
+    Error binding_pattern_error_;
+    Error assignment_pattern_error_;
   };
 
+  void ReportClassifierError(
+      const typename ExpressionClassifier::Error& error) {
+    Traits::ReportMessageAt(error.location, error.message, error.arg,
+                            kSyntaxError);
+  }
+
+  void ValidateExpression(const ExpressionClassifier* classifier, bool* ok) {
+    if (!classifier->is_valid_expression()) {
+      ReportClassifierError(classifier->expression_error());
+      *ok = false;
+    }
+  }
+
+  void ValidateBindingPattern(const ExpressionClassifier* classifier,
+                              bool* ok) {
+    if (!classifier->is_valid_binding_pattern()) {
+      ReportClassifierError(classifier->binding_pattern_error());
+      *ok = false;
+    }
+  }
+
+  void ValidateAssignmentPattern(const ExpressionClassifier* classifier,
+                                 bool* ok) {
+    if (!classifier->is_valid_assignment_pattern()) {
+      ReportClassifierError(classifier->assignment_pattern_error());
+      *ok = false;
+    }
+  }
+
+
+  // Recursive descent functions:
+
+  // Parses an identifier that is valid for the current scope, in particular it
+  // fails on strict mode future reserved keywords in a strict scope. If
+  // allow_eval_or_arguments is kAllowEvalOrArguments, we allow "eval" or
+  // "arguments" as identifier even in strict mode (this is needed in cases like
+  // "var foo = eval;").
+  IdentifierT ParseIdentifier(AllowRestrictedIdentifiers, bool* ok);
+  IdentifierT ParseAndClassifyIdentifier(ExpressionClassifier* classifier,
+                                         bool* ok);
+  // Parses an identifier or a strict mode future reserved word, and indicate
+  // whether it is strict mode future reserved.
+  IdentifierT ParseIdentifierOrStrictReservedWord(bool* is_strict_reserved,
+                                                  bool* ok);
+  IdentifierT ParseIdentifierName(bool* ok);
+  // Parses an identifier and determines whether or not it is 'get' or 'set'.
+  IdentifierT ParseIdentifierNameOrGetOrSet(bool* is_get, bool* is_set,
+                                            bool* ok);
+
+
   ExpressionT ParseRegExpLiteral(bool seen_equal,
                                  ExpressionClassifier* classifier, bool* ok);
 
@@ -1960,23 +2019,45 @@ void ParserBase<Traits>::ReportUnexpectedTokenAt(
 template <class Traits>
 typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParseIdentifier(
     AllowRestrictedIdentifiers allow_restricted_identifiers, bool* ok) {
+  ExpressionClassifier classifier;
+  auto result = ParseAndClassifyIdentifier(&classifier, ok);
+  if (!*ok) return Traits::EmptyIdentifier();
+
+  if (allow_restricted_identifiers == kDontAllowRestrictedIdentifiers) {
+    ValidateAssignmentPattern(&classifier, ok);
+    if (!*ok) return Traits::EmptyIdentifier();
+    ValidateBindingPattern(&classifier, ok);
+    if (!*ok) return Traits::EmptyIdentifier();
+  } else {
+    ValidateExpression(&classifier, ok);
+    if (!*ok) return Traits::EmptyIdentifier();
+  }
+
+  return result;
+}
+
+
+template <class Traits>
+typename ParserBase<Traits>::IdentifierT
+ParserBase<Traits>::ParseAndClassifyIdentifier(ExpressionClassifier* classifier,
+                                               bool* ok) {
   Token::Value next = Next();
   if (next == Token::IDENTIFIER) {
     IdentifierT name = this->GetSymbol(scanner());
-    if (allow_restricted_identifiers == kDontAllowRestrictedIdentifiers) {
-      if (is_strict(language_mode()) && this->IsEvalOrArguments(name)) {
-        ReportMessage("strict_eval_arguments");
-        *ok = false;
-      }
-      if (is_strong(language_mode()) && this->IsUndefined(name)) {
-        ReportMessage("strong_undefined");
-        *ok = false;
-      }
-    } else {
-      if (is_strong(language_mode()) && this->IsArguments(name)) {
-        ReportMessage("strong_arguments");
-        *ok = false;
-      }
+    if (is_strict(language_mode()) && this->IsEvalOrArguments(name)) {
+      classifier->RecordBindingPatternError(scanner()->location(),
+                                            "strict_eval_arguments");
+    }
+    if (is_strong(language_mode()) && this->IsUndefined(name)) {
+      // TODO(dslomov): allow 'undefined' in nested patterns.
+      classifier->RecordBindingPatternError(scanner()->location(),
+                                            "strong_undefined");
+      classifier->RecordAssignmentPatternError(scanner()->location(),
+                                               "strong_undefined");
+    }
+    if (is_strong(language_mode()) && this->IsArguments(name)) {
+      classifier->RecordExpressionError(scanner()->location(),
+                                        "strong_arguments");
     }
     if (this->IsArguments(name)) scope_->RecordArgumentsUsage();
     return name;
@@ -1992,6 +2073,7 @@ typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParseIdentifier(
   }
 }
 
+
 template <class Traits>
 typename ParserBase<Traits>::IdentifierT ParserBase<
     Traits>::ParseIdentifierOrStrictReservedWord(bool* is_strict_reserved,
@@ -2141,7 +2223,7 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
     case Token::YIELD:
     case Token::FUTURE_STRICT_RESERVED_WORD: {
       // Using eval or arguments in this context is OK even in strict mode.
-      IdentifierT name = ParseIdentifier(kAllowRestrictedIdentifiers, CHECK_OK);
+      IdentifierT name = ParseAndClassifyIdentifier(classifier, CHECK_OK);
       result = this->ExpressionFromIdentifier(name, beg_pos, end_pos, scope_,
                                               factory());
       break;
@@ -2171,6 +2253,10 @@ ParserBase<Traits>::ParsePrimaryExpression(ExpressionClassifier* classifier,
 
     case Token::LPAREN:
       Consume(Token::LPAREN);
+      classifier->RecordBindingPatternError(scanner()->location(),
+                                            "unexpected_token", "(");
+      classifier->RecordAssignmentPatternError(scanner()->location(),
+                                               "unexpected_token", "(");
       if (allow_harmony_arrow_functions() && Check(Token::RPAREN)) {
         // As a primary expression, the only thing that can follow "()" is "=>".
         Scope* scope = this->NewScope(scope_, ARROW_SCOPE);
@@ -2241,7 +2327,7 @@ typename ParserBase<Traits>::ExpressionT ParserBase<Traits>::ParseExpression(
     bool accept_IN, bool* ok) {
   ExpressionClassifier classifier;
   ExpressionT result = ParseExpression(accept_IN, &classifier, CHECK_OK);
-  // TODO(dslomov): report error if not a valid expression.
+  ValidateExpression(&classifier, CHECK_OK);
   return result;
 }
 

diff --git a/src/scanner.h b/src/scanner.h
index 4d7411f..5162fbe 100644 (file)
--- a/src/scanner.h
+++ b/src/scanner.h
@@ -355,14 +355,6 @@ class Scanner {
 
     int beg_pos;
     int end_pos;
-
-    bool inline operator==(const Location& other) const {
-      return beg_pos == other.beg_pos && end_pos == other.end_pos;
-    }
-
-    bool inline operator!=(const Location& other) const {
-      return !(*this == other);
-    }
   };
 
   // -1 is outside of the range of any real source code.