[Lldb-commits] [PATCH] D73594: Refactor CommandObjectTargetSymbolsAdd::AddModuleSymbols

[Adrian McCarthy via Phabricator via lldb-commits]

amccarth marked an inline comment as done.
amccarth added inline comments.


================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:4149
+    
+    lldbassert(matching_modules.GetSize() == 1);
+    ModuleSP module_sp(matching_modules.GetModuleAtIndex(0));
----------------
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.


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

https://reviews.llvm.org/D73594