[lldb][NFC] remove AdaptedConstIterator and AdaptedIterable #127507

daniilavdeev · 2025-02-17T15:31:42Z

AdaptedConstIterator currently doesn't have iterator traits, so I can't use STL algorithms with containers like WatchpointList.

This patch replaces AdaptedConstIterator and AdaptedIterable with llvm::iterator_adaped_base and llvm::iterator_range respectively.

llvmbot · 2025-02-17T15:32:18Z

@llvm/pr-subscribers-lldb

Author: None (dlav-sc)

Changes

AdaptedConstIterator currently doesn't have iterator traits, so I can't use STL algorithms with containers like WatchpointList.

This patch replaces AdaptedConstIterator and AdaptedIterable with llvm::iterator_adaped_base and llvm::iterator_range respectively.

Full diff: https://github.com/llvm/llvm-project/pull/127507.diff

15 Files Affected:

(modified) lldb/include/lldb/Breakpoint/BreakpointList.h (+1-2)
(modified) lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h (+1-2)
(modified) lldb/include/lldb/Breakpoint/BreakpointLocationList.h (+1-2)
(modified) lldb/include/lldb/Breakpoint/WatchpointList.h (+1-2)
(modified) lldb/include/lldb/Breakpoint/WatchpointResource.h (+1-2)
(modified) lldb/include/lldb/Core/ModuleList.h (+2-3)
(modified) lldb/include/lldb/Core/ModuleSpec.h (+1-2)
(modified) lldb/include/lldb/Host/common/NativeProcessProtocol.h (+3-7)
(modified) lldb/include/lldb/Symbol/SymbolContext.h (+1-1)
(modified) lldb/include/lldb/Symbol/TypeList.h (+1-2)
(modified) lldb/include/lldb/Symbol/TypeMap.h (+2-1)
(modified) lldb/include/lldb/Target/QueueList.h (+1-3)
(modified) lldb/include/lldb/Target/TargetList.h (+1-2)
(modified) lldb/include/lldb/Target/ThreadCollection.h (+1-2)
(modified) lldb/include/lldb/Utility/Iterable.h (+20-155)

