Revert "[preparser] Refactor VariableProxies to use ThreadedLists interface"

This reverts commit 78f8ff95.

Reason for revert: Causing failures on ClusterFuzz and flakes on the waterfall.
BUG: v8:8166, chromium:883042, chromium:883054, chromium:883119, chromium:883110

Original change's description:
> [preparser] Refactor VariableProxies to use ThreadedLists interface
>
> Bug: v8:7926,
> Change-Id: Idfc520b67696c8a838a0ee297ea392d416dd899e
> Reviewed-on: https://chromium-review.googlesource.com/1206292
> Commit-Queue: Florian Sattler <sattlerf@google.com>
> Reviewed-by: Igor Sheludko <ishell@chromium.org>
> Reviewed-by: Marja Hölttä <marja@chromium.org>
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#55801}

TBR=marja@chromium.org,cbruni@chromium.org,ishell@chromium.org,verwaest@chromium.org,sattlerf@google.com

Change-Id: Ibebff76b5ae69b9790b73c6bd1d53beff5d53673
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7926
Reviewed-on: https://chromium-review.googlesource.com/1221227Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55819}

BUILD.gn

@@ -1581,7 +1581,6 @@ v8_source_set("v8_base") {
     "src/ast/modules.h",
     "src/ast/prettyprinter.cc",
     "src/ast/prettyprinter.h",
-    "src/ast/scopes-inl.h",
     "src/ast/scopes.cc",
     "src/ast/scopes.h",
     "src/ast/variables.cc",

src/ast/ast.h

@@ -1577,7 +1577,8 @@ class VariableProxy final : public Expression {
   // Bind this proxy to the variable var.
   void BindTo(Variable* var);
 
-  V8_INLINE VariableProxy* next_unresolved() { return next_unresolved_; }
+  void set_next_unresolved(VariableProxy* next) { next_unresolved_ = next; }
+  VariableProxy* next_unresolved() { return next_unresolved_; }
 
   // Provides an access type for the ThreadedList used by the PreParsers
   // expressions, lists, and formal parameters.
@@ -1620,14 +1621,10 @@ class VariableProxy final : public Expression {
     const AstRawString* raw_name_;  // if !is_resolved_
     Variable* var_;                 // if is_resolved_
   };
-
-  V8_INLINE VariableProxy** next() { return &next_unresolved_; }
   VariableProxy* next_unresolved_;
 
   VariableProxy** pre_parser_expr_next() { return &pre_parser_expr_next_; }
   VariableProxy* pre_parser_expr_next_;
-
-  friend ThreadedListTraits<VariableProxy>;
 };
 
 // Left-hand side can only be a property, a global or a (parameter or local)

src/ast/scopes-inl.h

-// Copyright 2018 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_AST_SCOPES_INL_H_
-#define V8_AST_SCOPES_INL_H_
-
-#include "src/ast/scopes.h"
-
-namespace v8 {
-namespace internal {
-
-template <typename T>
-void Scope::ResolveScopesThenForEachVariable(DeclarationScope* max_outer_scope,
-                                             T variable_proxy_stackvisitor,
-                                             ParseInfo* info) {
-  // Module variables must be allocated before variable resolution
-  // to ensure that UpdateNeedsHoleCheck() can detect import variables.
-  if (info != nullptr && is_module_scope()) {
-    AsModuleScope()->AllocateModuleVariables();
-  }
-  // Lazy parsed declaration scopes are already partially analyzed. If there are
-  // unresolved references remaining, they just need to be resolved in outer
-  // scopes.
-  Scope* lookup =
-      is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()
-          ? outer_scope()
-          : this;
-
-  for (VariableProxy *proxy = unresolved_list_.first(), *next = nullptr;
-       proxy != nullptr; proxy = next) {
-    next = proxy->next_unresolved();
-
-    DCHECK(!proxy->is_resolved());
-    Variable* var =
-        lookup->LookupRecursive(info, proxy, max_outer_scope->outer_scope());
-    if (var == nullptr) {
-      variable_proxy_stackvisitor(proxy);
-    } else if (var != Scope::kDummyPreParserVariable &&
-               var != Scope::kDummyPreParserLexicalVariable) {
-      if (info != nullptr) {
-        // In this case we need to leave scopes in a way that they can be
-        // allocated. If we resolved variables from lazy parsed scopes, we need
-        // to context allocate the var.
-        ResolveTo(info, proxy, var);
-        if (!var->is_dynamic() && lookup != this) var->ForceContextAllocation();
-      } else {
-        var->set_is_used();
-        if (proxy->is_assigned()) var->set_maybe_assigned();
-      }
-    }
-  }
-
-  // Clear unresolved_list_ as it's in an inconsistent state.
-  unresolved_list_.Clear();
-
-  for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
-    scope->ResolveScopesThenForEachVariable(max_outer_scope,
-                                            variable_proxy_stackvisitor, info);
-  }
-}
-
-}  // namespace internal
-}  // namespace v8
-
-#endif  // V8_AST_SCOPES_INL_H_

src/ast/scopes.cc

