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