jingham added a comment.
This LGTM with a couple of quibbles but I haven't been working on the platform
support recently so we should wait till people who have done so weight in.
================
Comment at: lldb/source/Target/TargetList.cpp:175-176
+ if (module_specs.GetModuleSpecAtIndex(i, module_spec)) {
+ // See if there was a selected platform and check that first
+ // since the user may have specified it.
+ if (platform_sp) {
----------------
This comment was confusing before, but it's more confusing now because
platform_sp might be the user specified platform or it might be the one passed
in with --platform.
================
Comment at: lldb/source/Target/TargetList.cpp:245
}
+ error_strm.Printf("), specify an architecture to disambiguate");
+ error.SetErrorString(error_strm.GetString());
----------------
I'm not sure why you removed mention of --platform from this error because with
this patch it should now be possible to disambiguate either by specifying
--platform or --arch, right?
================
Comment at: lldb/test/API/commands/target/basic/bogus.yaml:945
+ Values: []
+ debug_line:
+ - Length: 39
----------------
You don't use the debug info, might as well leave it out.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84809/new/
https://reviews.llvm.org/D84809
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
