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

David Blaikie via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Jan 5 18:51:49 PST 2021

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
+  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.



More information about the lldb-commits mailing list