[zone] Cleanup ZoneList and ScopedList classes

Also make ScopedList class Zone-agnostic and move it to src/utils.

Bug: v8:10506
Change-Id: Ibf0869566caa767809bdf95cb03c01e599613938
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2292234Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Commit-Queue: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68825}

BUILD.gn

@@ -3165,6 +3165,7 @@ v8_source_set("v8_base_without_compiler") {
     "src/utils/ostreams.cc",
     "src/utils/ostreams.h",
     "src/utils/pointer-with-payload.h",
+    "src/utils/scoped-list.h",
     "src/utils/utils-inl.h",
     "src/utils/utils.cc",
     "src/utils/utils.h",
@@ -3248,7 +3249,6 @@ v8_source_set("v8_base_without_compiler") {
     "src/wasm/wasm-value.h",
     "src/zone/accounting-allocator.cc",
     "src/zone/accounting-allocator.h",
-    "src/zone/scoped-list.h",
     "src/zone/zone-allocator.h",
     "src/zone/zone-chunk-list.h",
     "src/zone/zone-containers.h",
@@ -3259,6 +3259,7 @@ v8_source_set("v8_base_without_compiler") {
     "src/zone/zone-list.h",
     "src/zone/zone-segment.cc",
     "src/zone/zone-segment.h",
+    "src/zone/zone-utils.h",
     "src/zone/zone.cc",
     "src/zone/zone.h",
   ]

src/ast/ast.cc

@@ -950,9 +950,7 @@ Call::CallType Call::GetCallType() const {
 
 CaseClause::CaseClause(Zone* zone, Expression* label,
                        const ScopedPtrList<Statement>& statements)
-    : label_(label), statements_(0, nullptr) {
-  statements.CopyTo(&statements_, zone);
-}
+    : label_(label), statements_(statements.ToConstVector(), zone) {}
 
 bool Literal::IsPropertyName() const {
   if (type() != kString) return false;

src/ast/ast.h

@@ -320,7 +320,7 @@ class Block final : public BreakableStatement {
   void InitializeStatements(const ScopedPtrList<Statement>& statements,
                             Zone* zone) {
     DCHECK_EQ(0, statements_.length());
-    statements.CopyTo(&statements_, zone);
+    statements_ = ZonePtrList<Statement>(statements.ToConstVector(), zone);
   }
 
  private:
@@ -1324,12 +1324,11 @@ class ObjectLiteral final : public AggregateLiteral {
                 bool has_rest_property)
       : AggregateLiteral(pos, kObjectLiteral),
         boilerplate_properties_(boilerplate_properties),
-        properties_(0, nullptr) {
+        properties_(properties.ToConstVector(), zone) {
     bit_field_ |= HasElementsField::encode(false) |
                   HasRestPropertyField::encode(has_rest_property) |
                   FastElementsField::encode(false) |
                   HasNullPrototypeField::encode(false);
-    properties.CopyTo(&properties_, zone);
   }
 
   void InitFlagsForPendingNullPrototype(int i);
@@ -1398,9 +1397,7 @@ class ArrayLiteral final : public AggregateLiteral {
                int first_spread_index, int pos)
       : AggregateLiteral(pos, kArrayLiteral),
         first_spread_index_(first_spread_index),
-        values_(0, nullptr) {
-    values.CopyTo(&values_, zone);
-  }
+        values_(values.ToConstVector(), zone) {}
 
   int first_spread_index_;
   Handle<ArrayBoilerplateDescription> boilerplate_description_;
@@ -1683,12 +1680,11 @@ class Call final : public Expression {
        PossiblyEval possibly_eval, bool optional_chain)
       : Expression(pos, kCall),
         expression_(expression),
-        arguments_(0, nullptr) {
+        arguments_(arguments.ToConstVector(), zone) {
     bit_field_ |=
         IsPossiblyEvalField::encode(possibly_eval == IS_POSSIBLY_EVAL) |
         IsTaggedTemplateField::encode(false) |
         IsOptionalChainLinkField::encode(optional_chain);
-    arguments.CopyTo(&arguments_, zone);
   }
 
   Call(Zone* zone, Expression* expression,
@@ -1696,11 +1692,10 @@ class Call final : public Expression {
        TaggedTemplateTag tag)
       : Expression(pos, kCall),
         expression_(expression),
-        arguments_(0, nullptr) {
+        arguments_(arguments.ToConstVector(), zone) {
     bit_field_ |= IsPossiblyEvalField::encode(false) |
                   IsTaggedTemplateField::encode(true) |
                   IsOptionalChainLinkField::encode(false);
-    arguments.CopyTo(&arguments_, zone);
   }
 
   using IsPossiblyEvalField = Expression::NextBitField<bool, 1>;
@@ -1729,9 +1724,7 @@ class CallNew final : public Expression {
           const ScopedPtrList<Expression>& arguments, int pos)
       : Expression(pos, kCallNew),
         expression_(expression),
-        arguments_(0, nullptr) {
-    arguments.CopyTo(&arguments_, zone);
-  }
+        arguments_(arguments.ToConstVector(), zone) {}
 
   Expression* expression_;
   ZonePtrList<Expression> arguments_;
@@ -1765,17 +1758,13 @@ class CallRuntime final : public Expression {
               const ScopedPtrList<Expression>& arguments, int pos)
       : Expression(pos, kCallRuntime),
         function_(function),
-        arguments_(0, nullptr) {
-    arguments.CopyTo(&arguments_, zone);
-  }
+        arguments_(arguments.ToConstVector(), zone) {}
   CallRuntime(Zone* zone, int context_index,
               const ScopedPtrList<Expression>& arguments, int pos)
       : Expression(pos, kCallRuntime),
         context_index_(context_index),
         function_(nullptr),
-        arguments_(0, nullptr) {
-    arguments.CopyTo(&arguments_, zone);
-  }
+        arguments_(arguments.ToConstVector(), zone) {}
 
   int context_index_;
   const Runtime::Function* function_;
@@ -2307,7 +2296,7 @@ class FunctionLiteral final : public Expression {
         function_literal_id_(function_literal_id),
         raw_name_(name),
         scope_(scope),
-        body_(0, nullptr),
+        body_(body.ToConstVector(), zone),
         raw_inferred_name_(ast_value_factory->empty_cons_string()),
         produced_preparse_data_(produced_preparse_data) {
     bit_field_ |= FunctionSyntaxKindBits::encode(function_syntax_kind) |
@@ -2319,7 +2308,6 @@ class FunctionLiteral final : public Expression {
                   HasBracesField::encode(has_braces) |
                   OneshotIIFEBit::encode(false);
     if (eager_compile_hint == kShouldEagerCompile) SetShouldEagerCompile();
-    body.CopyTo(&body_, zone);
   }
 
   using FunctionSyntaxKindBits =

src/interpreter/bytecode-generator.cc

@@ -2913,8 +2913,8 @@ void BytecodeGenerator::BuildCreateArrayLiteral(
   Register array = register_allocator()->NewRegister();
   SharedFeedbackSlot element_slot(feedback_spec(),
                                   FeedbackSlotKind::kStoreInArrayLiteral);
-  ZonePtrList<Expression>::iterator current = elements->begin();
-  ZonePtrList<Expression>::iterator end = elements->end();
+  ZonePtrList<Expression>::const_iterator current = elements->begin();
+  ZonePtrList<Expression>::const_iterator end = elements->end();
   bool is_empty = elements->is_empty();
 
   if (!is_empty && (*current)->IsSpread()) {
@@ -2976,7 +2976,7 @@ void BytecodeGenerator::BuildCreateArrayLiteral(
     // index, into the initial array (the remaining elements will be inserted
     // below).
     DCHECK_EQ(current, elements->begin());
-    ZonePtrList<Expression>::iterator first_spread_or_end =
+    ZonePtrList<Expression>::const_iterator first_spread_or_end =
         expr->first_spread_index() >= 0 ? current + expr->first_spread_index()
                                         : end;
     int array_index = 0;

src/parsing/parser.cc

@@ -3191,7 +3191,7 @@ Expression* Parser::CloseTemplateLiteral(TemplateLiteralState* state, int start,
     // Call TagFn
     ScopedPtrList<Expression> call_args(pointer_buffer());
     call_args.Add(template_object);
-    call_args.AddAll(*expressions);
+    call_args.AddAll(expressions->ToConstVector());
     return factory()->NewTaggedTemplate(tag, call_args, pos);
   }
 }

src/parsing/preparse-data.cc

@@ -17,6 +17,7 @@
 #include "src/parsing/preparser.h"
 #include "src/roots/roots.h"
 #include "src/zone/zone-list-inl.h"  // crbug.com/v8/8816
+#include "src/zone/zone-utils.h"
 
 namespace v8 {
 namespace internal {
@@ -254,7 +255,8 @@ void PreparseDataBuilder::AddChild(PreparseDataBuilder* child) {
 
 void PreparseDataBuilder::FinalizeChildren(Zone* zone) {
   DCHECK(!finalized_children_);
-  Vector<PreparseDataBuilder*> children = children_buffer_.CopyTo(zone);
+  Vector<PreparseDataBuilder*> children =
+      CloneVector(zone, children_buffer_.ToConstVector());
   children_buffer_.Rewind();
   children_ = children;
 #ifdef DEBUG

src/parsing/preparse-data.h

@@ -10,8 +10,8 @@
 #include "src/common/globals.h"
 #include "src/handles/handles.h"
 #include "src/handles/maybe-handles.h"
+#include "src/utils/scoped-list.h"
 #include "src/utils/vector.h"
-#include "src/zone/scoped-list.h"
 #include "src/zone/zone-chunk-list.h"
 #include "src/zone/zone-containers.h"
 

src/zone/scoped-list.h → src/utils/scoped-list.h

@@ -2,14 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef V8_ZONE_SCOPED_LIST_H_
-#define V8_ZONE_SCOPED_LIST_H_
+#ifndef V8_UTILS_SCOPED_LIST_H_
+#define V8_UTILS_SCOPED_LIST_H_
 
 #include <type_traits>
 #include <vector>
 
 #include "src/base/logging.h"
-#include "src/zone/zone.h"
 
 namespace v8 {
 namespace internal {
@@ -24,15 +23,14 @@ class ZoneList;
 // re-used between ScopedLists. Note that a ScopedList in an outer scope cannot
 // add any entries if there is a ScopedList with the same backing in an inner
 // scope.
-//
-// TODO(ishell): move header to src/utils/ once zone dependency is resolved.
 template <typename T, typename TBacking = T>
 class ScopedList final {
   // The backing can either be the same type as the list type, or, for pointers,
   // we additionally allow a void* backing store.
-  STATIC_ASSERT((std::is_same<TBacking, T>::value) ||
+  static_assert((std::is_same<TBacking, T>::value) ||
                     (std::is_same<TBacking, void*>::value &&
-                 std::is_pointer<T>::value));
+                     std::is_pointer<T>::value),
+                "Incompatible combination of T and TBacking types");
 
  public:
   explicit ScopedList(std::vector<TBacking>* buffer)
@@ -69,22 +67,9 @@ class ScopedList final {
     return *reinterpret_cast<T*>(&buffer_[index]);
   }
 
-  void CopyTo(ZoneList<T>* target, Zone* zone) const {
-    DCHECK_LE(end_, buffer_.size());
-    // Make sure we don't reference absent elements below.
-    if (length() == 0) return;
-    target->Initialize(length(), zone);
-    T* data = reinterpret_cast<T*>(&buffer_[start_]);
-    target->AddAll(Vector<T>(data, length()), zone);
-  }
-
-  Vector<T> CopyTo(Zone* zone) {
-    DCHECK_LE(end_, buffer_.size());
-    T* data = zone->NewArray<T>(length());
-    if (length() != 0) {
-      MemCopy(data, &buffer_[start_], length() * sizeof(T));
-    }
-    return Vector<T>(data, length());
+  Vector<const T> ToConstVector() const {
+    T* data = reinterpret_cast<T*>(buffer_.data() + start_);
+    return Vector<const T>(data, length());
   }
 
   void Add(const T& value) {
@@ -93,7 +78,7 @@ class ScopedList final {
     ++end_;
   }
 
-  void AddAll(const ZoneList<T>& list) {
+  void AddAll(const Vector<const T>& list) {
     DCHECK_EQ(buffer_.size(), end_);
     buffer_.reserve(buffer_.size() + list.length());
     for (int i = 0; i < list.length(); i++) {
@@ -103,10 +88,17 @@ class ScopedList final {
   }
 
   using iterator = T*;
-  inline iterator begin() const {
+  using const_iterator = const T*;
+
+  inline iterator begin() {
     return reinterpret_cast<T*>(buffer_.data() + start_);
   }
-  inline iterator end() const {
+  inline const_iterator begin() const {
+    return reinterpret_cast<T*>(buffer_.data() + start_);
+  }
+
+  inline iterator end() { return reinterpret_cast<T*>(buffer_.data() + end_); }
+  inline const_iterator end() const {
     return reinterpret_cast<T*>(buffer_.data() + end_);
   }
 
@@ -122,4 +114,4 @@ using ScopedPtrList = ScopedList<T*, void*>;
 }  // namespace internal
 }  // namespace v8
 
-#endif  // V8_ZONE_SCOPED_LIST_H_
+#endif  // V8_UTILS_SCOPED_LIST_H_

src/zone/zone-list-inl.h

@@ -28,7 +28,7 @@ void ZoneList<T>::AddAll(const ZoneList<T>& other, Zone* zone) {
 }
 
 template <typename T>
-void ZoneList<T>::AddAll(const Vector<T>& other, Zone* zone) {
+void ZoneList<T>::AddAll(const Vector<const T>& other, Zone* zone) {
   int result_length = length_ + other.length();
   if (capacity_ < result_length) Resize(result_length, zone);
   if (std::is_fundamental<T>()) {

src/zone/zone-list.h

@@ -5,8 +5,6 @@
 #ifndef V8_ZONE_ZONE_LIST_H_
 #define V8_ZONE_ZONE_LIST_H_
 
-#include <initializer_list>
-
 #include "src/base/logging.h"
 #include "src/zone/zone-fwd.h"
 #include "src/zone/zone.h"
@@ -32,18 +30,43 @@ class ZoneList final : public ZoneObject {
  public:
   // Construct a new ZoneList with the given capacity; the length is
   // always zero. The capacity must be non-negative.
-  ZoneList(int capacity, Zone* zone) { Initialize(capacity, zone); }
+  ZoneList(int capacity, Zone* zone) : capacity_(capacity) {
+    DCHECK_GE(capacity, 0);
+    data_ = (capacity_ > 0) ? zone->NewArray<T>(capacity_) : nullptr;
+  }
+
   // Construct a new ZoneList by copying the elements of the given ZoneList.
-  ZoneList(const ZoneList<T>& other, Zone* zone) {
-    Initialize(other.length(), zone);
+  ZoneList(const ZoneList<T>& other, Zone* zone)
+      : ZoneList(other.length(), zone) {
     AddAll(other, zone);
   }
 
+  // Construct a new ZoneList by copying the elements of the given vector.
+  ZoneList(const Vector<const T>& other, Zone* zone)
+      : ZoneList(other.length(), zone) {
+    AddAll(other, zone);
+  }
+
+  ZoneList(ZoneList<T>&& other) V8_NOEXCEPT { *this = std::move(other); }
+
   // The ZoneList objects are usually allocated as a fields in other
   // zone-allocated objects for which destructors are not called anyway, so
   // we are not going to clear the memory here as well.
   ~ZoneList() = default;
 
+  ZoneList& operator=(ZoneList&& other) V8_NOEXCEPT {
+    // We don't have a Zone object, so we'll have to drop the data_ array.
+    // If this assert ever fails, consider calling Clear(Zone*) or
+    // DropAndClear() before the move assignment to make it explicit what's
+    // happenning with the lvalue.
+    DCHECK_NULL(data_);
+    data_ = other.data_;
+    capacity_ = other.capacity_;
+    length_ = other.length_;
+    other.DropAndClear();
+    return *this;
+  }
+
   // Returns a reference to the element at index i. This reference is not safe
   // to use after operations that can change the list's backing store
   // (e.g. Add).
@@ -57,8 +80,12 @@ class ZoneList final : public ZoneObject {
   inline T& first() const { return at(0); }
 
   using iterator = T*;
-  inline iterator begin() const { return &data_[0]; }
-  inline iterator end() const { return &data_[length_]; }
+  inline iterator begin() { return &data_[0]; }
+  inline iterator end() { return &data_[length_]; }
+
+  using const_iterator = const T*;
+  inline const_iterator begin() const { return &data_[0]; }
+  inline const_iterator end() const { return &data_[length_]; }
 
   V8_INLINE bool is_empty() const { return length_ == 0; }
   V8_INLINE int length() const { return length_; }
@@ -74,23 +101,13 @@ class ZoneList final : public ZoneObject {
     return Vector<const T>(data_, length_);
   }
 
-  // TODO(v8:10572): remove this method in favor of more flexible constructor,
-  // allowing initialization from provided iterators.
-  V8_INLINE void Initialize(int capacity, Zone* zone) {
-    DCHECK_GE(capacity, 0);
-    DCHECK_NULL(data_);
-    capacity_ = capacity;
-    data_ = (capacity_ > 0) ? zone->NewArray<T>(capacity_) : nullptr;
-    length_ = 0;
-  }
-
   // Adds a copy of the given 'element' to the end of the list,
   // expanding the list if necessary.
   void Add(const T& element, Zone* zone);
   // Add all the elements from the argument list to this list.
   void AddAll(const ZoneList<T>& other, Zone* zone);
   // Add all the elements from the vector to this list.
-  void AddAll(const Vector<T>& other, Zone* zone);
+  void AddAll(const Vector<const T>& other, Zone* zone);
   // Inserts the element at the specific index.
   void InsertAt(int index, const T& element, Zone* zone);
 

src/zone/zone-utils.h

+// Copyright 2020 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_ZONE_ZONE_UTILS_H_
+#define V8_ZONE_ZONE_UTILS_H_
+
+#include <algorithm>
+#include <type_traits>
+
+#include "src/utils/vector.h"
+#include "src/zone/zone.h"
+
+namespace v8 {
+namespace internal {
+
+template <typename T>
+Vector<T> CloneVector(Zone* zone, const Vector<const T>& other) {
+  int length = other.length();
+  if (length == 0) return Vector<T>();
+
+  T* data = zone->NewArray<T>(length);
+  if (std::is_fundamental<T>()) {
+    MemCopy(data, other.data(), length * sizeof(T));
+  } else {
+    std::copy(other.begin(), other.end(), data);
+  }
+  return Vector<T>(data, length);
+}
+
+}  // namespace internal
+}  // namespace v8
+
+#endif  // V8_ZONE_ZONE_UTILS_H_