Reduce the memory footprint of expression classifiers

src/parsing/expression-classifier.h

 // Copyright 2015 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_EXPRESSION_CLASSIFIER_H
 #define V8_PARSING_EXPRESSION_CLASSIFIER_H
 
 #include "src/messages.h"
 #include "src/parsing/scanner.h"
 #include "src/parsing/token.h"
 
 namespace v8 {
 namespace internal {
 
 
+#define ERROR_CODES(T)                          \
+  T(ExpressionProduction, 0)                    \
+  T(FormalParameterInitializerProduction, 1)    \
+  T(BindingPatternProduction, 2)                \
+  T(AssignmentPatternProduction, 3)             \
+  T(DistinctFormalParametersProduction, 4)      \
+  T(StrictModeFormalParametersProduction, 5)    \
+  T(ArrowFormalParametersProduction, 6)         \
+  T(LetPatternProduction, 7)                    \
+  T(CoverInitializedNameProduction, 8)          \
+  T(TailCallExpressionProduction, 9)            \
+  T(AsyncArrowFormalParametersProduction, 10)   \
+  T(AsyncBindingPatternProduction, 11)
+
+
 template <typename Traits>
 class ExpressionClassifier {
  public:
+  enum ErrorKind : unsigned {
+#define DEFINE_ERROR_KIND(NAME, CODE) k##NAME = CODE,
+    ERROR_CODES(DEFINE_ERROR_KIND)
+#undef DEFINE_ERROR_KIND
+    kUnusedError = 15  // Larger than error codes; should fit in 4 bits
+  };
+
   struct Error {
-    Error()
+    V8_INLINE Error()
         : location(Scanner::Location::invalid()),
           message(MessageTemplate::kNone),
+          kind(kUnusedError),
           type(kSyntaxError),
           arg(nullptr) {}
+    V8_INLINE explicit Error(Scanner::Location loc,
+                             MessageTemplate::Template msg, ErrorKind k,
+                             const char* a = nullptr,
+                             ParseErrorType t = kSyntaxError)
+        : location(loc), message(msg), kind(k), type(t), arg(a) {}
 
     Scanner::Location location;
-    MessageTemplate::Template message : 30;
+    MessageTemplate::Template message : 26;
+    unsigned kind : 4;
     ParseErrorType type : 2;
     const char* arg;
   };
 
-  enum TargetProduction {
-    ExpressionProduction = 1 << 0,
-    FormalParameterInitializerProduction = 1 << 1,
-    BindingPatternProduction = 1 << 2,
-    AssignmentPatternProduction = 1 << 3,
-    DistinctFormalParametersProduction = 1 << 4,
-    StrictModeFormalParametersProduction = 1 << 5,
-    ArrowFormalParametersProduction = 1 << 6,
-    LetPatternProduction = 1 << 7,
-    CoverInitializedNameProduction = 1 << 8,
-    TailCallExpressionProduction = 1 << 9,
-    AsyncArrowFormalParametersProduction = 1 << 10,
-    AsyncBindingPatternProduction = 1 << 11,
+  enum TargetProduction : unsigned {
+#define DEFINE_PRODUCTION(NAME, CODE) NAME = 1 << CODE,
+    ERROR_CODES(DEFINE_PRODUCTION)
+#undef DEFINE_PRODUCTION
 
     ExpressionProductions =
         (ExpressionProduction | FormalParameterInitializerProduction |
          TailCallExpressionProduction),
     PatternProductions =
         (BindingPatternProduction | AssignmentPatternProduction |
          LetPatternProduction | AsyncBindingPatternProduction),
     FormalParametersProductions = (DistinctFormalParametersProduction |
                                    StrictModeFormalParametersProduction),
     StandardProductions = ExpressionProductions | PatternProductions,
     AllProductions =
         (StandardProductions | FormalParametersProductions |
          ArrowFormalParametersProduction | CoverInitializedNameProduction |
          AsyncArrowFormalParametersProduction | AsyncBindingPatternProduction)
   };
 
-  enum FunctionProperties { NonSimpleParameter = 1 << 0 };
+  enum FunctionProperties : unsigned {
+    NonSimpleParameter = 1 << 0
+  };
 
   explicit ExpressionClassifier(const Traits* t)
       : zone_(t->zone()),
         non_patterns_to_rewrite_(t->GetNonPatternList()),
         invalid_productions_(0),
         function_properties_(0),
+        reported_errors_(t->GetReportedErrorList()),
         duplicate_finder_(nullptr) {
+    reported_errors_begin_ = reported_errors_end_ = reported_errors_->length();
     non_pattern_begin_ = non_patterns_to_rewrite_->length();
   }
 
   ExpressionClassifier(const Traits* t, DuplicateFinder* duplicate_finder)
       : zone_(t->zone()),
         non_patterns_to_rewrite_(t->GetNonPatternList()),
         invalid_productions_(0),
         function_properties_(0),
+        reported_errors_(t->GetReportedErrorList()),
         duplicate_finder_(duplicate_finder) {
+    reported_errors_begin_ = reported_errors_end_ = reported_errors_->length();
     non_pattern_begin_ = non_patterns_to_rewrite_->length();
   }
 
   ~ExpressionClassifier() { Discard(); }
 
-  bool is_valid(unsigned productions) const {
+  V8_INLINE bool is_valid(unsigned productions) const {
     return (invalid_productions_ & productions) == 0;
   }
 
-  DuplicateFinder* duplicate_finder() const { return duplicate_finder_; }
+  V8_INLINE DuplicateFinder* duplicate_finder() const {
+    return duplicate_finder_;
+  }
 
-  bool is_valid_expression() const { return is_valid(ExpressionProduction); }
+  V8_INLINE bool is_valid_expression() const {
+    return is_valid(ExpressionProduction);
+  }
 
-  bool is_valid_formal_parameter_initializer() const {
+  V8_INLINE bool is_valid_formal_parameter_initializer() const {
     return is_valid(FormalParameterInitializerProduction);
   }
 
-  bool is_valid_binding_pattern() const {
+  V8_INLINE bool is_valid_binding_pattern() const {
     return is_valid(BindingPatternProduction);
   }
 
-  bool is_valid_assignment_pattern() const {
+  V8_INLINE bool is_valid_assignment_pattern() const {
     return is_valid(AssignmentPatternProduction);
   }
 
-  bool is_valid_arrow_formal_parameters() const {
+  V8_INLINE bool is_valid_arrow_formal_parameters() const {
     return is_valid(ArrowFormalParametersProduction);
   }
 
-  bool is_valid_formal_parameter_list_without_duplicates() const {
+  V8_INLINE bool is_valid_formal_parameter_list_without_duplicates() const {
     return is_valid(DistinctFormalParametersProduction);
   }
 
   // Note: callers should also check
   // is_valid_formal_parameter_list_without_duplicates().
-  bool is_valid_strict_mode_formal_parameters() const {
+  V8_INLINE bool is_valid_strict_mode_formal_parameters() const {
     return is_valid(StrictModeFormalParametersProduction);
   }
 
-  bool is_valid_let_pattern() const { return is_valid(LetPatternProduction); }
+  V8_INLINE bool is_valid_let_pattern() const {
+    return is_valid(LetPatternProduction);
+  }
 
   bool is_valid_async_arrow_formal_parameters() const {
     return is_valid(AsyncArrowFormalParametersProduction);
   }
 
   bool is_valid_async_binding_pattern() const {
     return is_valid(AsyncBindingPatternProduction);
   }
 
-  const Error& expression_error() const { return expression_error_; }
-
-  const Error& formal_parameter_initializer_error() const {
-    return formal_parameter_initializer_error_;
+  V8_INLINE const Error& expression_error() const {
+    return reported_error(kExpressionProduction);
   }
 
-  const Error& binding_pattern_error() const { return binding_pattern_error_; }
-
-  const Error& assignment_pattern_error() const {
-    return assignment_pattern_error_;
+  V8_INLINE const Error& formal_parameter_initializer_error() const {
+    return reported_error(kFormalParameterInitializerProduction);
   }
 
-  const Error& arrow_formal_parameters_error() const {
-    return arrow_formal_parameters_error_;
+  V8_INLINE const Error& binding_pattern_error() const {
+    return reported_error(kBindingPatternProduction);
   }
 
-  const Error& duplicate_formal_parameter_error() const {
-    return duplicate_formal_parameter_error_;
+  V8_INLINE const Error& assignment_pattern_error() const {
+    return reported_error(kAssignmentPatternProduction);
   }
 
-  const Error& strict_mode_formal_parameter_error() const {
-    return strict_mode_formal_parameter_error_;
+  V8_INLINE const Error& arrow_formal_parameters_error() const {
+    return reported_error(kArrowFormalParametersProduction);
   }
 
-  const Error& let_pattern_error() const { return let_pattern_error_; }
+  V8_INLINE const Error& duplicate_formal_parameter_error() const {
+    return reported_error(kDistinctFormalParametersProduction);
+  }
 
-  bool has_cover_initialized_name() const {
+  V8_INLINE const Error& strict_mode_formal_parameter_error() const {
+    return reported_error(kStrictModeFormalParametersProduction);
+  }
+
+  V8_INLINE const Error& let_pattern_error() const {
+    return reported_error(kLetPatternProduction);
+  }
+
+  V8_INLINE bool has_cover_initialized_name() const {
     return !is_valid(CoverInitializedNameProduction);
   }
-  const Error& cover_initialized_name_error() const {
-    return cover_initialized_name_error_;
+
+  V8_INLINE const Error& cover_initialized_name_error() const {
+    return reported_error(kCoverInitializedNameProduction);
   }
 
-  bool has_tail_call_expression() const {
+  V8_INLINE bool has_tail_call_expression() const {
     return !is_valid(TailCallExpressionProduction);
   }
-  const Error& tail_call_expression_error() const {
-    return tail_call_expression_error_;
+  V8_INLINE const Error& tail_call_expression_error() const {
+    return reported_error(kTailCallExpressionProduction);
   }
-  const Error& async_arrow_formal_parameters_error() const {
-    return async_arrow_formal_parameters_error_;
+  V8_INLINE const Error& async_arrow_formal_parameters_error() const {
+    return reported_error(kAsyncArrowFormalParametersProduction);
   }
 
-  const Error& async_binding_pattern_error() const {
-    return async_binding_pattern_error_;
+  V8_INLINE const Error& async_binding_pattern_error() const {
+    return reported_error(kAsyncBindingPatternProduction);
   }
 
-  bool is_simple_parameter_list() const {
+  V8_INLINE bool is_simple_parameter_list() const {
     return !(function_properties_ & NonSimpleParameter);
   }
 
-  void RecordNonSimpleParameter() {
+  V8_INLINE void RecordNonSimpleParameter() {
     function_properties_ |= NonSimpleParameter;
   }
 
   void RecordExpressionError(const Scanner::Location& loc,
                              MessageTemplate::Template message,
                              const char* arg = nullptr) {
     if (!is_valid_expression()) return;
     invalid_productions_ |= ExpressionProduction;
-    expression_error_.location = loc;
-    expression_error_.message = message;
-    expression_error_.arg = arg;
+    Add(Error(loc, message, kExpressionProduction, arg));
   }
 
   void RecordExpressionError(const Scanner::Location& loc,
                              MessageTemplate::Template message,
                              ParseErrorType type, const char* arg = nullptr) {
     if (!is_valid_expression()) return;
     invalid_productions_ |= ExpressionProduction;
-    expression_error_.location = loc;
-    expression_error_.message = message;
-    expression_error_.arg = arg;
-    expression_error_.type = type;
+    Add(Error(loc, message, kExpressionProduction, arg, type));
   }
 
   void RecordFormalParameterInitializerError(const Scanner::Location& loc,
                                              MessageTemplate::Template message,
                                              const char* arg = nullptr) {
     if (!is_valid_formal_parameter_initializer()) return;
     invalid_productions_ |= FormalParameterInitializerProduction;
-    formal_parameter_initializer_error_.location = loc;
-    formal_parameter_initializer_error_.message = message;
-    formal_parameter_initializer_error_.arg = arg;
+    Add(Error(loc, message, kFormalParameterInitializerProduction, arg));
+  }
+
+  static Error BindingPatternError(const Scanner::Location& loc,
+                                   MessageTemplate::Template message,
+                                   const char* arg = nullptr) {
+    return Error(loc, message, kBindingPatternProduction, arg);
+  }
+
+  void RecordBindingPatternError(const Error& e) {
+    if (!is_valid_binding_pattern()) return;
+    invalid_productions_ |= BindingPatternProduction;
+    Add(e);
   }
 
   void RecordBindingPatternError(const Scanner::Location& loc,
                                  MessageTemplate::Template message,
                                  const char* arg = nullptr) {
-    if (!is_valid_binding_pattern()) return;
-    invalid_productions_ |= BindingPatternProduction;
-    binding_pattern_error_.location = loc;
-    binding_pattern_error_.message = message;
-    binding_pattern_error_.arg = arg;
+    RecordBindingPatternError(BindingPatternError(loc, message, arg));
+  }
+
+  static Error AssignmentPatternError(const Scanner::Location& loc,
+                                      MessageTemplate::Template message,
+                                      const char* arg = nullptr) {
+    return Error(loc, message, kAssignmentPatternProduction, arg);
+  }
+
+  void RecordAssignmentPatternError(const Error& e) {
+    if (!is_valid_assignment_pattern()) return;
+    invalid_productions_ |= AssignmentPatternProduction;
+    Add(e);
   }
 
   void RecordAssignmentPatternError(const Scanner::Location& loc,
                                     MessageTemplate::Template message,
                                     const char* arg = nullptr) {
-    if (!is_valid_assignment_pattern()) return;
-    invalid_productions_ |= AssignmentPatternProduction;
-    assignment_pattern_error_.location = loc;
-    assignment_pattern_error_.message = message;
-    assignment_pattern_error_.arg = arg;
+    RecordAssignmentPatternError(AssignmentPatternError(loc, message, arg));
   }
 
   void RecordPatternError(const Scanner::Location& loc,
                           MessageTemplate::Template message,
                           const char* arg = nullptr) {
     RecordBindingPatternError(loc, message, arg);
     RecordAssignmentPatternError(loc, message, arg);
   }
 
   void RecordArrowFormalParametersError(const Scanner::Location& loc,
                                         MessageTemplate::Template message,
                                         const char* arg = nullptr) {
     if (!is_valid_arrow_formal_parameters()) return;
     invalid_productions_ |= ArrowFormalParametersProduction;
-    arrow_formal_parameters_error_.location = loc;
-    arrow_formal_parameters_error_.message = message;
-    arrow_formal_parameters_error_.arg = arg;
+    Add(Error(loc, message, kArrowFormalParametersProduction, arg));
   }
 
   void RecordAsyncArrowFormalParametersError(const Scanner::Location& loc,
                                              MessageTemplate::Template message,
                                              const char* arg = nullptr) {
     if (!is_valid_async_arrow_formal_parameters()) return;
     invalid_productions_ |= AsyncArrowFormalParametersProduction;
-    async_arrow_formal_parameters_error_.location = loc;
-    async_arrow_formal_parameters_error_.message = message;
-    async_arrow_formal_parameters_error_.arg = arg;
+    Add(Error(loc, message, kAsyncArrowFormalParametersProduction, arg));
   }
 
   void RecordAsyncBindingPatternError(const Scanner::Location& loc,
                                       MessageTemplate::Template message,
                                       const char* arg = nullptr) {
     if (!is_valid_async_binding_pattern()) return;
     invalid_productions_ |= AsyncBindingPatternProduction;
-    async_binding_pattern_error_.location = loc;
-    async_binding_pattern_error_.message = message;
-    async_binding_pattern_error_.arg = arg;
+    Add(Error(loc, message, kAsyncBindingPatternProduction, arg));
   }
 
   void RecordDuplicateFormalParameterError(const Scanner::Location& loc) {
     if (!is_valid_formal_parameter_list_without_duplicates()) return;
     invalid_productions_ |= DistinctFormalParametersProduction;
-    duplicate_formal_parameter_error_.location = loc;
-    duplicate_formal_parameter_error_.message = MessageTemplate::kParamDupe;
-    duplicate_formal_parameter_error_.arg = nullptr;
+    Add(Error(loc, MessageTemplate::kParamDupe,
+              kDistinctFormalParametersProduction));
   }
 
   // Record a binding that would be invalid in strict mode.  Confusingly this
   // is not the same as StrictFormalParameterList, which simply forbids
   // duplicate bindings.
   void RecordStrictModeFormalParameterError(const Scanner::Location& loc,
                                             MessageTemplate::Template message,
                                             const char* arg = nullptr) {
     if (!is_valid_strict_mode_formal_parameters()) return;
     invalid_productions_ |= StrictModeFormalParametersProduction;
-    strict_mode_formal_parameter_error_.location = loc;
-    strict_mode_formal_parameter_error_.message = message;
-    strict_mode_formal_parameter_error_.arg = arg;
+    Add(Error(loc, message, kStrictModeFormalParametersProduction, arg));
   }
 
   void RecordLetPatternError(const Scanner::Location& loc,
                              MessageTemplate::Template message,
                              const char* arg = nullptr) {
     if (!is_valid_let_pattern()) return;
     invalid_productions_ |= LetPatternProduction;
-    let_pattern_error_.location = loc;
-    let_pattern_error_.message = message;
-    let_pattern_error_.arg = arg;
+    Add(Error(loc, message, kLetPatternProduction, arg));
   }
 
   void RecordCoverInitializedNameError(const Scanner::Location& loc,
                                        MessageTemplate::Template message,
                                        const char* arg = nullptr) {
     if (has_cover_initialized_name()) return;
     invalid_productions_ |= CoverInitializedNameProduction;
-    cover_initialized_name_error_.location = loc;
-    cover_initialized_name_error_.message = message;
-    cover_initialized_name_error_.arg = arg;
+    Add(Error(loc, message, kCoverInitializedNameProduction, arg));
   }
 
   void RecordTailCallExpressionError(const Scanner::Location& loc,
                                      MessageTemplate::Template message,
                                      const char* arg = nullptr) {
     if (has_tail_call_expression()) return;
     invalid_productions_ |= TailCallExpressionProduction;
-    tail_call_expression_error_.location = loc;
-    tail_call_expression_error_.message = message;
-    tail_call_expression_error_.arg = arg;
+    Add(Error(loc, message, kTailCallExpressionProduction, arg));
   }
 
   void ForgiveCoverInitializedNameError() {
+    if (!(invalid_productions_ & CoverInitializedNameProduction)) return;
     invalid_productions_ &= ~CoverInitializedNameProduction;

[nickie, 2016/06/08 11:38:46]

This was a bug, and fixed.  Line 356 was moved after line 358. Otherwise, line
357 would not find the error and its 'kind' would not be set to kUnusedError. 

-    cover_initialized_name_error_ = Error();
+    Error& e = reported_error(kCoverInitializedNameProduction);
+    e.kind = kUnusedError;
   }
 
   void ForgiveAssignmentPatternError() {
+    if (!(invalid_productions_ & AssignmentPatternProduction)) return;
     invalid_productions_ &= ~AssignmentPatternProduction;

[nickie, 2016/06/08 11:38:45]

Same thing.

-    assignment_pattern_error_ = Error();
+    Error& e = reported_error(kAssignmentPatternProduction);
+    e.kind = kUnusedError;
   }
 
   void Accumulate(ExpressionClassifier* inner,
                   unsigned productions = StandardProductions,
                   bool merge_non_patterns = true) {
+    DCHECK_EQ(inner->reported_errors_, reported_errors_);
+    DCHECK_EQ(inner->reported_errors_begin_, reported_errors_end_);
+    DCHECK_EQ(inner->reported_errors_end_, reported_errors_->length());
     if (merge_non_patterns) MergeNonPatterns(inner);
     // Propagate errors from inner, but don't overwrite already recorded
     // errors.
     unsigned non_arrow_inner_invalid_productions =
         inner->invalid_productions_ & ~ArrowFormalParametersProduction;
-    if (non_arrow_inner_invalid_productions == 0) return;
-    unsigned non_arrow_productions =
-        productions & ~ArrowFormalParametersProduction;
-    unsigned errors =
-        non_arrow_productions & non_arrow_inner_invalid_productions;
-    errors &= ~invalid_productions_;
-    if (errors != 0) {
-      invalid_productions_ |= errors;
-      if (errors & ExpressionProduction)
-        expression_error_ = inner->expression_error_;
-      if (errors & FormalParameterInitializerProduction)
-        formal_parameter_initializer_error_ =
-            inner->formal_parameter_initializer_error_;
-      if (errors & BindingPatternProduction)
-        binding_pattern_error_ = inner->binding_pattern_error_;
-      if (errors & AssignmentPatternProduction)
-        assignment_pattern_error_ = inner->assignment_pattern_error_;
-      if (errors & DistinctFormalParametersProduction)
-        duplicate_formal_parameter_error_ =
-            inner->duplicate_formal_parameter_error_;
-      if (errors & StrictModeFormalParametersProduction)
-        strict_mode_formal_parameter_error_ =
-            inner->strict_mode_formal_parameter_error_;
-      if (errors & LetPatternProduction)
-        let_pattern_error_ = inner->let_pattern_error_;
-      if (errors & CoverInitializedNameProduction)
-        cover_initialized_name_error_ = inner->cover_initialized_name_error_;
-      if (errors & TailCallExpressionProduction)
-        tail_call_expression_error_ = inner->tail_call_expression_error_;
-      if (errors & AsyncArrowFormalParametersProduction)
-        async_arrow_formal_parameters_error_ =
-            inner->async_arrow_formal_parameters_error_;
-      if (errors & AsyncBindingPatternProduction)
-        async_binding_pattern_error_ = inner->async_binding_pattern_error_;
-    }
+    if (non_arrow_inner_invalid_productions) {
+      unsigned errors = non_arrow_inner_invalid_productions & productions &
+                        ~invalid_productions_;
+      // As an exception to the above, the result continues to be a valid arrow

[adamk, 2016/06/06 20:51:02]

This comment is now out-of-order (it refers to "above").

[nickie, 2016/06/08 11:38:45]

I removed the beginning of the sentence.

+      // formal parameters if the inner expression is a valid binding pattern.
+      if (productions & ArrowFormalParametersProduction &&
+          is_valid_arrow_formal_parameters()) {
+        // Also copy function properties if expecting an arrow function
+        // parameter.
+        function_properties_ |= inner->function_properties_;
 
-    // As an exception to the above, the result continues to be a valid arrow
-    // formal parameters if the inner expression is a valid binding pattern.
-    if (productions & ArrowFormalParametersProduction &&
-        is_valid_arrow_formal_parameters()) {
-      // Also copy function properties if expecting an arrow function
-      // parameter.
-      function_properties_ |= inner->function_properties_;
+        if (!inner->is_valid_binding_pattern())
+          errors |= ArrowFormalParametersProduction;
+      }
 
-      if (!inner->is_valid_binding_pattern()) {
-        invalid_productions_ |= ArrowFormalParametersProduction;
-        arrow_formal_parameters_error_ = inner->binding_pattern_error_;
+      if (errors != 0) {
+        invalid_productions_ |= errors;
+        int arrow_index = inner->reported_errors_end_;
+        for (int i = inner->reported_errors_begin_;
+             i < inner->reported_errors_end_; i++) {
+          if (reported_errors_->at(i).kind == kUnusedError ||
+              reported_errors_->at(i).kind == kArrowFormalParametersProduction)
+            continue;
+          if (errors & (1 << reported_errors_->at(i).kind))
+            Move(i);
+          if (reported_errors_->at(i).kind == kBindingPatternProduction &&
+              errors & ArrowFormalParametersProduction) {
+            if (reported_errors_end_ <= i) {
+              Move(i);
+              reported_errors_->at(reported_errors_end_-1).kind =
+                  kArrowFormalParametersProduction;
+            } else {
+              DCHECK_EQ(reported_errors_end_, i+1);
+              arrow_index = i;
+            }
+          }
+        }
+        if (arrow_index < inner->reported_errors_end_) {
+          if (reported_errors_end_ < inner->reported_errors_end_) {
+            Replace(reported_errors_end_, arrow_index);
+            reported_errors_->at(reported_errors_end_++).kind =
+                kArrowFormalParametersProduction;
+          } else {
+            Add(reported_errors_->at(arrow_index));
+            reported_errors_->at(reported_errors_end_-1).kind =
+                kArrowFormalParametersProduction;
+          }
+        }
       }
     }
+    reported_errors_->Rewind(reported_errors_end_);
+    inner->reported_errors_begin_ = inner->reported_errors_end_ =
+        reported_errors_end_;
   }
 
   V8_INLINE int GetNonPatternBegin() const { return non_pattern_begin_; }
 
   V8_INLINE void Discard() {
+    if (reported_errors_end_ == reported_errors_->length()) {
+      reported_errors_->Rewind(reported_errors_begin_);
+      reported_errors_end_ = reported_errors_begin_;
+    }
+    DCHECK_EQ(reported_errors_begin_, reported_errors_end_);
     DCHECK_LE(non_pattern_begin_, non_patterns_to_rewrite_->length());
     non_patterns_to_rewrite_->Rewind(non_pattern_begin_);
   }
 
   V8_INLINE void MergeNonPatterns(ExpressionClassifier* inner) {
     DCHECK_LE(non_pattern_begin_, inner->non_pattern_begin_);
     inner->non_pattern_begin_ = inner->non_patterns_to_rewrite_->length();
   }
 
  private:
+  V8_INLINE Error& reported_error(ErrorKind kind) const {
+    if (invalid_productions_ & (1 << kind)) {
+      for (int i = reported_errors_begin_; i < reported_errors_end_; i++) {
+        if (reported_errors_->at(i).kind == kind)
+          return reported_errors_->at(i);
+      }

[nickie, 2016/06/08 11:38:46]

I'm introducing an UNREACHABLE() here, partly to address the next comment.  This
should be OK; the errors in a classifier's reported list should always be
consistent with the current value of invalid_productions_.

+    }
+    // We should only be looking for an error when we know that one has
+    // been reported.  But we're not...  So this is to make sure we have

[adamk, 2016/06/06 20:51:02]

This sounds odd, where does this show up?

[nickie, 2016/06/08 11:38:46]

If you replace these lines with UNREACHABLE(), you get a crash when building the
snapshot.  Autopsy shows that in line:

https://cs.chromium.org/chromium/src/v8/src/parsing/parser.cc?rcl=0&l=4120

we're taking the location of a duplicate_formal_parameter_error when none has
been recorded.  They may be other cases like this.

+    // the same behaviour.
+    static Error none;
+    return none;
+  }
+
+  V8_INLINE void Add(const Error& e) {

[adamk, 2016/06/06 20:51:02]

Can you give each of Add, Move, and Replace a comment? Or maybe just Move and
Replace. Add is fairly straightforward (and easy to read, but both Move and
Replace look tricky enough that I'd like a comment explaining what the method is
supposed to do. Those comments would also make reading Accumulate easier (it's
much trickier than it used to be).

[nickie, 2016/06/08 11:38:46]

Done.  I simplified this by merging Move and Replace.

+    DCHECK_EQ(reported_errors_end_, reported_errors_->length());
+    reported_errors_->Add(e, zone_);
+    reported_errors_end_++;
+  }
+
+  V8_INLINE void Move(int i) {
+    DCHECK_LE(reported_errors_end_, i);
+    DCHECK_LT(i, reported_errors_->length());
+    if (reported_errors_end_ < i)
+      reported_errors_->at(reported_errors_end_) = reported_errors_->at(i);
+    reported_errors_end_++;
+  }
+
+  V8_INLINE void Replace(int j, int i) {
+    DCHECK_LT(i, reported_errors_->length());
+    DCHECK_LE(j, reported_errors_end_);
+    DCHECK(j == reported_errors_end_ ||
+           reported_errors_->at(j).kind == kUnusedError);
+    if (i != j) reported_errors_->at(j) = reported_errors_->at(i);
+  }
+
   Zone* zone_;
   ZoneList<typename Traits::Type::Expression>* non_patterns_to_rewrite_;
   int non_pattern_begin_;
-  unsigned invalid_productions_;
-  unsigned function_properties_;
-  // TODO(ishell): consider using Zone[Hash]Map<TargetProduction, Error>
-  // here to consume less stack space during parsing.
-  Error expression_error_;
-  Error formal_parameter_initializer_error_;
-  Error binding_pattern_error_;
-  Error assignment_pattern_error_;
-  Error arrow_formal_parameters_error_;
-  Error duplicate_formal_parameter_error_;
-  Error strict_mode_formal_parameter_error_;
-  Error let_pattern_error_;
-  Error cover_initialized_name_error_;
-  Error tail_call_expression_error_;
-  Error async_arrow_formal_parameters_error_;
-  Error async_binding_pattern_error_;
+  unsigned invalid_productions_ : 14;
+  unsigned function_properties_ : 2;
+  ZoneList<Error>* reported_errors_;
+  int reported_errors_begin_;
+  int reported_errors_end_;
   DuplicateFinder* duplicate_finder_;
 };
 
 
+#undef ERROR_CODES
+
+
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_PARSING_EXPRESSION_CLASSIFIER_H