[Lldb-commits] [PATCH] D89428: Add support for more OS types to AddClangModuleCompilationOptionsForSDKType()

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1525
+  // If the SDK type is for the host OS, use its version number.
+  auto get_host_os = []() { return HostInfo::GetTargetTriple().getOS(); };
   switch (sdk_type) {
----------------
teemperor wrote:
> `Triple::OSType host_os = HostInfo::GetTargetTriple().getOS();` ?
I think the idea was to avoid pointlessly calling it for the simulators.


================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1576
+    case XcodeSDK::Type::AppleTVOS:
+      minimum_version_option.PutCString(opt_mtvos_simulator_version_min_EQ);
       break;
----------------
teemperor wrote:
> `opt_mtvos_version_min_EQ`
Thank you!


================
Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp:1585
+    case XcodeSDK::Type::bridgeOS:
+      minimum_version_option.PutCString(opt_mwatchos_version_min_EQ);
+      break;
----------------
teemperor wrote:
> If this is on purpose, then I think there should be a comment explaining why this is not `-mbridgeos-version-min=` ?
The Clang support for this doesn't exist on llvm.org. I have removed it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89428/new/

https://reviews.llvm.org/D89428