@@ -8,7 +8,6 @@
 
 #include "src/accessors.h"
 #include "src/ast/ast.h"
-#include "src/ast/scopes-inl.h"
 #include "src/base/optional.h"
 #include "src/bootstrapper.h"
 #include "src/counters.h"
@@ -24,11 +23,15 @@ namespace v8 {
 namespace internal {
 
 namespace {
+void* kDummyPreParserVariable = reinterpret_cast<void*>(0x1);
+void* kDummyPreParserLexicalVariable = reinterpret_cast<void*>(0x2);
+
 bool IsLexical(Variable* variable) {
-  if (variable == Scope::kDummyPreParserLexicalVariable) return true;
-  if (variable == Scope::kDummyPreParserVariable) return false;
+  if (variable == kDummyPreParserLexicalVariable) return true;
+  if (variable == kDummyPreParserVariable) return false;
   return IsLexicalVariableMode(variable->mode());
 }
+
 }  // namespace
 
 // ----------------------------------------------------------------------------
@@ -73,9 +76,8 @@ Variable* VariableMap::DeclareName(Zone* zone, const AstRawString* name,
   if (p->value == nullptr) {
     // The variable has not been declared yet -> insert it.
     DCHECK_EQ(name, p->key);
-    p->value = mode == VariableMode::kVar
-                   ? Scope::kDummyPreParserVariable
-                   : Scope::kDummyPreParserLexicalVariable;
+    p->value = mode == VariableMode::kVar ? kDummyPreParserVariable
+                                          : kDummyPreParserLexicalVariable;
   }
   return reinterpret_cast<Variable*>(p->value);
 }
@@ -152,7 +154,7 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type)
 Scope::Snapshot::Snapshot(Scope* scope)
     : outer_scope_(scope),
       top_inner_scope_(scope->inner_scope_),
-      top_unresolved_(scope->unresolved_list_.first()),
+      top_unresolved_(scope->unresolved_),
       top_local_(scope->GetClosureScope()->locals_.end()),
       top_decl_(scope->GetClosureScope()->decls_.end()),
       outer_scope_calls_eval_(scope->scope_calls_eval_) {
@@ -335,7 +337,7 @@ void Scope::SetDefaults() {
 #endif
   inner_scope_ = nullptr;
   sibling_ = nullptr;
-  unresolved_list_.Clear();
+  unresolved_ = nullptr;
 
   start_position_ = kNoSourcePosition;
   end_position_ = kNoSourcePosition;
@@ -832,9 +834,16 @@ Scope* Scope::FinalizeBlockScope() {
   }
 
   // Move unresolved variables
-  if (!unresolved_list_.is_empty()) {
-    outer_scope()->unresolved_list_.Prepend(std::move(unresolved_list_));
-    unresolved_list_.Clear();
+  if (unresolved_ != nullptr) {
+    if (outer_scope()->unresolved_ != nullptr) {
+      VariableProxy* unresolved = unresolved_;
+      while (unresolved->next_unresolved() != nullptr) {
+        unresolved = unresolved->next_unresolved();
+      }
+      unresolved->set_next_unresolved(outer_scope()->unresolved_);
+    }
+    outer_scope()->unresolved_ = unresolved_;
+    unresolved_ = nullptr;
   }
 
   if (inner_scope_calls_eval_) outer_scope()->inner_scope_calls_eval_ = true;
@@ -878,7 +887,7 @@ void Scope::Snapshot::Reparent(DeclarationScope* new_parent) const {
   DCHECK_EQ(new_parent->outer_scope_, outer_scope_);
   DCHECK_EQ(new_parent, new_parent->GetClosureScope());
   DCHECK_NULL(new_parent->inner_scope_);
-  DCHECK(new_parent->unresolved_list_.is_empty());
+  DCHECK_NULL(new_parent->unresolved_);
   DCHECK(new_parent->locals_.is_empty());
   Scope* inner_scope = new_parent->sibling_;
   if (inner_scope != top_inner_scope_) {
@@ -901,21 +910,14 @@ void Scope::Snapshot::Reparent(DeclarationScope* new_parent) const {
     new_parent->sibling_ = top_inner_scope_;
   }
 
-  if (outer_scope_->unresolved_list_.first() != top_unresolved_) {
-    // If the marked VariableProxy (snapshoted) is not the first, we need to
-    // find it and move all VariableProxys up to that point into the new_parent,
-    // then we restore the snapshoted state by reinitializing the outer_scope
-    // list.
-    {
-      auto iter = outer_scope_->unresolved_list_.begin();
-      while (iter->next_unresolved() != top_unresolved_) {
-        ++iter;
-      }
-      outer_scope_->unresolved_list_.Rewind(iter);
+  if (outer_scope_->unresolved_ != top_unresolved_) {
+    VariableProxy* last = outer_scope_->unresolved_;
+    while (last->next_unresolved() != top_unresolved_) {
+      last = last->next_unresolved();
     }
-
-    new_parent->unresolved_list_ = std::move(outer_scope_->unresolved_list_);
-    outer_scope_->unresolved_list_.ReinitializeHead(top_unresolved_);
+    last->set_next_unresolved(nullptr);
+    new_parent->unresolved_ = outer_scope_->unresolved_;
+    outer_scope_->unresolved_ = top_unresolved_;
   }
 
   // TODO(verwaest): This currently only moves do-expression declared variables
@@ -1259,7 +1261,8 @@ void Scope::DeclareCatchVariableName(const AstRawString* name) {
 void Scope::AddUnresolved(VariableProxy* proxy) {
   DCHECK(!already_resolved_);
   DCHECK(!proxy->is_resolved());
-  unresolved_list_.AddFront(proxy);
+  proxy->set_next_unresolved(unresolved_);
+  unresolved_ = proxy;
 }
 
 Variable* DeclarationScope::DeclareDynamicGlobal(const AstRawString* name,
@@ -1271,7 +1274,22 @@ Variable* DeclarationScope::DeclareDynamicGlobal(const AstRawString* name,
 }
 
 bool Scope::RemoveUnresolved(VariableProxy* var) {
-  return unresolved_list_.Remove(var);
+  if (unresolved_ == var) {
+    unresolved_ = var->next_unresolved();
+    var->set_next_unresolved(nullptr);
+    return true;
+  }
+  VariableProxy* current = unresolved_;
+  while (current != nullptr) {
+    VariableProxy* next = current->next_unresolved();
+    if (var == next) {
+      current->set_next_unresolved(next->next_unresolved());
+      var->set_next_unresolved(nullptr);
+      return true;
+    }
+    current = next;
+  }
+  return false;
 }
 
 Variable* Scope::NewTemporary(const AstRawString* name) {
@@ -1465,12 +1483,11 @@ Scope* Scope::GetOuterScopeWithContext() {
 
 Handle<StringSet> DeclarationScope::CollectNonLocals(
     Isolate* isolate, ParseInfo* info, Handle<StringSet> non_locals) {
-  ResolveScopesThenForEachVariable(this,
-                                   [=, &non_locals](VariableProxy* proxy) {
-                                     non_locals = StringSet::Add(
-                                         isolate, non_locals, proxy->name());
-                                   },
-                                   info);
+  VariableProxy* free_variables = FetchFreeVariables(this, info);
+  for (VariableProxy* proxy = free_variables; proxy != nullptr;
+       proxy = proxy->next_unresolved()) {
+    non_locals = StringSet::Add(isolate, non_locals, proxy->name());
+  }
   return non_locals;
 }
 
@@ -1487,7 +1504,7 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   decls_.Clear();
   locals_.Clear();
   inner_scope_ = nullptr;
-  unresolved_list_.Clear();
+  unresolved_ = nullptr;
   sloppy_block_function_map_ = nullptr;
   rare_data_ = nullptr;
   has_rest_ = false;
@@ -1533,7 +1550,8 @@ void DeclarationScope::SavePreParsedScopeDataForDeclarationScope() {
 
 void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
   DCHECK(!force_eager_compilation_);
-  ThreadedList<VariableProxy> new_unresolved_list;
+  VariableProxy* unresolved = nullptr;
+
   if (!outer_scope_->is_script_scope() ||
       (FLAG_preparser_scope_analysis &&
        preparsed_scope_data_builder_ != nullptr &&
@@ -1541,11 +1559,13 @@ void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
     // Try to resolve unresolved variables for this Scope and migrate those
     // which cannot be resolved inside. It doesn't make sense to try to resolve
     // them in the outer Scopes here, because they are incomplete.
-    ResolveScopesThenForEachVariable(
-        this, [=, &new_unresolved_list](VariableProxy* proxy) {
-          VariableProxy* copy = ast_node_factory->CopyVariableProxy(proxy);
-          new_unresolved_list.AddFront(copy);
-        });
+    for (VariableProxy* proxy = FetchFreeVariables(this); proxy != nullptr;
+         proxy = proxy->next_unresolved()) {
+      DCHECK(!proxy->is_resolved());
+      VariableProxy* copy = ast_node_factory->CopyVariableProxy(proxy);
+      copy->set_next_unresolved(unresolved);
+      unresolved = copy;
+    }
 
     // Migrate function_ to the right Zone.
     if (function_ != nullptr) {
@@ -1566,7 +1586,7 @@ void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
 
   ResetAfterPreparsing(ast_node_factory->ast_value_factory(), false);
 
-  unresolved_list_ = std::move(new_unresolved_list);
+  unresolved_ = unresolved;
 }
 
 #ifdef DEBUG
@@ -1653,8 +1673,8 @@ void PrintMap(int indent, const char* label, VariableMap* map, bool locals,
   for (VariableMap::Entry* p = map->Start(); p != nullptr; p = map->Next(p)) {
     Variable* var = reinterpret_cast<Variable*>(p->value);
     if (var == function_var) continue;
-    if (var == Scope::kDummyPreParserVariable ||
-        var == Scope::kDummyPreParserLexicalVariable) {
+    if (var == kDummyPreParserVariable ||
+        var == kDummyPreParserLexicalVariable) {
       continue;
     }
     bool local = !IsDynamicVariableMode(var->mode());
@@ -2025,7 +2045,8 @@ bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
   // scopes.
   if (is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()) {
     DCHECK_EQ(variables_.occupancy(), 0);
-    for (VariableProxy* proxy : unresolved_list_) {
+    for (VariableProxy* proxy = unresolved_; proxy != nullptr;
+         proxy = proxy->next_unresolved()) {
       Variable* var = outer_scope()->LookupRecursive(info, proxy, nullptr);
       if (var == nullptr) {
         DCHECK(proxy->is_private_field());
@@ -2039,7 +2060,8 @@ bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
     }
   } else {
     // Resolve unresolved variables for this scope.
-    for (VariableProxy* proxy : unresolved_list_) {
+    for (VariableProxy* proxy = unresolved_; proxy != nullptr;
+         proxy = proxy->next_unresolved()) {
       if (!ResolveVariable(info, proxy)) return false;
     }
 
@@ -2052,6 +2074,57 @@ bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
   return true;
 }
 
+VariableProxy* Scope::FetchFreeVariables(DeclarationScope* max_outer_scope,
+                                         ParseInfo* info,
+                                         VariableProxy* stack) {
+  // Module variables must be allocated before variable resolution
+  // to ensure that UpdateNeedsHoleCheck() can detect import variables.
+  if (info != nullptr && is_module_scope()) {
+    AsModuleScope()->AllocateModuleVariables();
+  }
+  // Lazy parsed declaration scopes are already partially analyzed. If there are
+  // unresolved references remaining, they just need to be resolved in outer
+  // scopes.
+  Scope* lookup =
+      is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()
+          ? outer_scope()
+          : this;
+  for (VariableProxy *proxy = unresolved_, *next = nullptr; proxy != nullptr;
+       proxy = next) {
+    next = proxy->next_unresolved();
+    DCHECK(!proxy->is_resolved());
+    Variable* var =
+        lookup->LookupRecursive(info, proxy, max_outer_scope->outer_scope());
+    if (var == nullptr) {
+      proxy->set_next_unresolved(stack);
+      stack = proxy;
+    } else if (var != kDummyPreParserVariable &&
+               var != kDummyPreParserLexicalVariable) {
+      if (info != nullptr) {
+        // In this case we need to leave scopes in a way that they can be
+        // allocated. If we resolved variables from lazy parsed scopes, we need
+        // to context allocate the var.
+        ResolveTo(info, proxy, var);
+        if (!var->is_dynamic() && lookup != this) var->ForceContextAllocation();
+      } else {
+        var->set_is_used();
+        if (proxy->is_assigned()) {
+          var->set_maybe_assigned();
+        }
+      }
+    }
+  }
+
+  // Clear unresolved_ as it's in an inconsistent state.
+  unresolved_ = nullptr;
+
+  for (Scope* scope = inner_scope_; scope != nullptr; scope = scope->sibling_) {
+    stack = scope->FetchFreeVariables(max_outer_scope, info, stack);
+  }
+
+  return stack;
+}
+
 bool Scope::MustAllocate(Variable* var) {
   if (var == kDummyPreParserLexicalVariable || var == kDummyPreParserVariable) {
     return true;
@@ -2337,9 +2410,5 @@ int Scope::ContextLocalCount() const {
          (is_function_var_in_context ? 1 : 0);
 }
 
-void* const Scope::kDummyPreParserVariable = reinterpret_cast<void*>(0x1);
-void* const Scope::kDummyPreParserLexicalVariable =
-    reinterpret_cast<void*>(0x2);
-
 }  // namespace internal
 }  // namespace v8

src/ast/scopes.h

@@ -217,7 +217,8 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
     DCHECK(!already_resolved_);
     DCHECK_EQ(factory->zone(), zone());
     VariableProxy* proxy = factory->NewVariableProxy(name, kind, start_pos);
-    AddUnresolved(proxy);
+    proxy->set_next_unresolved(unresolved_);
+    unresolved_ = proxy;
     return proxy;
   }
 
@@ -478,9 +479,6 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
     return false;
   }
 
-  static void* const kDummyPreParserVariable;
-  static void* const kDummyPreParserLexicalVariable;
-
  protected:
   explicit Scope(Zone* zone);
 
@@ -526,7 +524,7 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   ThreadedList<Variable> locals_;
   // Unresolved variables referred to from this scope. The proxies themselves
   // form a linked list of all unresolved proxies.
-  ThreadedList<VariableProxy> unresolved_list_;
+  VariableProxy* unresolved_;
   // Declarations.
   ThreadedList<Declaration> decls_;
 
@@ -598,10 +596,9 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
   // Finds free variables of this scope. This mutates the unresolved variables
   // list along the way, so full resolution cannot be done afterwards.
   // If a ParseInfo* is passed, non-free variables will be resolved.
-  template <typename T>
-  void ResolveScopesThenForEachVariable(DeclarationScope* max_outer_scope,
-                                        T variable_proxy_stackvisitor,
-                                        ParseInfo* info = nullptr);
+  VariableProxy* FetchFreeVariables(DeclarationScope* max_outer_scope,
+                                    ParseInfo* info = nullptr,
+                                    VariableProxy* stack = nullptr);
 
   // Predicates.
   bool MustAllocate(Variable* var);

src/parsing/preparser.h

@@ -126,7 +126,7 @@ class PreParserExpression {
                                    right.variables_);
       }
       if (right.variables_ != nullptr) {
-        left.variables_->Append(std::move(*right.variables_));
+        left.variables_->Append(right.variables_);
       }
       return PreParserExpression(TypeField::encode(kExpression),
                                  left.variables_);
@@ -457,7 +457,7 @@ inline void PreParserList<PreParserExpression>::Add(
     if (variables_ == nullptr) {
       variables_ = new (zone) VariableZoneThreadedListType();
     }
-    variables_->Append(std::move(*expression.variables_));
+    variables_->Append(expression.variables_);
   }
   ++length_;
 }

src/utils.h

@@ -1646,65 +1646,9 @@ class ThreadedListBase final : public BaseClass {
     tail_ = TLTraits::next(v);
   }
 
-  void AddFront(T* v) {
-    DCHECK_NULL(*TLTraits::next(v));
-    DCHECK_NOT_NULL(v);
-    T** const next = TLTraits::next(v);
-
-    *next = head_;
-    if (head_ == nullptr) tail_ = next;
-    head_ = v;
-  }
-
-  // Reinitializing the head to a new node, this costs O(n).
-  void ReinitializeHead(T* v) {
-    SLOW_DCHECK(Verify());
-
-    head_ = v;
-    T* current = v;
-    if (current != nullptr) {  // Find tail
-      T* tmp;
-      while ((tmp = *TLTraits::next(current))) {
-        current = tmp;
-      }
-      tail_ = TLTraits::next(current);
-    } else {
-      tail_ = &head_;
-    }
-  }
-
-  void DropHead() {
-    DCHECK_NOT_NULL(head_);
-    SLOW_DCHECK(Verify());
-
-    T* old_head = head_;
-    head_ = *TLTraits::next(head_);
-    if (head_ == nullptr) tail_ = &head_;
-    *TLTraits::next(old_head) = nullptr;
-  }
-
-  void Append(ThreadedListBase&& list) {
-    SLOW_DCHECK(Verify());
-    SLOW_DCHECK(list.Verify());
-
-    *tail_ = list.head_;
-    tail_ = list.tail_;
-    list.Clear();
-  }
-
-  void Prepend(ThreadedListBase&& list) {
-    SLOW_DCHECK(Verify());
-    SLOW_DCHECK(list.Verify());
-
-    if (list.head_ == nullptr) return;
-
-    T* new_head = list.head_;
-    *list.tail_ = head_;
-    if (head_ == nullptr) {
-      tail_ = list.tail_;
-    }
-    head_ = new_head;
-    list.Clear();
+  void Append(ThreadedListBase* list) {
+    *tail_ = list->head_;
+    tail_ = list->tail_;
   }
 
   void Clear() {
@@ -1712,67 +1656,13 @@ class ThreadedListBase final : public BaseClass {
     tail_ = &head_;
   }
 
-  ThreadedListBase& operator=(ThreadedListBase&& other) V8_NOEXCEPT {
-    head_ = other.head_;
-    tail_ = other.head_ ? other.tail_ : &head_;
-#ifdef DEBUG
-    other.Clear();
-#endif
-    return *this;
-  }
-
-  ThreadedListBase(ThreadedListBase&& other) V8_NOEXCEPT
-      : head_(other.head_),
-        tail_(other.head_ ? other.tail_ : &head_) {
-#ifdef DEBUG
-    other.Clear();
-#endif
-  }
-
-  bool Remove(T* v) {
-    SLOW_DCHECK(Verify());
-
-    T* current = first();
-    if (current == v) {
-      DropHead();
-      return true;
-    }
-
-    while (current != nullptr) {
-      T* next = *TLTraits::next(current);
-      if (next == v) {
-        *TLTraits::next(current) = *TLTraits::next(next);
-        *TLTraits::next(next) = nullptr;
-
-        if (TLTraits::next(next) == tail_) {
-          tail_ = TLTraits::next(current);
-        }
-        return true;
-      }
-      current = next;
-    }
-    return false;
-  }
-
   class Iterator final {
-   public:
-    using iterator_category = std::forward_iterator_tag;
-    using difference_type = std::ptrdiff_t;
-    using value_type = T*;
-    using reference = value_type;
-    using pointer = value_type*;
-
    public:
     Iterator& operator++() {
       entry_ = TLTraits::next(*entry_);
       return *this;
     }
-    bool operator==(const Iterator& other) const {
-      return entry_ == other.entry_;
-    }
-    bool operator!=(const Iterator& other) const {
-      return entry_ != other.entry_;
-    }
+    bool operator!=(const Iterator& other) { return entry_ != other.entry_; }
     T* operator*() { return *entry_; }
     T* operator->() { return *entry_; }
     Iterator& operator=(T* entry) {
@@ -1791,22 +1681,12 @@ class ThreadedListBase final : public BaseClass {
   };
 
   class ConstIterator final {
-   public:
-    using iterator_category = std::forward_iterator_tag;
-    using difference_type = std::ptrdiff_t;
-    using value_type = T*;
-    using reference = const value_type;
-    using pointer = const value_type*;
-
    public:
     ConstIterator& operator++() {
       entry_ = TLTraits::next(*entry_);
       return *this;
     }
-    bool operator==(const ConstIterator& other) const {
-      return entry_ == other.entry_;
-    }
-    bool operator!=(const ConstIterator& other) const {
+    bool operator!=(const ConstIterator& other) {
       return entry_ != other.entry_;
     }
     const T* operator*() const { return *entry_; }
@@ -1825,34 +1705,23 @@ class ThreadedListBase final : public BaseClass {
   ConstIterator begin() const { return ConstIterator(&head_); }
   ConstIterator end() const { return ConstIterator(tail_); }
 
-  // Rewinds the list's tail to the reset point, i.e., cutting of the rest of
-  // the list.
   void Rewind(Iterator reset_point) {
-    SLOW_DCHECK(Verify());
-
     tail_ = reset_point.entry_;
     *tail_ = nullptr;
   }
 
-  // Moves the tail of the from_list, starting at the from_location, to the end
-  // of this list.
-  void MoveTail(ThreadedListBase* from_list, Iterator from_location) {
-    SLOW_DCHECK(Verify());
-
-    if (from_list->end() != from_location) {
+  void MoveTail(ThreadedListBase<T, BaseClass>* parent, Iterator location) {
+    if (parent->end() != location) {
       DCHECK_NULL(*tail_);
-      *tail_ = *from_location;
-      tail_ = from_list->tail_;
-      from_list->Rewind(from_location);
-
-      SLOW_DCHECK(Verify());
-      SLOW_DCHECK(from_list->Verify());
+      *tail_ = *location;
+      tail_ = parent->tail_;
+      parent->Rewind(location);
     }
   }
 
   bool is_empty() const { return head_ == nullptr; }
 
-  T* first() const { return head_; }
+  T* first() { return head_; }
 
   // Slow. For testing purposes.
   int LengthForTest() {
@@ -1860,26 +1729,12 @@ class ThreadedListBase final : public BaseClass {
     for (Iterator t = begin(); t != end(); ++t) ++result;
     return result;
   }
-
   T* AtForTest(int i) {
     Iterator t = begin();
     while (i-- > 0) ++t;
     return *t;
   }
 
-  bool Verify() {
-    T* last = this->first();
-    if (last == nullptr) {
-      CHECK_EQ(&head_, tail_);
-    } else {
-      while (*TLTraits::next(last) != nullptr) {
-        last = *TLTraits::next(last);
-      }
-      CHECK_EQ(TLTraits::next(last), tail_);
-    }
-    return true;
-  }
-
  private:
   T* head_;
   T** tail_;

test/unittests/BUILD.gn

@@ -196,7 +196,6 @@ v8_source_set("unittests_sources") {
     "torque/earley-parser-unittest.cc",
     "unicode-unittest.cc",
     "utils-unittest.cc",
-    "utils/threaded-list.cc",
     "value-serializer-unittest.cc",
     "wasm/control-transfer-unittest.cc",
     "wasm/decoder-unittest.cc",

test/unittests/utils/threaded-list.cc

-// Copyright 2018 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.
-
-#include <iterator>
-
-#include "src/v8.h"
-
-#include "src/utils.h"
-#include "testing/gtest-support.h"
-
-namespace v8 {
-namespace internal {
-
-struct ThreadedListTestNode {
-  ThreadedListTestNode() : next_(nullptr), other_next_(nullptr) {}
-
-  ThreadedListTestNode** next() { return &next_; }
-
-  ThreadedListTestNode* next_;
-
-  struct OtherTraits {
-    static ThreadedListTestNode** next(ThreadedListTestNode* t) {
-      return t->other_next();
-    }
-  };
-
-  ThreadedListTestNode** other_next() { return &other_next_; }
-
-  ThreadedListTestNode* other_next_;
-};
-
-struct ThreadedListTest : public ::testing::Test {
-  static const size_t INIT_NODES = 5;
-  ThreadedListTest() {}
-
-  void SetUp() override {
-    for (size_t i = 0; i < INIT_NODES; i++) {
-      nodes[i] = ThreadedListTestNode();
-    }
-
-    for (size_t i = 0; i < INIT_NODES; i++) {
-      list.Add(&nodes[i]);
-      normal_next_list.Add(&nodes[i]);
-    }
-
-    // Verify if setup worked
-    CHECK(list.Verify());
-    CHECK_EQ(list.LengthForTest(), INIT_NODES);
-    CHECK(normal_next_list.Verify());
-    CHECK_EQ(normal_next_list.LengthForTest(), INIT_NODES);
-
-    extra_test_node_0 = ThreadedListTestNode();
-    extra_test_node_1 = ThreadedListTestNode();
-    extra_test_node_2 = ThreadedListTestNode();
-
-    extra_test_list.Add(&extra_test_node_0);
-    extra_test_list.Add(&extra_test_node_1);
-    extra_test_list.Add(&extra_test_node_2);
-    CHECK_EQ(extra_test_list.LengthForTest(), 3);
-    CHECK(extra_test_list.Verify());
-
-    normal_extra_test_list.Add(&extra_test_node_0);
-    normal_extra_test_list.Add(&extra_test_node_1);
-    normal_extra_test_list.Add(&extra_test_node_2);
-    CHECK_EQ(normal_extra_test_list.LengthForTest(), 3);
-    CHECK(normal_extra_test_list.Verify());
-  }
-
-  void TearDown() override {
-    // Check if the normal list threaded through next is still untouched.
-    CHECK(normal_next_list.Verify());
-    CHECK_EQ(normal_next_list.LengthForTest(), INIT_NODES);
-    CHECK_EQ(normal_next_list.AtForTest(0), &nodes[0]);
-    CHECK_EQ(normal_next_list.AtForTest(4), &nodes[4]);
-    CHECK(normal_extra_test_list.Verify());
-    CHECK_EQ(normal_extra_test_list.LengthForTest(), 3);
-    CHECK_EQ(normal_extra_test_list.AtForTest(0), &extra_test_node_0);
-    CHECK_EQ(normal_extra_test_list.AtForTest(2), &extra_test_node_2);
-
-    list.Clear();
-    extra_test_list.Clear();
-  }
-
-  ThreadedListTestNode nodes[INIT_NODES];
-  ThreadedList<ThreadedListTestNode, ThreadedListTestNode::OtherTraits> list;
-  ThreadedList<ThreadedListTestNode> normal_next_list;
-
-  ThreadedList<ThreadedListTestNode, ThreadedListTestNode::OtherTraits>
-      extra_test_list;
-  ThreadedList<ThreadedListTestNode> normal_extra_test_list;
-  ThreadedListTestNode extra_test_node_0;
-  ThreadedListTestNode extra_test_node_1;
-  ThreadedListTestNode extra_test_node_2;
-};
-
-TEST_F(ThreadedListTest, Add) {
-  CHECK_EQ(list.LengthForTest(), 5);
-  ThreadedListTestNode new_node;
-  // Add to existing list
-  list.Add(&new_node);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 6);
-  CHECK_EQ(list.AtForTest(5), &new_node);
-
-  list.Clear();
-  CHECK_EQ(list.LengthForTest(), 0);
-
-  new_node = ThreadedListTestNode();
-  // Add to empty list
-  list.Add(&new_node);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 1);
-  CHECK_EQ(list.AtForTest(0), &new_node);
-}
-
-TEST_F(ThreadedListTest, AddFront) {
-  CHECK_EQ(list.LengthForTest(), 5);
-  ThreadedListTestNode new_node;
-  // AddFront to existing list
-  list.AddFront(&new_node);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 6);
-  CHECK_EQ(list.first(), &new_node);
-
-  list.Clear();
-  CHECK_EQ(list.LengthForTest(), 0);
-
-  new_node = ThreadedListTestNode();
-  // AddFront to empty list
-  list.AddFront(&new_node);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 1);
-  CHECK_EQ(list.first(), &new_node);
-}
-
-TEST_F(ThreadedListTest, ReinitializeHead) {
-  CHECK_EQ(list.LengthForTest(), 5);
-  CHECK_NE(extra_test_list.first(), list.first());
-  list.ReinitializeHead(&extra_test_node_0);
-  list.Verify();
-  CHECK_EQ(extra_test_list.first(), list.first());
-  CHECK_EQ(extra_test_list.end(), list.end());
-  CHECK_EQ(extra_test_list.LengthForTest(), 3);
-}
-
-TEST_F(ThreadedListTest, DropHead) {
-  CHECK_EQ(extra_test_list.LengthForTest(), 3);
-  CHECK_EQ(extra_test_list.first(), &extra_test_node_0);
-  extra_test_list.DropHead();
-  extra_test_list.Verify();
-  CHECK_EQ(extra_test_list.first(), &extra_test_node_1);
-  CHECK_EQ(extra_test_list.LengthForTest(), 2);
-}
-
-TEST_F(ThreadedListTest, Append) {
-  auto initial_extra_list_end = extra_test_list.end();
-  CHECK_EQ(list.LengthForTest(), 5);
-  list.Append(std::move(extra_test_list));
-  list.Verify();
-  extra_test_list.Verify();
-  CHECK(extra_test_list.is_empty());
-  CHECK_EQ(list.LengthForTest(), 8);
-  CHECK_EQ(list.AtForTest(4), &nodes[4]);
-  CHECK_EQ(list.AtForTest(5), &extra_test_node_0);
-  CHECK_EQ(list.end(), initial_extra_list_end);
-}
-
-TEST_F(ThreadedListTest, Prepend) {
-  CHECK_EQ(list.LengthForTest(), 5);
-  list.Prepend(std::move(extra_test_list));
-  list.Verify();
-  extra_test_list.Verify();
-  CHECK(extra_test_list.is_empty());
-  CHECK_EQ(list.LengthForTest(), 8);
-  CHECK_EQ(list.first(), &extra_test_node_0);
-  CHECK_EQ(list.AtForTest(2), &extra_test_node_2);
-  CHECK_EQ(list.AtForTest(3), &nodes[0]);
-}
-
-TEST_F(ThreadedListTest, Clear) {
-  CHECK_NE(list.LengthForTest(), 0);
-  list.Clear();
-  CHECK_EQ(list.LengthForTest(), 0);
-  CHECK_NULL(list.first());
-}
-
-TEST_F(ThreadedListTest, MoveAssign) {
-  ThreadedList<ThreadedListTestNode, ThreadedListTestNode::OtherTraits> m_list;
-  CHECK_EQ(extra_test_list.LengthForTest(), 3);
-  m_list = std::move(extra_test_list);
-
-  m_list.Verify();
-  CHECK_EQ(m_list.first(), &extra_test_node_0);
-  CHECK_EQ(m_list.LengthForTest(), 3);
-
-  // move assign from empty list
-  extra_test_list.Clear();
-  CHECK_EQ(extra_test_list.LengthForTest(), 0);
-  m_list = std::move(extra_test_list);
-  CHECK_EQ(m_list.LengthForTest(), 0);
-
-  m_list.Verify();
-  CHECK_NULL(m_list.first());
-}
-
-TEST_F(ThreadedListTest, MoveCtor) {
-  CHECK_EQ(extra_test_list.LengthForTest(), 3);
-  ThreadedList<ThreadedListTestNode, ThreadedListTestNode::OtherTraits> m_list(
-      std::move(extra_test_list));
-
-  m_list.Verify();
-  CHECK_EQ(m_list.LengthForTest(), 3);
-  CHECK_EQ(m_list.first(), &extra_test_node_0);
-
-  // move construct from empty list
-  extra_test_list.Clear();
-  CHECK_EQ(extra_test_list.LengthForTest(), 0);
-  ThreadedList<ThreadedListTestNode, ThreadedListTestNode::OtherTraits> m_list2(
-      std::move(extra_test_list));
-  CHECK_EQ(m_list2.LengthForTest(), 0);
-
-  m_list2.Verify();
-  CHECK_NULL(m_list2.first());
-}
-
-TEST_F(ThreadedListTest, Remove) {
-  CHECK_EQ(list.LengthForTest(), 5);
-
-  // Remove first
-  CHECK_EQ(list.first(), &nodes[0]);
-  list.Remove(&nodes[0]);
-  list.Verify();
-  CHECK_EQ(list.first(), &nodes[1]);
-  CHECK_EQ(list.LengthForTest(), 4);
-
-  // Remove middle
-  list.Remove(&nodes[2]);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 3);
-  CHECK_EQ(list.first(), &nodes[1]);
-  CHECK_EQ(list.AtForTest(1), &nodes[3]);
-
-  // Remove last
-  list.Remove(&nodes[4]);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 2);
-  CHECK_EQ(list.first(), &nodes[1]);
-  CHECK_EQ(list.AtForTest(1), &nodes[3]);
-
-  // Remove rest
-  list.Remove(&nodes[1]);
-  list.Remove(&nodes[3]);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 0);
-
-  // Remove not found
-  list.Remove(&nodes[4]);
-  list.Verify();
-  CHECK_EQ(list.LengthForTest(), 0);
-}
-
-TEST_F(ThreadedListTest, Rewind) {
-  CHECK_EQ(extra_test_list.LengthForTest(), 3);
-  for (auto iter = extra_test_list.begin(); iter != extra_test_list.end();
-       ++iter) {
-    if (*iter == &extra_test_node_2) {
-      extra_test_list.Rewind(iter);
-      break;
-    }
-  }
-  CHECK_EQ(extra_test_list.LengthForTest(), 2);
-  auto iter = extra_test_list.begin();
-  CHECK_EQ(*iter, &extra_test_node_0);
-  std::advance(iter, 1);
-  CHECK_EQ(*iter, &extra_test_node_1);
-
-  extra_test_list.Rewind(extra_test_list.begin());
-  CHECK_EQ(extra_test_list.LengthForTest(), 0);
-}
-
-TEST_F(ThreadedListTest, IterComp) {
-  ThreadedList<ThreadedListTestNode, ThreadedListTestNode::OtherTraits> c_list =
-      std::move(extra_test_list);
-  bool found_first;
-  for (auto iter = c_list.begin(); iter != c_list.end(); ++iter) {
-    // This triggers the operator== on the iterator
-    if (iter == c_list.begin()) {
-      found_first = true;
-    }
-  }
-  CHECK(found_first);
-}
-
-TEST_F(ThreadedListTest, ConstIterComp) {
-  const ThreadedList<ThreadedListTestNode, ThreadedListTestNode::OtherTraits>
-      c_list = std::move(extra_test_list);
-  bool found_first;
-  for (auto iter = c_list.begin(); iter != c_list.end(); ++iter) {
-    // This triggers the operator== on the iterator
-    if (iter == c_list.begin()) {
-      found_first = true;
-    }
-  }
-  CHECK(found_first);
-}
-
-}  // namespace internal
-}  // namespace v8