clayborg added inline comments.
================ Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4149 + + lldbassert(matching_modules.GetSize() == 1); + ModuleSP module_sp(matching_modules.GetModuleAtIndex(0)); ---------------- amccarth wrote: > clayborg wrote: > > labath wrote: > > > This should be a regular assert according to > > > <https://lldb.llvm.org/resources/contributing.html#error-handling-and-use-of-assertions-in-lldb>. > > This assert will never trigger with the above code already checking for > > empty on line 4123 and then checking if size > 1 on line 4140. Remove it? > It's true that this assert won't happen given the early returns above, so I > will remove it if you like. > > But I did it intentionally: (1) to document the requirement and (2) to > protect against future changes accidentally breaking the assumptions. If the > function were much shorter, then it might be obvious enough as is. But early > returns, especially halfway down a long function, can be easy to miss. Gotcha. It is fine to leave it in, just switch to assert() as Pavel mentioned. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73594/new/ https://reviews.llvm.org/D73594 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
