Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][modules] Enable built-in modules for the upcoming Apple releases #102239

Merged

Conversation

ian-twilightcoder
Copy link
Contributor

The upcoming Apple SDK releases will support the clang built-in headers being in the clang built-in modules: stop passing -fbuiltin-headers-in-system-modules for those SDK versions.

The upcoming Apple SDK releases will support the clang built-in headers being in the clang built-in modules: stop passing -fbuiltin-headers-in-system-modules for those SDK versions.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Aug 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 6, 2024

@llvm/pr-subscribers-clang

Author: Ian Anderson (ian-twilightcoder)

Changes

The upcoming Apple SDK releases will support the clang built-in headers being in the clang built-in modules: stop passing -fbuiltin-headers-in-system-modules for those SDK versions.


Full diff: https://github.com/llvm/llvm-project/pull/102239.diff

4 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+32-7)
  • (added) clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json (+1)
  • (renamed) clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json ()
  • (modified) clang/test/Driver/darwin-builtin-modules.c (+3-2)
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 17b6074160716..fbaa0ff22659f 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2923,22 +2923,47 @@ bool Darwin::isAlignedAllocationUnavailable() const {
   return TargetVersion < alignedAllocMinVersion(OS);
 }
 
-static bool sdkSupportsBuiltinModules(const Darwin::DarwinPlatformKind &TargetPlatform, const std::optional<DarwinSDKInfo> &SDKInfo) {
+static bool sdkSupportsBuiltinModules(
+    const Darwin::DarwinPlatformKind &TargetPlatform,
+    const Darwin::DarwinEnvironmentKind &TargetEnvironment,
+    const std::optional<DarwinSDKInfo> &SDKInfo) {
+  switch (TargetEnvironment) {
+  case Darwin::NativeEnvironment:
+  case Darwin::Simulator:
+  case Darwin::MacCatalyst:
+    // Standard xnu/Mach/Darwin based environments
+    // depend on the SDK version.
+    break;
+  default:
+    // All other environments support builtin modules from the start.
+    return true;
+  }
+
   if (!SDKInfo)
+    // If there is no SDK info, assume this is building against a
+    // pre-SDK version of macOS (i.e. before Mac OS X 10.4). Those
+    // don't support modules anyway, but the headers definitely
+    // don't support builtin modules either. It might also be some
+    // kind of degenerate build environment, err on the side of
+    // the old behavior which is to not use builtin modules.
     return false;
 
   VersionTuple SDKVersion = SDKInfo->getVersion();
   switch (TargetPlatform) {
+  // Existing SDKs added support for builtin modules in the fall
+  // 2024 major releases.
   case Darwin::MacOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(15U);
   case Darwin::IPhoneOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(18U);
   case Darwin::TvOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(18U);
   case Darwin::WatchOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(11U);
   case Darwin::XROS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(2U);
+
+  // New SDKs support builtin modules from the start.
   default:
     return true;
   }
@@ -3030,7 +3055,7 @@ void Darwin::addClangTargetOptions(
   // i.e. when the builtin stdint.h is in the Darwin module too, the cycle
   // goes away. Note that -fbuiltin-headers-in-system-modules does nothing
   // to fix the same problem with C++ headers, and is generally fragile.
-  if (!sdkSupportsBuiltinModules(TargetPlatform, SDKInfo))
+  if (!sdkSupportsBuiltinModules(TargetPlatform, TargetEnvironment, SDKInfo))
     CC1Args.push_back("-fbuiltin-headers-in-system-modules");
 
   if (!DriverArgs.hasArgNoClaim(options::OPT_fdefine_target_os_macros,
diff --git a/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json b/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json
new file mode 100644
index 0000000000000..7ba6c244df211
--- /dev/null
+++ b/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json
@@ -0,0 +1 @@
+{"Version":"23.0", "MaximumDeploymentTarget": "23.0.99"}
diff --git a/clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json b/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
similarity index 100%
rename from clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json
rename to clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
diff --git a/clang/test/Driver/darwin-builtin-modules.c b/clang/test/Driver/darwin-builtin-modules.c
index 1c56e13bfb929..ec515133be8ab 100644
--- a/clang/test/Driver/darwin-builtin-modules.c
+++ b/clang/test/Driver/darwin-builtin-modules.c
@@ -6,6 +6,7 @@
 // RUN: %clang -isysroot %S/Inputs/iPhoneOS13.0.sdk -target arm64-apple-ios13.0 -### %s 2>&1 | FileCheck %s
 // CHECK: -fbuiltin-headers-in-system-modules
 
-// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos98.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
-// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos99.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos14.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos15.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// RUN: %clang -isysroot %S/Inputs/DriverKit23.0.sdk -target arm64-apple-driverkit23.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
 // CHECK_FUTURE-NOT: -fbuiltin-headers-in-system-modules

@llvmbot
Copy link
Member

llvmbot commented Aug 6, 2024

@llvm/pr-subscribers-clang-driver

Author: Ian Anderson (ian-twilightcoder)

Changes

The upcoming Apple SDK releases will support the clang built-in headers being in the clang built-in modules: stop passing -fbuiltin-headers-in-system-modules for those SDK versions.


Full diff: https://github.com/llvm/llvm-project/pull/102239.diff

4 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+32-7)
  • (added) clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json (+1)
  • (renamed) clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json ()
  • (modified) clang/test/Driver/darwin-builtin-modules.c (+3-2)
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 17b6074160716..fbaa0ff22659f 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2923,22 +2923,47 @@ bool Darwin::isAlignedAllocationUnavailable() const {
   return TargetVersion < alignedAllocMinVersion(OS);
 }
 
-static bool sdkSupportsBuiltinModules(const Darwin::DarwinPlatformKind &TargetPlatform, const std::optional<DarwinSDKInfo> &SDKInfo) {
+static bool sdkSupportsBuiltinModules(
+    const Darwin::DarwinPlatformKind &TargetPlatform,
+    const Darwin::DarwinEnvironmentKind &TargetEnvironment,
+    const std::optional<DarwinSDKInfo> &SDKInfo) {
+  switch (TargetEnvironment) {
+  case Darwin::NativeEnvironment:
+  case Darwin::Simulator:
+  case Darwin::MacCatalyst:
+    // Standard xnu/Mach/Darwin based environments
+    // depend on the SDK version.
+    break;
+  default:
+    // All other environments support builtin modules from the start.
+    return true;
+  }
+
   if (!SDKInfo)
+    // If there is no SDK info, assume this is building against a
+    // pre-SDK version of macOS (i.e. before Mac OS X 10.4). Those
+    // don't support modules anyway, but the headers definitely
+    // don't support builtin modules either. It might also be some
+    // kind of degenerate build environment, err on the side of
+    // the old behavior which is to not use builtin modules.
     return false;
 
   VersionTuple SDKVersion = SDKInfo->getVersion();
   switch (TargetPlatform) {
+  // Existing SDKs added support for builtin modules in the fall
+  // 2024 major releases.
   case Darwin::MacOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(15U);
   case Darwin::IPhoneOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(18U);
   case Darwin::TvOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(18U);
   case Darwin::WatchOS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(11U);
   case Darwin::XROS:
-    return SDKVersion >= VersionTuple(99U);
+    return SDKVersion >= VersionTuple(2U);
+
+  // New SDKs support builtin modules from the start.
   default:
     return true;
   }
@@ -3030,7 +3055,7 @@ void Darwin::addClangTargetOptions(
   // i.e. when the builtin stdint.h is in the Darwin module too, the cycle
   // goes away. Note that -fbuiltin-headers-in-system-modules does nothing
   // to fix the same problem with C++ headers, and is generally fragile.
-  if (!sdkSupportsBuiltinModules(TargetPlatform, SDKInfo))
+  if (!sdkSupportsBuiltinModules(TargetPlatform, TargetEnvironment, SDKInfo))
     CC1Args.push_back("-fbuiltin-headers-in-system-modules");
 
   if (!DriverArgs.hasArgNoClaim(options::OPT_fdefine_target_os_macros,
diff --git a/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json b/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json
new file mode 100644
index 0000000000000..7ba6c244df211
--- /dev/null
+++ b/clang/test/Driver/Inputs/DriverKit23.0.sdk/SDKSettings.json
@@ -0,0 +1 @@
+{"Version":"23.0", "MaximumDeploymentTarget": "23.0.99"}
diff --git a/clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json b/clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
similarity index 100%
rename from clang/test/Driver/Inputs/MacOSX99.0.sdk/SDKSettings.json
rename to clang/test/Driver/Inputs/MacOSX15.0.sdk/SDKSettings.json
diff --git a/clang/test/Driver/darwin-builtin-modules.c b/clang/test/Driver/darwin-builtin-modules.c
index 1c56e13bfb929..ec515133be8ab 100644
--- a/clang/test/Driver/darwin-builtin-modules.c
+++ b/clang/test/Driver/darwin-builtin-modules.c
@@ -6,6 +6,7 @@
 // RUN: %clang -isysroot %S/Inputs/iPhoneOS13.0.sdk -target arm64-apple-ios13.0 -### %s 2>&1 | FileCheck %s
 // CHECK: -fbuiltin-headers-in-system-modules
 
-// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos98.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
-// RUN: %clang -isysroot %S/Inputs/MacOSX99.0.sdk -target x86_64-apple-macos99.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos14.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// RUN: %clang -isysroot %S/Inputs/MacOSX15.0.sdk -target x86_64-apple-macos15.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
+// RUN: %clang -isysroot %S/Inputs/DriverKit23.0.sdk -target arm64-apple-driverkit23.0 -### %s 2>&1 | FileCheck --check-prefix=CHECK_FUTURE %s
 // CHECK_FUTURE-NOT: -fbuiltin-headers-in-system-modules

Copy link
Member

@cyndyishida cyndyishida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ian-twilightcoder ian-twilightcoder merged commit 9616399 into llvm:main Aug 7, 2024
10 checks passed
@ian-twilightcoder ian-twilightcoder added this to the LLVM 19.X Release milestone Aug 7, 2024
@ian-twilightcoder
Copy link
Contributor Author

/cherry-pick 9616399

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 7, 2024
…ses (llvm#102239)

The upcoming Apple SDK releases will support the clang built-in headers
being in the clang built-in modules: stop passing
-fbuiltin-headers-in-system-modules for those SDK versions.

(cherry picked from commit 9616399)
@llvmbot
Copy link
Member

llvmbot commented Aug 7, 2024

/pull-request #102335

@ian-twilightcoder
Copy link
Contributor Author

/cherry-pick 0f1361b

@llvmbot
Copy link
Member

llvmbot commented Aug 8, 2024

Failed to cherry-pick: 0f1361b

https://github.com/llvm/llvm-project/actions/runs/10294025156

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

TIFitis pushed a commit that referenced this pull request Aug 8, 2024
…ses (#102239)

The upcoming Apple SDK releases will support the clang built-in headers
being in the clang built-in modules: stop passing
-fbuiltin-headers-in-system-modules for those SDK versions.
ian-twilightcoder added a commit to ian-twilightcoder/llvm-project that referenced this pull request Aug 8, 2024
…ng Apple releases (llvm#102239)

The upcoming Apple SDK releases will support the clang built-in headers
being in the clang built-in modules: stop passing
-fbuiltin-headers-in-system-modules for those SDK versions.
@ian-twilightcoder
Copy link
Contributor Author

Failed to cherry-pick: 0f1361b

https://github.com/llvm/llvm-project/actions/runs/10294025156

Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request

Manual pull request: #102517

@ian-twilightcoder
Copy link
Contributor Author

/cherry-pick 9616399 0f1361b

llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 8, 2024
…ses (llvm#102239)

The upcoming Apple SDK releases will support the clang built-in headers
being in the clang built-in modules: stop passing
-fbuiltin-headers-in-system-modules for those SDK versions.

(cherry picked from commit 9616399)
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Aug 10, 2024
…ses (llvm#102239)

The upcoming Apple SDK releases will support the clang built-in headers
being in the clang built-in modules: stop passing
-fbuiltin-headers-in-system-modules for those SDK versions.

(cherry picked from commit 9616399)
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Aug 15, 2024
…ses (llvm#102239)

The upcoming Apple SDK releases will support the clang built-in headers
being in the clang built-in modules: stop passing
-fbuiltin-headers-in-system-modules for those SDK versions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

3 participants