 Chromium Code Reviews
 Chromium Code Reviews Issue 1066933005:
  Use ExpressionClassifier for bindings.  (Closed) 
  Base URL: https://chromium.googlesource.com/v8/v8.git@master
    
  
    Issue 1066933005:
  Use ExpressionClassifier for bindings.  (Closed) 
  Base URL: https://chromium.googlesource.com/v8/v8.git@master| Index: src/preparser.h | 
| diff --git a/src/preparser.h b/src/preparser.h | 
| index 36320513adb872f5ef4f51d450551af9ab1499d8..2b77625b0c6f221323a463179d0c1d9cef15351f 100644 | 
| --- a/src/preparser.h | 
| +++ b/src/preparser.h | 
| @@ -581,66 +581,123 @@ class ParserBase : public Traits { | 
| void ReportUnexpectedToken(Token::Value token); | 
| void ReportUnexpectedTokenAt(Scanner::Location location, Token::Value token); | 
| - // 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); | 
| - // 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); | 
| - | 
| - | 
| class ExpressionClassifier { | 
| public: | 
| ExpressionClassifier() | 
| : expression_error_(Scanner::Location::invalid()), | 
| + expression_error_message_(nullptr), | 
| binding_pattern_error_(Scanner::Location::invalid()), | 
| - assignment_pattern_error_(Scanner::Location::invalid()) {} | 
| + binding_pattern_error_message_(nullptr), | 
| + assignment_pattern_error_(Scanner::Location::invalid()), | 
| + assignment_pattern_error_message_(nullptr) {} | 
| bool is_valid_expression() const { | 
| return expression_error_ == Scanner::Location::invalid(); | 
| 
arv (Not doing code reviews)
2015/04/23 16:52:52
return expression_error_.IsValid();
 
Dmitry Lomov (no reviews)
2015/04/23 17:06:50
Done.
 | 
| } | 
| + Scanner::Location expression_error_loc() const { return expression_error_; } | 
| + const char* expression_error_message() const { | 
| + return expression_error_message_; | 
| + } | 
| + | 
| bool is_valid_binding_pattern() const { | 
| return binding_pattern_error_ == Scanner::Location::invalid(); | 
| } | 
| + Scanner::Location binding_pattern_error_loc() const { | 
| + return binding_pattern_error_; | 
| + } | 
| + const char* binding_pattern_error_message() const { | 
| + return binding_pattern_error_message_; | 
| + } | 
| - bool is_valid_assignmnent_pattern() const { | 
| + bool is_valid_assignment_pattern() const { | 
| return assignment_pattern_error_ == Scanner::Location::invalid(); | 
| } | 
| + Scanner::Location assignment_pattern_error_loc() const { | 
| + return assignment_pattern_error_; | 
| + } | 
| + const char* assignment_pattern_error_message() const { | 
| + return assignment_pattern_error_message_; | 
| + } | 
| - void RecordExpressionError(const Scanner::Location& loc) { | 
| + void RecordExpressionError(const Scanner::Location& loc, | 
| + const char* message) { | 
| if (!is_valid_expression()) return; | 
| expression_error_ = loc; | 
| + expression_error_message_ = message; | 
| } | 
| - void RecordBindingPatternError(const Scanner::Location& loc) { | 
| + void RecordBindingPatternError(const Scanner::Location& loc, | 
| + const char* message) { | 
| if (!is_valid_binding_pattern()) return; | 
| binding_pattern_error_ = loc; | 
| + binding_pattern_error_message_ = message; | 
| } | 
| - void RecordAssignmentPatternError(const Scanner::Location& loc) { | 
| - if (!is_valid_assignmnent_pattern()) return; | 
| + void RecordAssignmentPatternError(const Scanner::Location& loc, | 
| + const char* message) { | 
| + if (!is_valid_assignment_pattern()) return; | 
| assignment_pattern_error_ = loc; | 
| + assignment_pattern_error_message_ = message; | 
| } | 
| private: | 
| Scanner::Location expression_error_; | 
| + const char* expression_error_message_; | 
| Scanner::Location binding_pattern_error_; | 
| + const char* binding_pattern_error_message_; | 
| Scanner::Location assignment_pattern_error_; | 
| + const char* assignment_pattern_error_message_; | 
| }; | 
| + void ValidateExpression(const ExpressionClassifier* classifier, bool* ok) { | 
| + if (!classifier->is_valid_expression()) { | 
| + ReportMessageAt(classifier->expression_error_loc(), | 
| + classifier->expression_error_message()); | 
| + *ok = false; | 
| + } | 
| + } | 
| + | 
| + void ValidateBindingPattern(const ExpressionClassifier* classifier, | 
| + bool* ok) { | 
| + if (!classifier->is_valid_binding_pattern()) { | 
| + Traits::ReportMessageAt(classifier->binding_pattern_error_loc(), | 
| + classifier->binding_pattern_error_message()); | 
| + *ok = false; | 
| + } | 
| + } | 
| + | 
| + | 
| + void ValidateAssignmentPattern(const ExpressionClassifier* classifier, | 
| + bool* ok) { | 
| + if (!classifier->is_valid_assignment_pattern()) { | 
| + Traits::ReportMessageAt(classifier->assignment_pattern_error_loc(), | 
| + classifier->assignment_pattern_error_message()); | 
| + *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 +2017,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 +2071,7 @@ typename ParserBase<Traits>::IdentifierT ParserBase<Traits>::ParseIdentifier( | 
| } | 
| } | 
| + | 
| template <class Traits> | 
| typename ParserBase<Traits>::IdentifierT ParserBase< | 
| Traits>::ParseIdentifierOrStrictReservedWord(bool* is_strict_reserved, | 
| @@ -2141,7 +2221,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; | 
| @@ -2241,7 +2321,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; | 
| } |