aprantl added inline comments.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1273
+ "macosx", "iphonesimulator", "iphoneos", "appletvsimulator",
+ "appletvos", "watchsimulator", "watchos", "linux"};
----------------
How about hard-coding this in switch statement instead? The we can't have
off-by-one errors quite as easily...
`case watchOS: return "watchos"; ` etc.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1364
+ default:
+ return false;
}
----------------
Not your fault, but watchOS and tvOS should also be in this list.
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1433
FileSpec PlatformDarwin::GetSDKDirectoryForModules(SDKType sdk_type) {
FileSpec sdks_spec = GetXcodeContentsPath();
----------------
Not your fault again, but I find the name of this function super confusing.
What is the :"ForModules" part supposed to mean?
================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h:89
+ WatchSimulator,
+ watchOS,
+ Linux,
----------------
Technically, I suppose BridgeOS belongs in here, too.
================
Comment at: lldb/unittests/Platform/PlatformDarwinTest.cpp:80
+ EXPECT_EQ("/Applications/Xcode.app/Contents",
+ PlatformDarwinTester::FindXcodeContentsDirectoryInPath(standard));
+
----------------
Thanks!
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
