aprantl added inline comments.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp:225
+ lldb_private::Status error = Host::RunShellCommand(
+ "xcrun -sdk macosx --show-sdk-path", FileSpec(), &status, &signo,
+ &output_str, std::chrono::seconds(3));
----------------
Ah. We always ask for the macos SDK?
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp:226
+ "xcrun -sdk macosx --show-sdk-path", FileSpec(), &status, &signo,
+ &output_str, std::chrono::seconds(3));
+
----------------
There's a global variable for various timeouts that we should use instead. Grep
for commits with timeout in the name by me.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp:250
+ if (ComputeContentsPath(sdk))
+ return sdk;
+
----------------
This looks a lot nicer than the original function.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h:81
+ std::string default_sdk;
+ std::string contents_path;
+ XcodeSDK(std::string sdk = {})
----------------
Can you add a comment like:
`/// path/to/Xcode.app/Developer/...` so it's easier to visualize what these
are?
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.h:90
+ /// Get the SDK path from xcrun.
+ static XcodeSDK GetXcodeSDK(bool use_xcrun = true);
+
----------------
The comment and the parameter seem to contradict each other about whether xcrun
is being used? :-)
Should the function be called, GetDefaultXcodeSDK?
I'm also wondering why a function with this name doesn't take a triple.
================
Comment at: lldb/unittests/Platform/PlatformMacOSXTest.cpp:22
+
+TEST(PlatformMacOSXTest, ComputeContentsPath) {
+ PlatformMacOSX::XcodeSDK standard(
----------------
Awesome!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76261/new/
https://reviews.llvm.org/D76261
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
