jasonmolenda marked 2 inline comments as done.
jasonmolenda added a comment.
Thanks for looking the patch over.
================
Comment at: include/lldb/Target/Target.h:535
+ bool notify,
+ Status *error_ptr = nullptr);
----------------
clayborg wrote:
> Pavel had questions about this error. If we specify an error when we call
> this, is there a way to get a valid module shared pointer back and still get
> an error? Maybe this should be one of the llvm::ErrorOr return types?
There's only a handful of AddModule callers that pass in a Status object - the
Windows ones, the CommandObjectTarget command, and the Minidump. I'll look
through the Platform GetSharedModules implementations tomorrow to see who might
put useful things in the error object, but in general either AddModule finds a
binary that matches the ModuleSpec, and returns a shared pointer to it, or
finds nothing and returns an empty shared pointer. The error object may have a
message from a Platform indicating why the search failed ("could not find any
SDK directories" or something) but I suspect this optional Status arg isn't
doing anything.
================
Comment at: source/Commands/CommandObjectTarget.cpp:399-400
+ const bool notify = true;
+ ModuleSP module_sp = target_sp->AddModule(main_module_spec,
+ notify);
if (module_sp)
----------------
clayborg wrote:
> Remove all "const bool notify = true; "statements and Inline with comment?
>
> ```
> ModuleSP module_sp = target_sp->AddModule(main_module_spec,
> true /*notify*/);
> ```
> This would apply everywhere in this patch if so.
Can do. I don't have a preference.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60172/new/
https://reviews.llvm.org/D60172
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
