chore: add IndexedList::splice method and other tweaks (#5670)

This pull request is made in support of #5655 which remains in draft;
the intent is to pull these pieces out of that in order to have a more
focused code review of both pull requests.

We have a container class `IndexedList` which is used to store
non-movable data (and thus not usable in `std::vector`) with indexing
(and thus not usable in `std::list`). The implementation uses both
`std::list` and `std::vector` underneath.

This pull request makes some improvements to this class:
- we add the `splice` method whose semantics are the same as those of
the `std::list` `splice` method. This method transfers elements from one
`IndexedList` to another. In support of this we add new `iterator` and
`const_iterator` types.
- we provide the constructor definition in the header so that
implementations do not have to declare their own specialization.
- we update the `resize` method to accept variadic copyable arguments
which will be used to initialize each of the new elements of the
container.

We also add unit tests for `splice` correctness, and `resize` avoiding
the move constructor.

---
TYPE: NO_HISTORY
DESC: Add `IndexedList::splice`

Author: rroelke

tiledb/common/indexed_list.h

@@ -38,12 +38,13 @@
 #include <list>
 #include <vector>
 
-namespace tiledb::sm {
-class MemoryTracker;
-}
-
 namespace tiledb::common {
 
+namespace detail {
+template <typename T>
+class WhiteboxIndexedList;
+}
+
 /**
  * Container class for data that cannot be moved but that we want to access by
  * an index.
@@ -52,7 +53,136 @@ namespace tiledb::common {
  */
 template <class T>
 class IndexedList {
+  using Self = IndexedList<T>;
+  using Elements = tdb::pmr::list<T>;
+  using Indexes = std::vector<T*>;
+
+  friend class detail::WhiteboxIndexedList<T>;
+
  public:
+  /**
+   * Iterator handle to a mutable element of an `IndexedList`.
+   */
+  class iterator {
+    Elements::iterator elt_;
+    Indexes::iterator idx_;
+
+    friend class IndexedList<T>;
+
+    iterator(Elements::iterator elt, Indexes::iterator idx)
+        : elt_(elt)
+        , idx_(idx) {
+    }
+
+   public:
+    using difference_type = int64_t;
+    using value_type = T;
+
+    T& operator*() const {
+      return *elt_;
+    }
+
+    T* operator->() const {
+      return &*elt_;
+    }
+
+    iterator& operator++() {
+      ++elt_;
+      ++idx_;
+      return *this;
+    }
+
+    iterator& operator++(int) {
+      elt_++;
+      idx_++;
+      return *this;
+    }
+
+    iterator& operator--() {
+      --elt_;
+      --idx_;
+      return *this;
+    }
+
+    iterator& operator--(int) {
+      elt_--;
+      idx_--;
+      return *this;
+    }
+
+    bool operator==(const iterator& other) const {
+      return elt_ == other.elt_;
+    }
+
+    bool operator!=(const iterator& other) const {
+      return !(*this == other);
+    }
+  };
+
+  /**
+   * Iterator handle to an immutable element of an `IndexedList`.
+   */
+  class const_iterator {
+    Elements::const_iterator elt_;
+    Indexes::const_iterator idx_;
+
+    friend class IndexedList<T>;
+
+    const_iterator(Elements::const_iterator elt, Indexes::const_iterator idx)
+        : elt_(elt)
+        , idx_(idx) {
+    }
+
+   public:
+    using difference_type = int64_t;
+    using value_type = T;
+
+    const_iterator(IndexedList<T>::iterator ii)
+        : elt_(ii.elt_)
+        , idx_(ii.idx_) {
+    }
+
+    const T& operator*() const {
+      return *elt_;
+    }
+
+    const T* operator->() const {
+      return &*elt_;
+    }
+
+    const_iterator& operator++() {
+      ++elt_;
+      ++idx_;
+      return *this;
+    }
+
+    const_iterator& operator++(int) {
+      elt_++;
+      idx_++;
+      return *this;
+    }
+
+    const_iterator& operator--() {
+      --elt_;
+      --idx_;
+      return *this;
+    }
+
+    const_iterator& operator--(int) {
+      elt_--;
+      idx_--;
+      return *this;
+    }
+
+    bool operator==(const const_iterator& other) const {
+      return elt_ == other.elt_;
+    }
+
+    bool operator!=(const const_iterator& other) const {
+      return !(*this == other);
+    }
+  };
+
   /* ********************************* */
   /*     CONSTRUCTORS & DESTRUCTORS    */
   /* ********************************* */
@@ -65,7 +195,9 @@ class IndexedList {
    *
    * @param memory_tracker The memory tracker for the underlying containers.
    */
-  explicit IndexedList(shared_ptr<sm::MemoryTracker> memory_tracker);
+  explicit IndexedList(pmr::memory_resource* resource)
+      : list_(resource) {
+  }
 
   DISABLE_COPY_AND_COPY_ASSIGN(IndexedList);
   DISABLE_MOVE_AND_MOVE_ASSIGN(IndexedList);
@@ -92,24 +224,20 @@ class IndexedList {
     return list_.get_allocator();
   }
 
-  /** Returns an iterator to the beginning of the items. */
-  typename std::list<T>::iterator begin() {
-    return list_.begin();
+  iterator begin() {
+    return iterator(list_.begin(), vec_.begin());
   }
 
-  /** Returns an iterator to the beginning of the items. */
-  typename std::list<T>::const_iterator begin() const {
-    return list_.begin();
+  const_iterator begin() const {
+    return const_iterator(list_.begin(), vec_.begin());
   }
 
-  /** Returns an iterator to the end of the items. */
-  typename std::list<T>::iterator end() {
-    return list_.end();
+  iterator end() {
+    return iterator(list_.end(), vec_.end());
   }
 
-  /** Returns an iterator to the end of the items. */
-  typename std::list<T>::const_iterator end() const {
-    return list_.end();
+  const_iterator end() const {
+    return const_iterator(list_.end(), vec_.end());
   }
 
   /** Returns wether the container is empty or not. */
@@ -144,15 +272,21 @@ class IndexedList {
    *
    * @param num Number of items to add.
    */
-  void resize(size_t num) {
+  template <std::copyable... Args>
+  void resize(size_t num, Args... args) {
     if (list_.size() != 0 || vec_.size() != 0) {
       throw std::logic_error(
           "Resize should only be called on empty container.");
     }
 
     vec_.reserve(num);
     for (uint64_t n = 0; n < num; n++) {
-      emplace_back(memory_tracker_);
+      std::tuple<std::decay_t<Args>...> copied_args(args...);
+      std::apply(
+          [this](auto&&... copied_arg) {
+            this->emplace_back<Args...>(std::move(copied_arg)...);
+          },
+          copied_args);
     }
   }
 
@@ -204,15 +338,48 @@ class IndexedList {
     return *(vec_.at(index));
   }
 
- private:
-  /** The memory tracker for the underlying list. */
-  shared_ptr<sm::MemoryTracker> memory_tracker_;
+  /**
+   * @return a pointer to the last element of the list
+   */
+  T* back() {
+    if (empty()) {
+      return nullptr;
+    } else {
+      return vec_.back();
+    }
+  }
 
+  /**
+   * @return a pointer to the last element of the list
+   */
+  const T* back() const {
+    if (empty()) {
+      return nullptr;
+    } else {
+      return vec_.back();
+    }
+  }
+
+  /**
+   * Transfers the elements from `other` between its positions `first` and
+   * `last` to `*this`. The elements are inserted at `pos`.
+   */
+  void splice(
+      const_iterator pos,
+      Self& other,
+      const_iterator first,
+      const_iterator last) {
+    list_.splice(pos.elt_, other.list_, first.elt_, last.elt_);
+    vec_.insert(pos.idx_, first.idx_, last.idx_);
+    other.vec_.erase(first.idx_, last.idx_);
+  }
+
+ private:
   /** List that contains all the elements. */
-  tdb::pmr::list<T> list_;
+  Elements list_;
 
   /** Vector that contains a pointer to the elements allowing indexed access. */
-  std::vector<T*> vec_;
+  Indexes vec_;
 };
 
 }  // namespace tiledb::common

tiledb/common/test/CMakeLists.txt

@@ -34,6 +34,12 @@ commence(unit_test arithmetic)
     endif ()
 conclude(unit_test)
 
+commence(unit_test indexed_list)
+    this_target_sources(main.cc unit_indexed_list.cc ${CMAKE_SOURCE_DIR}/test/support/src/mem_helpers.cc)
+    this_target_object_libraries(baseline)
+    this_target_link_libraries(rapidcheck)
+conclude(unit_test)
+
 commence(unit_test memory_tracker)
     this_target_sources(main.cc unit_memory_tracker.cc unit_memory_tracker_types.cc)
     this_target_object_libraries(baseline)