[parser] Hide expression classifiers in parser implementation

This patch removes the explicit classifier parameters from all
parsing methods and makes expression classifiers implicit in
the (pre)parser's implementation.  In this way, the implementation
is simplified and a proper stack of classifiers is enforced.

R=adamk@chromium.org,littledan@chromium.org
BUG=
LOG=N

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

src/parsing/expression-classifier.h

@@ -7,7 +7,6 @@
 
 #include "src/messages.h"
 #include "src/parsing/scanner.h"
-#include "src/parsing/token.h"
 
 namespace v8 {
 namespace internal {
@@ -26,6 +25,28 @@ class DuplicateFinder;
   T(TailCallExpressionProduction, 8)         \
   T(AsyncArrowFormalParametersProduction, 9)
 
+// Expression classifiers serve two purposes:
+//
+// 1) They keep track of error messages that are pending (and other
+//    related information), waiting for the parser to decide whether
+//    the parsed expression is a pattern or not.
+// 2) They keep track of expressions that may need to be rewritten, if
+//    the parser decides that they are not patterns.  (A different
+//    mechanism implements the rewriting of patterns.)
+//
+// Expression classifiers are used by the parser in a stack fashion.
+// Each new classifier is pushed on top of the stack.  This happens
+// automatically by the class's constructor.  While on top of the
+// stack, the classifier records pending error messages and tracks the
+// pending non-patterns of the expression that is being parsed.
+//
+// At the end of its life, a classifier is either "accumulated" to the
+// one that is below it on the stack, or is "discarded".  The former
+// is achieved by calling the method Accumulate.  The latter is
+// achieved automatically by the destructor, but it can happen earlier
+// by calling the method Discard.  Both actions result in removing the
+// classifier from the parser's stack.
+
 template <typename Types>
 class ExpressionClassifier {
  public:
@@ -72,19 +93,25 @@ class ExpressionClassifier {
     NonSimpleParameter = 1 << 0
   };
 
-  explicit ExpressionClassifier(const typename Types::Base* base,
+  explicit ExpressionClassifier(typename Types::Base* base,
                                 DuplicateFinder* duplicate_finder = nullptr)
-      : zone_(base->impl()->zone()),
+      : base_(base),
+        previous_(base->classifier_),
+        zone_(base->impl()->zone()),
         non_patterns_to_rewrite_(base->impl()->GetNonPatternList()),
         reported_errors_(base->impl()->GetReportedErrorList()),
         duplicate_finder_(duplicate_finder),
         invalid_productions_(0),
         function_properties_(0) {
+    base->classifier_ = this;
     reported_errors_begin_ = reported_errors_end_ = reported_errors_->length();
     non_pattern_begin_ = non_patterns_to_rewrite_->length();
   }
 
-  ~ExpressionClassifier() { Discard(); }
+  V8_INLINE ~ExpressionClassifier() {
+    Discard();
+    if (base_->classifier_ == this) base_->classifier_ = previous_;
+  }
 
   V8_INLINE bool is_valid(unsigned productions) const {
     return (invalid_productions_ & productions) == 0;
@@ -357,20 +384,6 @@ 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() {
@@ -383,6 +396,8 @@ class ExpressionClassifier {
     non_patterns_to_rewrite_->Rewind(non_pattern_begin_);
   }
 
+  ExpressionClassifier* previous() const { return previous_; }
+
  private:
   V8_INLINE const Error& reported_error(ErrorKind kind) const {
     if (invalid_productions_ & (1 << kind)) {
@@ -419,6 +434,8 @@ class ExpressionClassifier {
     reported_errors_end_++;
   }
 
+  typename Types::Base* base_;
+  ExpressionClassifier* previous_;
   Zone* zone_;
   ZoneList<typename Types::Expression>* non_patterns_to_rewrite_;
   ZoneList<Error>* reported_errors_;
@@ -441,6 +458,8 @@ class ExpressionClassifier {
   // stack overflow while parsing.
   uint16_t reported_errors_begin_;
   uint16_t reported_errors_end_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExpressionClassifier);
 };
 
 

src/parsing/parser.h

@@ -146,9 +146,6 @@ struct ParserTypes<Parser> {
 
   typedef v8::internal::AstProperties AstProperties;
 
-  typedef v8::internal::ExpressionClassifier<ParserTypes<Parser>>
-      ExpressionClassifier;
-
   // Return types for traversing functions.
   typedef const AstRawString* Identifier;
   typedef v8::internal::Expression* Expression;
@@ -444,7 +441,6 @@ class Parser : public ParserBase<Parser> {
 
   void DesugarAsyncFunctionBody(const AstRawString* function_name, Scope* scope,
                                 ZoneList<Statement*>* body,
-                                ExpressionClassifier* classifier,
                                 FunctionKind kind, FunctionBodyType type,
                                 bool accept_IN, int pos, bool* ok);
 
@@ -456,8 +452,7 @@ class Parser : public ParserBase<Parser> {
       int function_token_position, FunctionLiteral::FunctionType type,
       LanguageMode language_mode, bool* ok);
 
-  Expression* ParseClassLiteral(ExpressionClassifier* classifier,
-                                const AstRawString* name,
+  Expression* ParseClassLiteral(const AstRawString* name,
                                 Scanner::Location class_name_location,
                                 bool name_is_strict_reserved, int pos,
                                 bool* ok);
@@ -579,12 +574,10 @@ class Parser : public ParserBase<Parser> {
 
   void ParseAsyncArrowSingleExpressionBody(ZoneList<Statement*>* body,
                                            bool accept_IN,
-                                           ExpressionClassifier* classifier,
                                            int pos, bool* ok) {
-    DesugarAsyncFunctionBody(ast_value_factory()->empty_string(), scope(), body,
-                             classifier, kAsyncArrowFunction,
-                             FunctionBodyType::kSingleExpression, accept_IN,
-                             pos, ok);
+    DesugarAsyncFunctionBody(
+        ast_value_factory()->empty_string(), scope(), body, kAsyncArrowFunction,
+        FunctionBodyType::kSingleExpression, accept_IN, pos, ok);
   }
 
   ZoneList<Expression*>* PrepareSpreadArguments(ZoneList<Expression*>* list);
@@ -611,7 +604,7 @@ class Parser : public ParserBase<Parser> {
   V8_INLINE Expression* RewriteSpreads(ArrayLiteral* lit);
 
   // Rewrite expressions that are not used as patterns
-  V8_INLINE void RewriteNonPattern(ExpressionClassifier* classifier, bool* ok);
+  V8_INLINE void RewriteNonPattern(bool* ok);
 
   V8_INLINE void QueueDestructuringAssignmentForRewriting(
       Expression* assignment);
@@ -953,10 +946,9 @@ class Parser : public ParserBase<Parser> {
 
   V8_INLINE void DeclareFormalParameter(
       DeclarationScope* scope,
-      const ParserFormalParameters::Parameter& parameter,
-      ExpressionClassifier* classifier) {
+      const ParserFormalParameters::Parameter& parameter) {
     bool is_duplicate = false;
-    bool is_simple = classifier->is_simple_parameter_list();
+    bool is_simple = classifier()->is_simple_parameter_list();
     auto name = is_simple || parameter.is_rest
                     ? parameter.name
                     : ast_value_factory()->empty_string();
@@ -967,7 +959,7 @@ class Parser : public ParserBase<Parser> {
         scope->DeclareParameter(name, mode, is_optional, parameter.is_rest,
                                 &is_duplicate, ast_value_factory());
     if (is_duplicate) {
-      classifier->RecordDuplicateFormalParameterError(scanner()->location());
+      classifier()->RecordDuplicateFormalParameterError(scanner()->location());
     }
     if (is_sloppy(scope->language_mode())) {
       // TODO(sigurds) Mark every parameter as maybe assigned. This is a

src/parsing/preparser.cc

@@ -401,8 +401,9 @@ PreParser::Statement PreParser::ParseClassDeclaration(bool* ok) {
   bool is_strict_reserved = false;
   Identifier name =
       ParseIdentifierOrStrictReservedWord(&is_strict_reserved, CHECK_OK);
-  ParseClassLiteral(nullptr, name, scanner()->location(), is_strict_reserved,
-                    pos, CHECK_OK);
+  ExpressionClassifier no_classifier(this);
+  ParseClassLiteral(name, scanner()->location(), is_strict_reserved, pos,
+                    CHECK_OK);
   return Statement::Default();
 }
 
@@ -501,12 +502,10 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
     PreParserExpression pattern = PreParserExpression::Default();
     {
       ExpressionClassifier pattern_classifier(this);
-      pattern = ParsePrimaryExpression(&pattern_classifier, CHECK_OK);
+      pattern = ParsePrimaryExpression(CHECK_OK);
 
-      ValidateBindingPattern(&pattern_classifier, CHECK_OK);
-      if (lexical) {
-        ValidateLetPattern(&pattern_classifier, CHECK_OK);
-      }
+      ValidateBindingPattern(CHECK_OK);
+      if (lexical) ValidateLetPattern(CHECK_OK);
     }
 
     is_pattern = pattern.IsObjectLiteral() || pattern.IsArrayLiteral();
@@ -515,9 +514,8 @@ PreParser::Statement PreParser::ParseVariableDeclarations(
     nvars++;
     if (Check(Token::ASSIGN)) {
       ExpressionClassifier classifier(this);
-      ParseAssignmentExpression(var_context != kForStatement, &classifier,
-                                CHECK_OK);
-      ValidateExpression(&classifier, CHECK_OK);
+      ParseAssignmentExpression(var_context != kForStatement, CHECK_OK);
+      ValidateExpression(CHECK_OK);
 
       variable_loc.end_pos = scanner()->location().end_pos;
       if (first_initializer_loc && !first_initializer_loc->IsValid()) {
@@ -582,8 +580,8 @@ PreParser::Statement PreParser::ParseExpressionOrLabelledStatement(
 
   bool starts_with_identifier = peek_any_identifier();
   ExpressionClassifier classifier(this);
-  Expression expr = ParseExpression(true, &classifier, CHECK_OK);
-  ValidateExpression(&classifier, CHECK_OK);
+  Expression expr = ParseExpressionCoverGrammar(true, CHECK_OK);
+  ValidateExpression(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
@@ -843,8 +841,8 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
 
         if (mode == ForEachStatement::ITERATE) {
           ExpressionClassifier classifier(this);
-          ParseAssignmentExpression(true, &classifier, CHECK_OK);
-          RewriteNonPattern(&classifier, CHECK_OK);
+          ParseAssignmentExpression(true, CHECK_OK);
+          RewriteNonPattern(CHECK_OK);
         } else {
           ParseExpression(true, CHECK_OK);
         }
@@ -860,16 +858,16 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
     } else {
       int lhs_beg_pos = peek_position();
       ExpressionClassifier classifier(this);
-      Expression lhs = ParseExpression(false, &classifier, CHECK_OK);
+      Expression lhs = ParseExpressionCoverGrammar(false, CHECK_OK);
       int lhs_end_pos = scanner()->location().end_pos;
       bool is_for_each = CheckInOrOf(&mode, CHECK_OK);
       bool is_destructuring = is_for_each &&
                               (lhs->IsArrayLiteral() || lhs->IsObjectLiteral());
 
       if (is_destructuring) {
-        ValidateAssignmentPattern(&classifier, CHECK_OK);
+        ValidateAssignmentPattern(CHECK_OK);
       } else {
-        ValidateExpression(&classifier, CHECK_OK);
+        ValidateExpression(CHECK_OK);
       }
 
       if (is_for_each) {
@@ -881,8 +879,8 @@ PreParser::Statement PreParser::ParseForStatement(bool* ok) {
 
         if (mode == ForEachStatement::ITERATE) {
           ExpressionClassifier classifier(this);
-          ParseAssignmentExpression(true, &classifier, CHECK_OK);
-          RewriteNonPattern(&classifier, CHECK_OK);
+          ParseAssignmentExpression(true, CHECK_OK);
+          RewriteNonPattern(CHECK_OK);
         } else {
           ParseExpression(true, CHECK_OK);
         }
@@ -974,8 +972,8 @@ PreParser::Statement PreParser::ParseTryStatement(bool* ok) {
     Expect(Token::LPAREN, CHECK_OK);
     Scope* catch_scope = NewScope(CATCH_SCOPE);
     ExpressionClassifier pattern_classifier(this);
-    ParsePrimaryExpression(&pattern_classifier, CHECK_OK);
-    ValidateBindingPattern(&pattern_classifier, CHECK_OK);
+    ParsePrimaryExpression(CHECK_OK);
+    ValidateBindingPattern(CHECK_OK);
     Expect(Token::RPAREN, CHECK_OK);
     {
       CollectExpressionsInTailPositionToListScope
@@ -1050,7 +1048,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
   int start_position = scanner()->location().beg_pos;
   function_scope->set_start_position(start_position);
   PreParserFormalParameters formals(function_scope);
-  ParseFormalParameterList(&formals, &formals_classifier, CHECK_OK);
+  ParseFormalParameterList(&formals, CHECK_OK);
   Expect(Token::RPAREN, CHECK_OK);
   int formals_end_position = scanner()->location().end_pos;
 
@@ -1079,8 +1077,7 @@ PreParser::Expression PreParser::ParseFunctionLiteral(
                     function_name_location, CHECK_OK);
   const bool allow_duplicate_parameters =
       is_sloppy(language_mode) && formals.is_simple && !IsConciseMethod(kind);
-  ValidateFormalParameters(&formals_classifier, language_mode,
-                           allow_duplicate_parameters, CHECK_OK);
+  ValidateFormalParameters(language_mode, allow_duplicate_parameters, CHECK_OK);
 
   if (is_strict(language_mode)) {
     int end_position = scanner()->location().end_pos;
@@ -1138,9 +1135,8 @@ void PreParser::ParseLazyFunctionLiteralBody(bool* ok,
 }
 
 PreParserExpression PreParser::ParseClassLiteral(
-    ExpressionClassifier* classifier, PreParserIdentifier name,
-    Scanner::Location class_name_location, bool name_is_strict_reserved,
-    int pos, bool* ok) {
+    PreParserIdentifier name, Scanner::Location class_name_location,
+    bool name_is_strict_reserved, int pos, bool* ok) {
   // All parts of a ClassDeclaration and ClassExpression are strict code.
   if (name_is_strict_reserved) {
     ReportMessageAt(class_name_location,
@@ -1164,13 +1160,10 @@ PreParserExpression PreParser::ParseClassLiteral(
   bool has_extends = Check(Token::EXTENDS);
   if (has_extends) {
     ExpressionClassifier extends_classifier(this);
-    ParseLeftHandSideExpression(&extends_classifier, CHECK_OK);
-    CheckNoTailCallExpressions(&extends_classifier, CHECK_OK);
-    ValidateExpression(&extends_classifier, CHECK_OK);
-    if (classifier != nullptr) {
-      classifier->AccumulateFormalParameterContainmentErrors(
-          &extends_classifier);
-    }
+    ParseLeftHandSideExpression(CHECK_OK);
+    CheckNoTailCallExpressions(CHECK_OK);
+    ValidateExpression(CHECK_OK);
+    impl()->AccumulateFormalParameterContainmentErrors();
   }
 
   ClassLiteralChecker checker(this);
@@ -1185,13 +1178,9 @@ PreParserExpression PreParser::ParseClassLiteral(
     Identifier name;
     ExpressionClassifier property_classifier(this);
     ParsePropertyDefinition(&checker, in_class, has_extends, &is_computed_name,
-                            &has_seen_constructor, &property_classifier, &name,
-                            CHECK_OK);
-    ValidateExpression(&property_classifier, CHECK_OK);
-    if (classifier != nullptr) {
-      classifier->AccumulateFormalParameterContainmentErrors(
-          &property_classifier);
-    }
+                            &has_seen_constructor, &name, CHECK_OK);
+    ValidateExpression(CHECK_OK);
+    impl()->AccumulateFormalParameterContainmentErrors();
   }
 
   Expect(Token::RBRACE, CHECK_OK);
@@ -1212,8 +1201,8 @@ PreParser::Expression PreParser::ParseV8Intrinsic(bool* ok) {
   ParseIdentifier(kAllowRestrictedIdentifiers, CHECK_OK);
   Scanner::Location spread_pos;
   ExpressionClassifier classifier(this);
-  ParseArguments(&spread_pos, &classifier, ok);
-  ValidateExpression(&classifier, CHECK_OK);
+  ParseArguments(&spread_pos, ok);
+  ValidateExpression(CHECK_OK);
 
   DCHECK(!spread_pos.IsValid());
 
@@ -1233,13 +1222,13 @@ PreParserExpression PreParser::ParseDoExpression(bool* ok) {
   return PreParserExpression::Default();
 }
 
-void PreParser::ParseAsyncArrowSingleExpressionBody(
-    PreParserStatementList body, bool accept_IN,
-    ExpressionClassifier* classifier, int pos, bool* ok) {
+void PreParser::ParseAsyncArrowSingleExpressionBody(PreParserStatementList body,
+                                                    bool accept_IN, int pos,
+                                                    bool* ok) {
   scope()->ForceContextAllocation();
 
   PreParserExpression return_value =
-      ParseAssignmentExpression(accept_IN, classifier, CHECK_OK_CUSTOM(Void));
+      ParseAssignmentExpression(accept_IN, CHECK_OK_CUSTOM(Void));
 
   body->Add(PreParserStatement::ExpressionStatement(return_value), zone());
 }

src/parsing/preparser.h

@@ -6,14 +6,7 @@
 #define V8_PARSING_PREPARSER_H
 
 #include "src/ast/scopes.h"
-#include "src/bailout-reason.h"
-#include "src/base/hashmap.h"
-#include "src/messages.h"
-#include "src/parsing/expression-classifier.h"
-#include "src/parsing/func-name-inferrer.h"
 #include "src/parsing/parser-base.h"
-#include "src/parsing/scanner.h"
-#include "src/parsing/token.h"
 
 namespace v8 {
 namespace internal {
@@ -607,9 +600,6 @@ struct ParserTypes<PreParser> {
 
   typedef int AstProperties;
 
-  typedef v8::internal::ExpressionClassifier<ParserTypes<PreParser>>
-      ExpressionClassifier;
-
   // Return types for traversing functions.
   typedef PreParserIdentifier Identifier;
   typedef PreParserExpression Expression;
@@ -787,8 +777,7 @@ class PreParser : public ParserBase<PreParser> {
   void ParseLazyFunctionLiteralBody(bool* ok,
                                     Scanner::BookmarkScope* bookmark = nullptr);
 
-  PreParserExpression ParseClassLiteral(ExpressionClassifier* classifier,
-                                        PreParserIdentifier name,
+  PreParserExpression ParseClassLiteral(PreParserIdentifier name,
                                         Scanner::Location class_name_location,
                                         bool name_is_strict_reserved, int pos,
                                         bool* ok);
@@ -810,7 +799,6 @@ class PreParser : public ParserBase<PreParser> {
 
   void ParseAsyncArrowSingleExpressionBody(PreParserStatementList body,
                                            bool accept_IN,
-                                           ExpressionClassifier* classifier,
                                            int pos, bool* ok);
 
   V8_INLINE PreParserExpressionList
@@ -846,9 +834,7 @@ class PreParser : public ParserBase<PreParser> {
                                                  int pos) {
     return PreParserExpression::Default();
   }
-  V8_INLINE void RewriteNonPattern(ExpressionClassifier* classifier, bool* ok) {
-    ValidateExpression(classifier, ok);
-  }
+  V8_INLINE void RewriteNonPattern(bool* ok) { ValidateExpression(ok); }
 
   V8_INLINE void QueueDestructuringAssignmentForRewriting(
       PreParserExpression assignment) {}
@@ -1106,9 +1092,8 @@ class PreParser : public ParserBase<PreParser> {
   }
 
   V8_INLINE void DeclareFormalParameter(DeclarationScope* scope,
-                                        PreParserIdentifier parameter,
-                                        ExpressionClassifier* classifier) {
-    if (!classifier->is_simple_parameter_list()) {
+                                        PreParserIdentifier parameter) {
+    if (!classifier()->is_simple_parameter_list()) {
       scope->SetHasNonSimpleParameters();
     }
   }