diff --git a/lldb/include/lldb/Breakpoint/BreakpointList.h b/lldb/include/lldb/Breakpoint/BreakpointList.h
index a7399d385f6f0..4a921fadfc066 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointList.h
@@ -163,8 +163,7 @@ class BreakpointList {
   bool m_is_internal;
 
 public:
-  typedef LockingAdaptedIterable<bp_collection, lldb::BreakpointSP,
-                                 list_adapter, std::recursive_mutex>
+  typedef LockingAdaptedIterable<std::recursive_mutex, bp_collection>
       BreakpointIterable;
   BreakpointIterable Breakpoints() {
     return BreakpointIterable(m_breakpoints, GetMutex());
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
index 34bd309864871..3aef1d658c0e5 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocationCollection.h
@@ -165,8 +165,7 @@ class BreakpointLocationCollection {
   mutable std::mutex m_collection_mutex;
 
 public:
-  typedef AdaptedIterable<collection, lldb::BreakpointLocationSP,
-                          vector_adapter>
+  typedef llvm::iterator_range<collection::const_iterator>
       BreakpointLocationCollectionIterable;
   BreakpointLocationCollectionIterable BreakpointLocations() {
     return BreakpointLocationCollectionIterable(m_break_loc_collection);
diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
index f76a8fcfdd7e7..17dc0bfe03148 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocationList.h
@@ -204,8 +204,7 @@ class BreakpointLocationList {
   BreakpointLocationCollection *m_new_location_recorder;
 
 public:
-  typedef AdaptedIterable<collection, lldb::BreakpointLocationSP,
-                          vector_adapter>
+  typedef llvm::iterator_range<collection::const_iterator>
       BreakpointLocationIterable;
 
   BreakpointLocationIterable BreakpointLocations() {
diff --git a/lldb/include/lldb/Breakpoint/WatchpointList.h b/lldb/include/lldb/Breakpoint/WatchpointList.h
index bf87495d79dba..d037d36e64290 100644
--- a/lldb/include/lldb/Breakpoint/WatchpointList.h
+++ b/lldb/include/lldb/Breakpoint/WatchpointList.h
@@ -39,8 +39,7 @@ class WatchpointList {
   ~WatchpointList();
 
   typedef std::list<lldb::WatchpointSP> wp_collection;
-  typedef LockingAdaptedIterable<wp_collection, lldb::WatchpointSP,
-                                 vector_adapter, std::recursive_mutex>
+  typedef LockingAdaptedIterable<std::recursive_mutex, wp_collection>
       WatchpointIterable;
 
   /// Add a Watchpoint to the list.
diff --git a/lldb/include/lldb/Breakpoint/WatchpointResource.h b/lldb/include/lldb/Breakpoint/WatchpointResource.h
index 070d84cff8f26..c1a81fc486eb6 100644
--- a/lldb/include/lldb/Breakpoint/WatchpointResource.h
+++ b/lldb/include/lldb/Breakpoint/WatchpointResource.h
@@ -39,8 +39,7 @@ class WatchpointResource
   void SetType(bool read, bool write);
 
   typedef std::vector<lldb::WatchpointSP> WatchpointCollection;
-  typedef LockingAdaptedIterable<WatchpointCollection, lldb::WatchpointSP,
-                                 vector_adapter, std::mutex>
+  typedef LockingAdaptedIterable<std::mutex, WatchpointCollection>
       WatchpointIterable;
 
   /// Iterate over the watchpoint constituents for this resource
diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h
index 43d931a844740..29b87de88520d 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -521,14 +521,13 @@ class ModuleList {
   Notifier *m_notifier = nullptr;
 
 public:
-  typedef LockingAdaptedIterable<collection, lldb::ModuleSP, vector_adapter,
-                                 std::recursive_mutex>
+  typedef LockingAdaptedIterable<std::recursive_mutex, collection>
       ModuleIterable;
   ModuleIterable Modules() const {
     return ModuleIterable(m_modules, GetMutex());
   }
 
-  typedef AdaptedIterable<collection, lldb::ModuleSP, vector_adapter>
+  typedef llvm::iterator_range<collection::const_iterator>
       ModuleIterableNoLocking;
   ModuleIterableNoLocking ModulesNoLocking() const {
     return ModuleIterableNoLocking(m_modules);
diff --git a/lldb/include/lldb/Core/ModuleSpec.h b/lldb/include/lldb/Core/ModuleSpec.h
index 4cbbbfa8a26e1..86be0383f8b47 100644
--- a/lldb/include/lldb/Core/ModuleSpec.h
+++ b/lldb/include/lldb/Core/ModuleSpec.h
@@ -389,8 +389,7 @@ class ModuleSpecList {
   }
 
   typedef std::vector<ModuleSpec> collection;
-  typedef LockingAdaptedIterable<collection, ModuleSpec, vector_adapter,
-                                 std::recursive_mutex>
+  typedef LockingAdaptedIterable<std::recursive_mutex, collection>
       ModuleSpecIterable;
 
   ModuleSpecIterable ModuleSpecs() {
diff --git a/lldb/include/lldb/Host/common/NativeProcessProtocol.h b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
index 744699210d4b5..1d5fecfcd5c27 100644
--- a/lldb/include/lldb/Host/common/NativeProcessProtocol.h
+++ b/lldb/include/lldb/Host/common/NativeProcessProtocol.h
@@ -51,13 +51,9 @@ class NativeProcessProtocol {
   virtual ~NativeProcessProtocol() = default;
 
   typedef std::vector<std::unique_ptr<NativeThreadProtocol>> thread_collection;
-  template <typename I>
-  static NativeThreadProtocol &thread_list_adapter(I &iter) {
-    assert(*iter);
-    return **iter;
-  }
-  typedef LockingAdaptedIterable<thread_collection, NativeThreadProtocol &,
-                                 thread_list_adapter, std::recursive_mutex>
+  typedef LockingAdaptedIterable<
+      std::recursive_mutex, thread_collection,
+      llvm::pointee_iterator<thread_collection::const_iterator>>
       ThreadIterable;
 
   virtual Status Resume(const ResumeActionList &resume_actions) = 0;
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h b/lldb/include/lldb/Symbol/SymbolContext.h
index 69fbe544c73cd..8b6317c6f33c2 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -467,7 +467,7 @@ class SymbolContextList {
   const_iterator begin() const { return m_symbol_contexts.begin(); }
   const_iterator end() const { return m_symbol_contexts.end(); }
 
-  typedef AdaptedIterable<collection, SymbolContext, vector_adapter>
+  typedef llvm::iterator_range<collection::const_iterator>
       SymbolContextIterable;
   SymbolContextIterable SymbolContexts() {
     return SymbolContextIterable(m_symbol_contexts);
diff --git a/lldb/include/lldb/Symbol/TypeList.h b/lldb/include/lldb/Symbol/TypeList.h
index d58772ad5b62e..6a38babd942ab 100644
--- a/lldb/include/lldb/Symbol/TypeList.h
+++ b/lldb/include/lldb/Symbol/TypeList.h
@@ -39,8 +39,7 @@ class TypeList {
   lldb::TypeSP GetTypeAtIndex(uint32_t idx);
 
   typedef std::vector<lldb::TypeSP> collection;
-  typedef AdaptedIterable<collection, lldb::TypeSP, vector_adapter>
-      TypeIterable;
+  typedef llvm::iterator_range<collection::const_iterator> TypeIterable;
 
   TypeIterable Types() { return TypeIterable(m_types); }
 
diff --git a/lldb/include/lldb/Symbol/TypeMap.h b/lldb/include/lldb/Symbol/TypeMap.h
index 89011efab5c31..6c36ff9369fa5 100644
--- a/lldb/include/lldb/Symbol/TypeMap.h
+++ b/lldb/include/lldb/Symbol/TypeMap.h
@@ -44,7 +44,8 @@ class TypeMap {
   lldb::TypeSP FirstType() const;
 
   typedef std::multimap<lldb::user_id_t, lldb::TypeSP> collection;
-  typedef AdaptedIterable<collection, lldb::TypeSP, map_adapter> TypeIterable;
+  typedef llvm::iterator_range<ValueMapIterator<collection::const_iterator>>
+      TypeIterable;
 
   TypeIterable Types() const { return TypeIterable(m_types); }
 
diff --git a/lldb/include/lldb/Target/QueueList.h b/lldb/include/lldb/Target/QueueList.h
index 7c74a6a99ac18..3f177c90d3989 100644
--- a/lldb/include/lldb/Target/QueueList.h
+++ b/lldb/include/lldb/Target/QueueList.h
@@ -48,9 +48,7 @@ class QueueList {
   lldb::QueueSP GetQueueAtIndex(uint32_t idx);
 
   typedef std::vector<lldb::QueueSP> collection;
-  typedef LockingAdaptedIterable<collection, lldb::QueueSP, vector_adapter,
-                                 std::mutex>
-      QueueIterable;
+  typedef LockingAdaptedIterable<std::mutex, collection> QueueIterable;
 
   /// Iterate over the list of queues
   ///
diff --git a/lldb/include/lldb/Target/TargetList.h b/lldb/include/lldb/Target/TargetList.h
index a0cddc6b2966f..080a6039c7ff8 100644
--- a/lldb/include/lldb/Target/TargetList.h
+++ b/lldb/include/lldb/Target/TargetList.h
@@ -44,8 +44,7 @@ class TargetList : public Broadcaster {
   }
 
   typedef std::vector<lldb::TargetSP> collection;
-  typedef LockingAdaptedIterable<collection, lldb::TargetSP, vector_adapter,
-                                 std::recursive_mutex>
+  typedef LockingAdaptedIterable<std::recursive_mutex, collection>
       TargetIterable;
 
   /// Create a new Target.
diff --git a/lldb/include/lldb/Target/ThreadCollection.h b/lldb/include/lldb/Target/ThreadCollection.h
index 29f5103e7eec7..3fe62787649f4 100644
--- a/lldb/include/lldb/Target/ThreadCollection.h
+++ b/lldb/include/lldb/Target/ThreadCollection.h
@@ -20,8 +20,7 @@ namespace lldb_private {
 class ThreadCollection {
 public:
   typedef std::vector<lldb::ThreadSP> collection;
-  typedef LockingAdaptedIterable<collection, lldb::ThreadSP, vector_adapter,
-                                 std::recursive_mutex>
+  typedef LockingAdaptedIterable<std::recursive_mutex, collection>
       ThreadIterable;
 
   ThreadCollection();
diff --git a/lldb/include/lldb/Utility/Iterable.h b/lldb/include/lldb/Utility/Iterable.h
index 5c38e46feb925..db1f0e65ef6f1 100644
--- a/lldb/include/lldb/Utility/Iterable.h
+++ b/lldb/include/lldb/Utility/Iterable.h
@@ -11,172 +11,37 @@
 
 #include <utility>
 
+#include <llvm/ADT/iterator.h>
 
 namespace lldb_private {
 
-template <typename I, typename E> E map_adapter(I &iter) {
-  return iter->second;
-}
-
-template <typename I, typename E> E vector_adapter(I &iter) { return *iter; }
-
-template <typename I, typename E> E list_adapter(I &iter) { return *iter; }
-
-template <typename C, typename E, E (*A)(typename C::const_iterator &)>
-class AdaptedConstIterator {
-public:
-  typedef typename C::const_iterator BackingIterator;
-
-  // Wrapping constructor
-  AdaptedConstIterator(BackingIterator backing_iterator)
-      : m_iter(backing_iterator) {}
-
-  // Default-constructible
-  AdaptedConstIterator() : m_iter() {}
-
-  // Copy-constructible
-  AdaptedConstIterator(const AdaptedConstIterator &rhs) : m_iter(rhs.m_iter) {}
-
-  // Copy-assignable
-  AdaptedConstIterator &operator=(const AdaptedConstIterator &rhs) {
-    m_iter = rhs.m_iter;
-    return *this;
-  }
-
-  // Destructible
-  ~AdaptedConstIterator() = default;
-
-  // Comparable
-  bool operator==(const AdaptedConstIterator &rhs) {
-    return m_iter == rhs.m_iter;
-  }
-
-  bool operator!=(const AdaptedConstIterator &rhs) {
-    return m_iter != rhs.m_iter;
-  }
-
-  // Rvalue dereferenceable
-  E operator*() { return (*A)(m_iter); }
-
-  E operator->() { return (*A)(m_iter); }
-
-  // Offset dereferenceable
-  E operator[](typename BackingIterator::difference_type offset) {
-    return AdaptedConstIterator(m_iter + offset);
-  }
-
-  // Incrementable
-  AdaptedConstIterator &operator++() {
-    m_iter++;
-    return *this;
-  }
-
-  // Decrementable
-  AdaptedConstIterator &operator--() {
-    m_iter--;
-    return *this;
-  }
-
-  // Compound assignment
-  AdaptedConstIterator &
-  operator+=(typename BackingIterator::difference_type offset) {
-    m_iter += offset;
-    return *this;
-  }
-
-  AdaptedConstIterator &
-  operator-=(typename BackingIterator::difference_type offset) {
-    m_iter -= offset;
-    return *this;
-  }
-
-  // Arithmetic
-  AdaptedConstIterator
-  operator+(typename BackingIterator::difference_type offset) {
-    return AdaptedConstIterator(m_iter + offset);
-  }
-
-  AdaptedConstIterator
-  operator-(typename BackingIterator::difference_type offset) {
-    return AdaptedConstIterator(m_iter - offset);
-  }
-
-  // Comparable
-  bool operator<(AdaptedConstIterator &rhs) { return m_iter < rhs.m_iter; }
-
-  bool operator<=(AdaptedConstIterator &rhs) { return m_iter <= rhs.m_iter; }
-
-  bool operator>(AdaptedConstIterator &rhs) { return m_iter > rhs.m_iter; }
-
-  bool operator>=(AdaptedConstIterator &rhs) { return m_iter >= rhs.m_iter; }
-
-  template <typename C1, typename E1, E1 (*A1)(typename C1::const_iterator &)>
-  friend AdaptedConstIterator<C1, E1, A1>
-  operator+(typename C1::const_iterator::difference_type,
-            AdaptedConstIterator<C1, E1, A1> &);
-
-  template <typename C1, typename E1, E1 (*A1)(typename C1::const_iterator &)>
-  friend typename C1::const_iterator::difference_type
-  operator-(AdaptedConstIterator<C1, E1, A1> &,
-            AdaptedConstIterator<C1, E1, A1> &);
-
-  template <typename C1, typename E1, E1 (*A1)(typename C1::const_iterator &)>
-  friend void swap(AdaptedConstIterator<C1, E1, A1> &,
-                   AdaptedConstIterator<C1, E1, A1> &);
-
-private:
-  BackingIterator m_iter;
-};
-
-template <typename C, typename E, E (*A)(typename C::const_iterator &)>
-AdaptedConstIterator<C, E, A> operator+(
-    typename AdaptedConstIterator<C, E, A>::BackingIterator::difference_type
-        offset,
-    AdaptedConstIterator<C, E, A> &rhs) {
-  return rhs.operator+(offset);
-}
-
-template <typename C, typename E, E (*A)(typename C::const_iterator &)>
-typename AdaptedConstIterator<C, E, A>::BackingIterator::difference_type
-operator-(AdaptedConstIterator<C, E, A> &lhs,
-          AdaptedConstIterator<C, E, A> &rhs) {
-  return (lhs.m_iter - rhs.m_iter);
-}
-
-template <typename C, typename E, E (*A)(typename C::const_iterator &)>
-void swap(AdaptedConstIterator<C, E, A> &lhs,
-          AdaptedConstIterator<C, E, A> &rhs) {
-  std::swap(lhs.m_iter, rhs.m_iter);
-}
-
-template <typename C, typename E, E (*A)(typename C::const_iterator &)>
-class AdaptedIterable {
-private:
-  const C &m_container;
-
-public:
-  AdaptedIterable(const C &container) : m_container(container) {}
-
-  AdaptedConstIterator<C, E, A> begin() {
-    return AdaptedConstIterator<C, E, A>(m_container.begin());
-  }
-
-  AdaptedConstIterator<C, E, A> end() {
-    return AdaptedConstIterator<C, E, A>(m_container.end());
-  }
+template <typename WrappedIteratorT,
+          typename T = typename std::iterator_traits<
+              WrappedIteratorT>::value_type::second_type>
+struct ValueMapIterator
+    : llvm::iterator_adaptor_base<
+          ValueMapIterator<WrappedIteratorT, T>, WrappedIteratorT,
+          typename std::iterator_traits<WrappedIteratorT>::iterator_category,
+          T> {
+  ValueMapIterator() = default;
+  explicit ValueMapIterator(WrappedIteratorT u)
+      : ValueMapIterator::iterator_adaptor_base(std::move(u)) {}
+
+  const T &operator*() { return (*this->I).second; }
+  const T &operator*() const { return (*this->I).second; }
 };
 
-template <typename C, typename E, E (*A)(typename C::const_iterator &),
-          typename MutexType>
-class LockingAdaptedIterable : public AdaptedIterable<C, E, A> {
+template <typename MutexType, typename C,
+          typename IteratorT = typename C::const_iterator>
+class LockingAdaptedIterable : public llvm::iterator_range<IteratorT> {
 public:
   LockingAdaptedIterable(const C &container, MutexType &mutex)
-      : AdaptedIterable<C, E, A>(container), m_mutex(&mutex) {
+      : llvm::iterator_range<IteratorT>(container), m_mutex(&mutex) {
     m_mutex->lock();
   }
 
   LockingAdaptedIterable(LockingAdaptedIterable &&rhs)
-      : AdaptedIterable<C, E, A>(rhs), m_mutex(rhs.m_mutex) {
+      : llvm::iterator_range<IteratorT>(rhs), m_mutex(rhs.m_mutex) {
     rhs.m_mutex = nullptr;
   }

AdaptedConstIterator currently doesn't have iterator traits, so I can't use STL algorithms with containers like WatchpointList. This patch replaces AdaptedConstIterator and AdaptedIterable with llvm::iterator_adaped_base and llvm::iterator_range respectively.

JDevlieghere

I suspect the LLDB classes preceded their LLVM counterparts, but the latter are much more complete and there's definitely no point having both. LGTM.

) AdaptedConstIterator currently doesn't have iterator traits, so I can't use STL algorithms with containers like WatchpointList. This patch replaces AdaptedConstIterator and AdaptedIterable with llvm::iterator_adaped_base and llvm::iterator_range respectively.

daniilavdeev requested a review from JDevlieghere as a code owner February 17, 2025 15:31

llvmbot added the lldb label Feb 17, 2025

daniilavdeev force-pushed the dlav-sc/lldb-replace-adapted-iter branch from 5b41afe to 9ac7f03 Compare February 17, 2025 15:37

daniilavdeev changed the title ~~[lldb] remove AdaptedConstIterator and AdaptedIterable~~ [lldb][NFC] remove AdaptedConstIterator and AdaptedIterable Feb 17, 2025

JDevlieghere approved these changes Feb 17, 2025

View reviewed changes

daniilavdeev merged commit 49453bf into llvm:main Feb 18, 2025
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][NFC] remove AdaptedConstIterator and AdaptedIterable #127507

[lldb][NFC] remove AdaptedConstIterator and AdaptedIterable #127507

Uh oh!

daniilavdeev commented Feb 17, 2025

Uh oh!

llvmbot commented Feb 17, 2025

Uh oh!

JDevlieghere left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[lldb][NFC] remove AdaptedConstIterator and AdaptedIterable #127507

[lldb][NFC] remove AdaptedConstIterator and AdaptedIterable #127507

Uh oh!

Conversation

daniilavdeev commented Feb 17, 2025

Uh oh!

llvmbot commented Feb 17, 2025

Uh oh!

JDevlieghere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants