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

This is a reland of 78f8ff95

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}

Bug: v8:7926, chromium:883059
Change-Id: Icaa496be1b4df8306fe6d623e5825909d7b0c9c5
Reviewed-on: https://chromium-review.googlesource.com/1221529
Commit-Queue: Florian Sattler <sattlerf@google.com>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Reviewed-by: Marja Hölttä <marja@chromium.org>
Cr-Commit-Position: refs/heads/master@{#55833}

BUILD.gn

@@ -1584,6 +1584,7 @@ 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,8 +1577,7 @@ class VariableProxy final : public Expression {
   // Bind this proxy to the variable var.
   void BindTo(Variable* var);
 
-  void set_next_unresolved(VariableProxy* next) { next_unresolved_ = next; }
-  VariableProxy* next_unresolved() { return next_unresolved_; }
+  V8_INLINE VariableProxy* next_unresolved() { return next_unresolved_; }
 
   // Provides an access type for the ThreadedList used by the PreParsers
   // expressions, lists, and formal parameters.
@@ -1621,10 +1620,14 @@ 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,6 +8,7 @@
 
 #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"
@@ -23,15 +24,11 @@ namespace v8 {
 namespace internal {
 
 namespace {
-void* kDummyPreParserVariable = reinterpret_cast<void*>(0x1);
-void* kDummyPreParserLexicalVariable = reinterpret_cast<void*>(0x2);
-
 bool IsLexical(Variable* variable) {
-  if (variable == kDummyPreParserLexicalVariable) return true;
-  if (variable == kDummyPreParserVariable) return false;
+  if (variable == Scope::kDummyPreParserLexicalVariable) return true;
+  if (variable == Scope::kDummyPreParserVariable) return false;
   return IsLexicalVariableMode(variable->mode());
 }
-
 }  // namespace
 
 // ----------------------------------------------------------------------------
@@ -76,8 +73,9 @@ 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 ? kDummyPreParserVariable
-                                          : kDummyPreParserLexicalVariable;
+    p->value = mode == VariableMode::kVar
+                   ? Scope::kDummyPreParserVariable
+                   : Scope::kDummyPreParserLexicalVariable;
   }
   return reinterpret_cast<Variable*>(p->value);
 }
@@ -154,7 +152,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_),
+      top_unresolved_(scope->unresolved_list_.first()),
       top_local_(scope->GetClosureScope()->locals_.end()),
       top_decl_(scope->GetClosureScope()->decls_.end()),
       outer_scope_calls_eval_(scope->scope_calls_eval_) {
@@ -337,7 +335,7 @@ void Scope::SetDefaults() {
 #endif
   inner_scope_ = nullptr;
   sibling_ = nullptr;
-  unresolved_ = nullptr;
+  unresolved_list_.Clear();
 
   start_position_ = kNoSourcePosition;
   end_position_ = kNoSourcePosition;
@@ -834,16 +832,9 @@ Scope* Scope::FinalizeBlockScope() {
   }
 
   // Move unresolved variables
-  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 (!unresolved_list_.is_empty()) {
+    outer_scope()->unresolved_list_.Prepend(std::move(unresolved_list_));
+    unresolved_list_.Clear();
   }
 
   if (inner_scope_calls_eval_) outer_scope()->inner_scope_calls_eval_ = true;
@@ -887,7 +878,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_NULL(new_parent->unresolved_);
+  DCHECK(new_parent->unresolved_list_.is_empty());
   DCHECK(new_parent->locals_.is_empty());
   Scope* inner_scope = new_parent->sibling_;
   if (inner_scope != top_inner_scope_) {
@@ -910,14 +901,21 @@ void Scope::Snapshot::Reparent(DeclarationScope* new_parent) const {
     new_parent->sibling_ = top_inner_scope_;
   }
 
-  if (outer_scope_->unresolved_ != top_unresolved_) {
-    VariableProxy* last = outer_scope_->unresolved_;
-    while (last->next_unresolved() != top_unresolved_) {
-      last = last->next_unresolved();
+  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 != top_unresolved_) {
+        ++iter;
+      }
+      outer_scope_->unresolved_list_.Rewind(iter);
     }
-    last->set_next_unresolved(nullptr);
-    new_parent->unresolved_ = outer_scope_->unresolved_;
-    outer_scope_->unresolved_ = top_unresolved_;
+
+    new_parent->unresolved_list_ = std::move(outer_scope_->unresolved_list_);
+    outer_scope_->unresolved_list_.ReinitializeHead(top_unresolved_);
   }
 
   // TODO(verwaest): This currently only moves do-expression declared variables
@@ -1261,8 +1259,7 @@ void Scope::DeclareCatchVariableName(const AstRawString* name) {
 void Scope::AddUnresolved(VariableProxy* proxy) {
   DCHECK(!already_resolved_);
   DCHECK(!proxy->is_resolved());
-  proxy->set_next_unresolved(unresolved_);
-  unresolved_ = proxy;
+  unresolved_list_.AddFront(proxy);
 }
 
 Variable* DeclarationScope::DeclareDynamicGlobal(const AstRawString* name,
@@ -1274,22 +1271,7 @@ Variable* DeclarationScope::DeclareDynamicGlobal(const AstRawString* name,
 }
 
 bool Scope::RemoveUnresolved(VariableProxy* 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;
+  return unresolved_list_.Remove(var);
 }
 
 Variable* Scope::NewTemporary(const AstRawString* name) {
@@ -1483,11 +1465,12 @@ Scope* Scope::GetOuterScopeWithContext() {
 
 Handle<StringSet> DeclarationScope::CollectNonLocals(
     Isolate* isolate, ParseInfo* info, Handle<StringSet> non_locals) {
-  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());
-  }
+  ResolveScopesThenForEachVariable(this,
+                                   [=, &non_locals](VariableProxy* proxy) {
+                                     non_locals = StringSet::Add(
+                                         isolate, non_locals, proxy->name());
+                                   },
+                                   info);
   return non_locals;
 }
 
@@ -1504,7 +1487,7 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory,
   decls_.Clear();
   locals_.Clear();
   inner_scope_ = nullptr;
-  unresolved_ = nullptr;
+  unresolved_list_.Clear();
   sloppy_block_function_map_ = nullptr;
   rare_data_ = nullptr;
   has_rest_ = false;
@@ -1550,8 +1533,7 @@ void DeclarationScope::SavePreParsedScopeDataForDeclarationScope() {
 
 void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
   DCHECK(!force_eager_compilation_);
-  VariableProxy* unresolved = nullptr;
-
+  ThreadedList<VariableProxy> new_unresolved_list;
   if (!outer_scope_->is_script_scope() ||
       (FLAG_preparser_scope_analysis &&
        preparsed_scope_data_builder_ != nullptr &&
@@ -1559,13 +1541,11 @@ 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.
-    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;
-    }
+    ResolveScopesThenForEachVariable(
+        this, [=, &new_unresolved_list](VariableProxy* proxy) {
+          VariableProxy* copy = ast_node_factory->CopyVariableProxy(proxy);
+          new_unresolved_list.AddFront(copy);
+        });
 
     // Migrate function_ to the right Zone.
     if (function_ != nullptr) {
@@ -1586,7 +1566,7 @@ void DeclarationScope::AnalyzePartially(AstNodeFactory* ast_node_factory) {
 
   ResetAfterPreparsing(ast_node_factory->ast_value_factory(), false);
 
-  unresolved_ = unresolved;
+  unresolved_list_ = std::move(new_unresolved_list);
 }
 
 #ifdef DEBUG
@@ -1673,8 +1653,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 == kDummyPreParserVariable ||
-        var == kDummyPreParserLexicalVariable) {
+    if (var == Scope::kDummyPreParserVariable ||
+        var == Scope::kDummyPreParserLexicalVariable) {
       continue;
     }
     bool local = !IsDynamicVariableMode(var->mode());
@@ -2045,8 +2025,7 @@ bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
   // scopes.
   if (is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()) {
     DCHECK_EQ(variables_.occupancy(), 0);
-    for (VariableProxy* proxy = unresolved_; proxy != nullptr;
-         proxy = proxy->next_unresolved()) {
+    for (VariableProxy* proxy : unresolved_list_) {
       Variable* var = outer_scope()->LookupRecursive(info, proxy, nullptr);
       if (var == nullptr) {
         DCHECK(proxy->is_private_field());
@@ -2060,8 +2039,7 @@ bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
     }
   } else {
     // Resolve unresolved variables for this scope.
-    for (VariableProxy* proxy = unresolved_; proxy != nullptr;
-         proxy = proxy->next_unresolved()) {
+    for (VariableProxy* proxy : unresolved_list_) {
       if (!ResolveVariable(info, proxy)) return false;
     }
 
@@ -2074,57 +2052,6 @@ 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;
@@ -2410,5 +2337,9 @@ 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,8 +217,7 @@ 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);
-    proxy->set_next_unresolved(unresolved_);
-    unresolved_ = proxy;
+    AddUnresolved(proxy);
     return proxy;
   }
 
@@ -479,6 +478,9 @@ 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);
 
@@ -524,7 +526,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.
-  VariableProxy* unresolved_;
+  ThreadedList<VariableProxy> unresolved_list_;
   // Declarations.
   ThreadedList<Declaration> decls_;
 
@@ -596,9 +598,10 @@ 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.
-  VariableProxy* FetchFreeVariables(DeclarationScope* max_outer_scope,
-                                    ParseInfo* info = nullptr,
-                                    VariableProxy* stack = nullptr);
+  template <typename T>
+  void ResolveScopesThenForEachVariable(DeclarationScope* max_outer_scope,
+                                        T variable_proxy_stackvisitor,
+                                        ParseInfo* info = 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(right.variables_);
+        left.variables_->Append(std::move(*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(expression.variables_);
+    variables_->Append(std::move(*expression.variables_));
   }
   ++length_;
 }

src/utils.h

@@ -1646,9 +1646,65 @@ class ThreadedListBase final : public BaseClass {
     tail_ = TLTraits::next(v);
   }
 
-  void Append(ThreadedListBase* list) {
-    *tail_ = list->head_;
-    tail_ = list->tail_;
+  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) {
+    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_;
+    }
+
+    SLOW_DCHECK(Verify());
+  }
+
+  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 Clear() {
@@ -1656,13 +1712,67 @@ 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) { return entry_ != other.entry_; }
+    bool operator==(const Iterator& other) const {
+      return entry_ == other.entry_;
+    }
+    bool operator!=(const Iterator& other) const {
+      return entry_ != other.entry_;
+    }
     T* operator*() { return *entry_; }
     T* operator->() { return *entry_; }
     Iterator& operator=(T* entry) {
@@ -1681,12 +1791,22 @@ 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) {
+    bool operator==(const ConstIterator& other) const {
+      return entry_ == other.entry_;
+    }
+    bool operator!=(const ConstIterator& other) const {
       return entry_ != other.entry_;
     }
     const T* operator*() const { return *entry_; }
@@ -1705,23 +1825,34 @@ 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, including the reset_point.
   void Rewind(Iterator reset_point) {
+    SLOW_DCHECK(Verify());
+
     tail_ = reset_point.entry_;
     *tail_ = nullptr;
   }
 
-  void MoveTail(ThreadedListBase<T, BaseClass>* parent, Iterator location) {
-    if (parent->end() != location) {
+  // 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) {
       DCHECK_NULL(*tail_);
-      *tail_ = *location;
-      tail_ = parent->tail_;
-      parent->Rewind(location);
+      *tail_ = *from_location;
+      tail_ = from_list->tail_;
+      from_list->Rewind(from_location);
+
+      SLOW_DCHECK(Verify());
+      SLOW_DCHECK(from_list->Verify());
     }
   }
 
   bool is_empty() const { return head_ == nullptr; }
 
-  T* first() { return head_; }
+  T* first() const { return head_; }
 
   // Slow. For testing purposes.
   int LengthForTest() {
@@ -1729,12 +1860,26 @@ 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/mjsunit/regress/regress-883059.js

+// 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.
+
+// Flags: --random-seed=-1595876594 --disable-in-process-stack-traces  --no-lazy
+
+var __v_47 = ({[__v_46]: __f_52}) => { var __v_46 = 'b'; return __f_52; };

test/unittests/BUILD.gn

@@ -196,6 +196,7 @@ 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