Simpler implementation 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"
@@ skipping 354 matching lines @@
     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;
-    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_;
+    // Propagate errors from inner, excluding arrow formal parameters.
+    unsigned errors = inner->invalid_productions_ & productions &
+                      ~ArrowFormalParametersProduction;
 
-      // 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 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 (!inner->is_valid_binding_pattern())
-          errors |= ArrowFormalParametersProduction;
-      }
-
-      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_; }
 
   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_);
@@ skipping 20 matching lines @@
     static Error none;
     return none;
   }
 
   V8_INLINE void Add(const Error& e) {
     DCHECK_EQ(reported_errors_end_, reported_errors_->length());
     reported_errors_->Add(e, zone_);
     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_;
   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