[PATCH] D133273: [compiler-rt][macOS]: Fix building compiler-rt without iOS related SDKs

[Brittany Blue Gaston via Phabricator via llvm-commits]

thetruestblue accepted this revision.
thetruestblue added a comment.

Nit: When you land this will you update commit message to reflect the new change?

This LGTM with the most recent changes. One additional Nit inline.



================
Comment at: compiler-rt/cmake/base-config-ix.cmake:156
+  if("${HAS_IOS_SDK}" STREQUAL "")
+    set(COMPILER_RT_ENABLE_IOS_DEFAULT Off)
+  endif()
----------------
Nit: I'm wondering if there is any concern about this being a silent failure if there is no iOS SDK unexpectedly, as now we explicitly need to set COMPILER_RT_ENABLE_IOS=ON to catch a missing iOS SDK. Which is a change of behavior, correct?

Is it worth adding a compiler message or warning similar to the messages printed when looking for macosx sdk above to call out the change in what was previously "default".

message(WARNING "No iOS SDK found. Building for iOS not enabled.")

Seems like an inconvenient warning in the the small subset of cases where there is no iOS SDK, but could save a headache if there's an unexpectedly missing SDK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133273