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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
