jasonmolenda added a comment.
OK looked things over a bit.
I think there's a case for adopting llvm::ErrorOr here, and elsewhere across,
but today the only place we're using that return type is when the error is
encoded in a std::error_code (aka fancy errno). We should have an ErrorOr that
uses a Status object to contain the error, I think that's possible? I've seen
people use their own objects to return errors with llvm::Expected. In any
event, I think a change like this is a little out of scope for what I'm doing
in this patch right now.
After spending time looking at all the error handling codepaths related to
AddOrCreateModule, if we simply fail to find a binary we return an empty
ModuleSP and the platform may set a Status with an error string (e.g.
PlatformPOSIX will say no such file) if that's non-nullptr. Or there may be a
genuine error - a Target that doesn't have a Platform, we found a binary but it
is an explicitly disallowed type (e.g. eTypeStubLibrary, a long-obsolete binary
format on Darwin), we can return an error message explaining that to the caller
if the Status ptr is non-nullptr.
The important wrinkle with AddOrCreateModule is that most of the callers are
deep inside lldb internal mechanisms -- the DynamicLoaders. And for most of
those, we're going to handle file-not-found in custom ways, we're not going to
relay the error messages. For instance, the user-process DynamicLoaderDarwin,
when it fails to find a file, it will call Process::ReadModuleFromMemory(). If
the kernel debugging DynamicLoaderDarwinKernel fails to load a kext (a solib),
it accumulates a list of kexts that it could not find, and at the end it will
present a sorted list of them. These callers have no interest in the returned
error message.
On the other hand, a caller like CommandObjectTarget, which is directly driven
by user interaction, we must print the error message ("file not found" and
such).
So in this particularly API, because only some callers want the error strings,
leaving this as an optional argument seems reasonable. An llvm::ErrorOr which
returns a ModuleSP or a Status would be a fine - and only the callers that want
to print the error message would retrieve the Status object, the others would
do their normal behavior when couldn't get the ModuleSP.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60172/new/
https://reviews.llvm.org/D60172
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
