[Lldb-commits] [PATCH] D94136: [lldb] Make ModuleList iterable (NFC)

[David Blaikie via Phabricator via lldb-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Looks good to me with a few minor cleanups.



================
Comment at: lldb/include/lldb/Core/ModuleList.h:73-74
+public:
+  explicit ModuleIterator(const ModuleList *module_list, size_t idx);
+  ModuleIterator(const ModuleList *module_list);
+
----------------
Interesting choice to make the single-arg ctor implicit but the multi-arg explicit (usually people end up the other way around - because until C++ 17 or something, explicit was only relevant for single-arg constructors). In any case the ctors are only used once each & probably might as well have them both explicit.

Or maybe make only one ctor that takes the index explicitly and use it from begin/end - not sure there's much value added by having two ctors here, and honestly I'd have expected the no-arg version to start at zero, and the arg-taking version to start wherever you want, and I see they're the opposite, so that'd probably reinforce the idea that just having the one that takes an explicit index might be less confusing.


================
Comment at: lldb/include/lldb/Core/ModuleList.h:78-87
+  ModuleIterator &operator++() {
+    ++m_index;
+    return *this;
+  }
+
+  ModuleIterator &operator--() {
+    --m_index;
----------------
I think you can drop these two now - they'll be provided by the iterator facade.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94136/new/

https://reviews.llvm.org/D94136