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

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added inline comments.


================
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);
+
----------------
dblaikie wrote:
> 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.
I copied all of this from somewhere else in LLDB without giving it much thought because honestly I'm not sure this iterator is that much better than then existing `LockingAdaptedIterable`. Sorry for making you review this half-baked code... 


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

https://reviews.llvm.org/D94136