Simpler implementation of expression classifiers

src/parsing/expression-classifier.h

Index: src/parsing/expression-classifier.h
diff --git a/src/parsing/expression-classifier.h b/src/parsing/expression-classifier.h
index eadd94a027b5a816fc8e2f2ce4aa77869f366a46..a2ae2b1cb2fcfef21e57882401d9300cd3970a2e 100644
--- a/src/parsing/expression-classifier.h
+++ b/src/parsing/expression-classifier.h
@@ -372,61 +372,33 @@ class ExpressionClassifier {
     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;
-    int next = inner->reported_errors_begin_;
-    if (non_arrow_inner_invalid_productions) {
-      unsigned non_arrow_productions =
-          productions & ~ArrowFormalParametersProduction;
-      unsigned errors =
-          non_arrow_productions & non_arrow_inner_invalid_productions;
-      errors &= ~invalid_productions_;
-
-      // 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;
-      }
+    // Propagate errors from inner, excluding arrow formal parameters.
+    unsigned errors = inner->invalid_productions_ & productions &
+                      ~ArrowFormalParametersProduction;
+
+    // If there's nothing to add, the inner classifier's array can be dropped.
+    if (errors & ~invalid_productions_) {

[nickie, 2016/05/18 17:32:41]

In this if-then-else, the condition is reversed.  Stupid bug...

+      reported_errors_->Rewind(reported_errors_end_);

[caitp (gmail), 2016/05/18 15:23:21]

All this does is set the ring buffer's length_ to reported_errors_end_ --- what
do we get out of this? Also, why do we have a `reported_errors_start_` concept?
Does this need to be a ring buffer? It seems like we're doing a lot of start/end
position management with no real purpose, unless I'm just missing it.

+    } else {
+      invalid_productions_ |= errors;
+      reported_errors_end_ = inner->reported_errors_end_;
+    }
 
-      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(next++, i);
-          if (reported_errors_->at(i).kind == kBindingPatternProduction &&
-              errors & ArrowFormalParametersProduction) {
-            if (next <= i) {
-              Move(next, i);
-              reported_errors_->at(next++).kind =
-                  kArrowFormalParametersProduction;
-            } else {
-              DCHECK_EQ(next, i+1);
-              arrow_index = i;
-            }
-          }
-        }
-        if (arrow_index < inner->reported_errors_end_) {
-          Add(reported_errors_->at(arrow_index));
-          reported_errors_->at(next++).kind = kArrowFormalParametersProduction;
-        }
+    // As an exception to error propagation, the result continues to be valid
+    // as 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()) {
+        invalid_productions_ |= ArrowFormalParametersProduction;
+        Add(inner->reported_error(kBindingPatternProduction));
       }
     }
-    DCHECK_EQ(reported_errors_end_, next);
-    reported_errors_->Rewind(next);
-    inner->reported_errors_begin_ = inner->reported_errors_end_ = next;
+    inner->reported_errors_begin_ = inner->reported_errors_end_ =
+        reported_errors_end_;
   }
 
   V8_INLINE int GetNonPatternBegin() const { return non_pattern_begin_; }
@@ -467,14 +439,6 @@ class ExpressionClassifier {
     reported_errors_end_++;
   }
 
-  V8_INLINE void Move(int next, int i) {
-    DCHECK_EQ(reported_errors_end_, next);
-    DCHECK_LE(next, i);
-    DCHECK_LT(i, reported_errors_->length());
-    if (next < i) reported_errors_->at(next++) = reported_errors_->at(i);
-    reported_errors_end_++;
-  }
-
   Zone* zone_;
   ZoneList<typename Traits::Type::Expression>* non_patterns_to_rewrite_;
   int non_pattern_begin_;