[clang] [clang][Darwin] Remove legacy framework search path logic in the frontend (PR #120149)
Louis Dionne via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 19 09:32:03 PST 2024
https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/120149
>From e578bd75d82a5ff16168222e4f30c32f9aa5e6bd Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 16 Dec 2024 13:28:38 -0500
Subject: [PATCH 1/3] [clang][Darwin] Remove legacy framework search path logic
in the frontend
This removes a long standing piece of technical debt. Most other platforms
have moved all their header search path logic to the driver, but Darwin
still had some logic for setting framework search paths present in the
frontend. This patch moves that logic to the driver alongside existing
logic that already handles part of these search paths.
To achieve parity with the previous search path order, this patch
introduces the -internal-iframework flag which is used to pass
system framework paths from the driver to the frontend. These paths
are handled specially in that they are added after normal framework
search paths, which preserves the old frontend behavior for system
frameworks.
This patch is a re-application of https://github.com/llvm/llvm-project/pull/75841
which was reverted in d34901f30 because it broke framework search paths.
In fact, the original patch was only adding framework search paths to
the linker job, but was not adding search paths for finding headers.
That issue is resolved in this version of the patch, with added tests.
Fixes #75638
---
clang/include/clang/Driver/Options.td | 5 ++
clang/lib/Driver/Job.cpp | 2 +-
clang/lib/Driver/ToolChains/Darwin.cpp | 49 ++++++++++++++-----
clang/lib/Frontend/CompilerInvocation.cpp | 7 ++-
clang/lib/Lex/InitHeaderSearch.cpp | 19 ++-----
.../Driver/darwin-framework-search-paths.c | 26 ++++++++++
clang/test/Driver/darwin-subframeworks.c | 18 -------
.../test/Preprocessor/cuda-macos-includes.cu | 13 ++---
8 files changed, 79 insertions(+), 60 deletions(-)
create mode 100644 clang/test/Driver/darwin-framework-search-paths.c
delete mode 100644 clang/test/Driver/darwin-subframeworks.c
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 88862ae9edb29d..8692d5d7eabe1c 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8265,6 +8265,11 @@ def internal_externc_isystem : Separate<["-"], "internal-externc-isystem">,
"implicit extern \"C\" semantics; these are assumed to not be "
"user-provided and are used to model system and standard headers' "
"paths.">;
+def internal_iframework : Separate<["-"], "internal-iframework">,
+ MetaVarName<"<directory>">,
+ HelpText<"Add directory to the internal system framework include search path; these "
+ "are assumed to not be user-provided and are used to model system "
+ "and standard frameworks' paths.">;
} // let Visibility = [CC1Option]
diff --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp
index ae2f1cd1f56c99..07d2d371c5626b 100644
--- a/clang/lib/Driver/Job.cpp
+++ b/clang/lib/Driver/Job.cpp
@@ -73,7 +73,7 @@ static bool skipArgs(const char *Flag, bool HaveCrashVFS, int &SkipNum,
.Cases("-internal-externc-isystem", "-iprefix", true)
.Cases("-iwithprefixbefore", "-isystem", "-iquote", true)
.Cases("-isysroot", "-I", "-F", "-resource-dir", true)
- .Cases("-iframework", "-include-pch", true)
+ .Cases("-internal-iframework", "-iframework", "-include-pch", true)
.Default(false);
if (IsInclude)
return !HaveCrashVFS;
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index cdb6d21a0148b6..76d6244daff920 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -800,9 +800,15 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
}
}
- // Add non-standard, platform-specific search paths, e.g., for DriverKit:
- // -L<sysroot>/System/DriverKit/usr/lib
- // -F<sysroot>/System/DriverKit/System/Library/Framework
+ // Add framework include paths and library search paths.
+ // There are two flavors:
+ // 1. The "non-standard" paths, e.g. for DriverKit:
+ // -L<sysroot>/System/DriverKit/usr/lib
+ // -F<sysroot>/System/DriverKit/System/Library/Frameworks
+ // 2. The "standard" paths, e.g. for macOS and iOS:
+ // -F<sysroot>/System/Library/Frameworks
+ // -F<sysroot>/System/Library/SubFrameworks
+ // -F<sysroot>/Library/Frameworks
{
bool NonStandardSearchPath = false;
const auto &Triple = getToolChain().getTriple();
@@ -813,18 +819,23 @@ void darwin::Linker::ConstructJob(Compilation &C, const JobAction &JA,
(Version.getMajor() == 605 && Version.getMinor().value_or(0) < 1);
}
- if (NonStandardSearchPath) {
- if (auto *Sysroot = Args.getLastArg(options::OPT_isysroot)) {
- auto AddSearchPath = [&](StringRef Flag, StringRef SearchPath) {
- SmallString<128> P(Sysroot->getValue());
- AppendPlatformPrefix(P, Triple);
- llvm::sys::path::append(P, SearchPath);
- if (getToolChain().getVFS().exists(P)) {
- CmdArgs.push_back(Args.MakeArgString(Flag + P));
- }
- };
+ if (auto *Sysroot = Args.getLastArg(options::OPT_isysroot)) {
+ auto AddSearchPath = [&](StringRef Flag, StringRef SearchPath) {
+ SmallString<128> P(Sysroot->getValue());
+ AppendPlatformPrefix(P, Triple);
+ llvm::sys::path::append(P, SearchPath);
+ if (getToolChain().getVFS().exists(P)) {
+ CmdArgs.push_back(Args.MakeArgString(Flag + P));
+ }
+ };
+
+ if (NonStandardSearchPath) {
AddSearchPath("-L", "/usr/lib");
AddSearchPath("-F", "/System/Library/Frameworks");
+ } else if (!Triple.isDriverKit()) {
+ AddSearchPath("-F", "/System/Library/Frameworks");
+ AddSearchPath("-F", "/System/Library/SubFrameworks");
+ AddSearchPath("-F", "/Library/Frameworks");
}
}
}
@@ -2539,6 +2550,18 @@ void DarwinClang::AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs
llvm::sys::path::append(P, "usr", "include");
addExternCSystemInclude(DriverArgs, CC1Args, P.str());
}
+
+ // Add default framework search paths
+ auto addFrameworkInclude = [&](auto ...Path) {
+ SmallString<128> P(Sysroot);
+ llvm::sys::path::append(P, Path...);
+
+ CC1Args.push_back("-internal-iframework");
+ CC1Args.push_back(DriverArgs.MakeArgString(P));
+ };
+ addFrameworkInclude("System", "Library", "Frameworks");
+ addFrameworkInclude("System", "Library", "SubFrameworks");
+ addFrameworkInclude("Library", "Frameworks");
}
bool DarwinClang::AddGnuCPlusPlusIncludePaths(const llvm::opt::ArgList &DriverArgs,
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 23906d5c06d380..4713fea5a26378 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3413,11 +3413,14 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
// Add the internal paths from a driver that detects standard include paths.
for (const auto *A :
- Args.filtered(OPT_internal_isystem, OPT_internal_externc_isystem)) {
+ Args.filtered(OPT_internal_isystem, OPT_internal_externc_isystem, OPT_internal_iframework)) {
frontend::IncludeDirGroup Group = frontend::System;
+ bool IsFramework = false;
if (A->getOption().matches(OPT_internal_externc_isystem))
Group = frontend::ExternCSystem;
- Opts.AddPath(A->getValue(), Group, false, true);
+ if (A->getOption().matches(OPT_internal_iframework))
+ IsFramework = true;
+ Opts.AddPath(A->getValue(), Group, IsFramework, true);
}
// Add the path prefixes which are implicitly treated as being system headers.
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index ea02f5dfb62644..76bc23a9054960 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -320,6 +320,9 @@ bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
break;
}
+ if (triple.isOSDarwin())
+ return false;
+
return true; // Everything else uses AddDefaultIncludePaths().
}
@@ -334,22 +337,6 @@ void InitHeaderSearch::AddDefaultIncludePaths(
if (!ShouldAddDefaultIncludePaths(triple))
return;
- // NOTE: some additional header search logic is handled in the driver for
- // Darwin.
- if (triple.isOSDarwin()) {
- if (HSOpts.UseStandardSystemIncludes) {
- // Add the default framework include paths on Darwin.
- if (triple.isDriverKit()) {
- AddPath("/System/DriverKit/System/Library/Frameworks", System, true);
- } else {
- AddPath("/System/Library/Frameworks", System, true);
- AddPath("/System/Library/SubFrameworks", System, true);
- AddPath("/Library/Frameworks", System, true);
- }
- }
- return;
- }
-
if (Lang.CPlusPlus && !Lang.AsmPreprocessor &&
HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemIncludes) {
if (HSOpts.UseLibcxx) {
diff --git a/clang/test/Driver/darwin-framework-search-paths.c b/clang/test/Driver/darwin-framework-search-paths.c
new file mode 100644
index 00000000000000..4bfe269f8d903d
--- /dev/null
+++ b/clang/test/Driver/darwin-framework-search-paths.c
@@ -0,0 +1,26 @@
+// UNSUPPORTED: system-windows
+// Windows is unsupported because we use the Unix path separator `/` in the test.
+
+// Add default directories before running clang to check default
+// search paths.
+// RUN: rm -rf %t && mkdir -p %t
+// RUN: cp -R %S/Inputs/MacOSX15.1.sdk %t/
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/Frameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/SubFrameworks
+// RUN: mkdir -p %t/MacOSX15.1.sdk/usr/include
+
+// RUN: %clang -xc %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck --check-prefix=CHECK-C %s
+//
+// CHECK-C: -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
+// CHECK-C: #include <...> search starts here:
+// CHECK-C: [[PATH]]/usr/include
+// CHECK-C: [[PATH]]/System/Library/Frameworks (framework directory)
+// CHECK-C: [[PATH]]/System/Library/SubFrameworks (framework directory)
+
+// RUN: %clang -xc++ %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck --check-prefix=CHECK-CXX %s
+//
+// CHECK-CXX: -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
+// CHECK-CXX: #include <...> search starts here:
+// CHECK-CXX: [[PATH]]/usr/include
+// CHECK-CXX: [[PATH]]/System/Library/Frameworks (framework directory)
+// CHECK-CXX: [[PATH]]/System/Library/SubFrameworks (framework directory)
diff --git a/clang/test/Driver/darwin-subframeworks.c b/clang/test/Driver/darwin-subframeworks.c
deleted file mode 100644
index 1a7a095c642922..00000000000000
--- a/clang/test/Driver/darwin-subframeworks.c
+++ /dev/null
@@ -1,18 +0,0 @@
-// UNSUPPORTED: system-windows
-// Windows is unsupported because we use the Unix path separator `\`.
-
-// Add default directories before running clang to check default
-// search paths.
-// RUN: rm -rf %t && mkdir -p %t
-// RUN: cp -R %S/Inputs/MacOSX15.1.sdk %t/
-// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/Frameworks
-// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/SubFrameworks
-// RUN: mkdir -p %t/MacOSX15.1.sdk/usr/include
-
-// RUN: %clang %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck %s
-
-// CHECK: -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
-// CHECK: #include <...> search starts here:
-// CHECK: [[PATH]]/usr/include
-// CHECK: [[PATH]]/System/Library/Frameworks (framework directory)
-// CHECK: [[PATH]]/System/Library/SubFrameworks (framework directory)
diff --git a/clang/test/Preprocessor/cuda-macos-includes.cu b/clang/test/Preprocessor/cuda-macos-includes.cu
index 6ef94b0e453520..dbc991dd610882 100644
--- a/clang/test/Preprocessor/cuda-macos-includes.cu
+++ b/clang/test/Preprocessor/cuda-macos-includes.cu
@@ -1,13 +1,6 @@
-// RUN: %clang -cc1 -fcuda-is-device -isysroot /var/empty \
-// RUN: -triple nvptx-nvidia-cuda -aux-triple i386-apple-macosx \
-// RUN: -E -fcuda-is-device -v -o /dev/null -x cuda %s 2>&1 | FileCheck %s
-
-// RUN: %clang -cc1 -isysroot /var/empty \
-// RUN: -triple i386-apple-macosx -aux-triple nvptx-nvidia-cuda \
-// RUN: -E -fcuda-is-device -v -o /dev/null -x cuda %s 2>&1 | FileCheck %s
-
// Check that when we do CUDA host and device compiles on MacOS, we check for
// includes in /System/Library/Frameworks and /Library/Frameworks.
-// CHECK-DAG: ignoring nonexistent directory "/var/empty/System/Library/Frameworks"
-// CHECK-DAG: ignoring nonexistent directory "/var/empty/Library/Frameworks"
+// RUN: %clang -isysroot /var/empty -target unknown-nvidia-cuda -v -fsyntax-only -x cuda %s -### 2>&1 | FileCheck %s
+// CHECK-DAG: "-internal-iframework" "/var/empty/System/Library/Frameworks"
+// CHECK-DAG: "-internal-iframework" "/var/empty/Library/Frameworks"
>From 80de3293ce4b5a0580b617859f2ea33dbfa2b7c9 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 19 Dec 2024 12:16:56 -0500
Subject: [PATCH 2/3] Remove CUDA on Mac test since that isn't supported
anymore
---
clang/test/Preprocessor/cuda-macos-includes.cu | 6 ------
1 file changed, 6 deletions(-)
delete mode 100644 clang/test/Preprocessor/cuda-macos-includes.cu
diff --git a/clang/test/Preprocessor/cuda-macos-includes.cu b/clang/test/Preprocessor/cuda-macos-includes.cu
deleted file mode 100644
index dbc991dd610882..00000000000000
--- a/clang/test/Preprocessor/cuda-macos-includes.cu
+++ /dev/null
@@ -1,6 +0,0 @@
-// Check that when we do CUDA host and device compiles on MacOS, we check for
-// includes in /System/Library/Frameworks and /Library/Frameworks.
-
-// RUN: %clang -isysroot /var/empty -target unknown-nvidia-cuda -v -fsyntax-only -x cuda %s -### 2>&1 | FileCheck %s
-// CHECK-DAG: "-internal-iframework" "/var/empty/System/Library/Frameworks"
-// CHECK-DAG: "-internal-iframework" "/var/empty/Library/Frameworks"
>From 21a85dfc30eab0775dad707a8d864d77c10e959e Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Thu, 19 Dec 2024 12:21:02 -0500
Subject: [PATCH 3/3] Use macosx15.1 target instead of darwin13.0
---
clang/test/Driver/darwin-framework-search-paths.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/test/Driver/darwin-framework-search-paths.c b/clang/test/Driver/darwin-framework-search-paths.c
index 4bfe269f8d903d..c65f7fd3266921 100644
--- a/clang/test/Driver/darwin-framework-search-paths.c
+++ b/clang/test/Driver/darwin-framework-search-paths.c
@@ -9,7 +9,7 @@
// RUN: mkdir -p %t/MacOSX15.1.sdk/System/Library/SubFrameworks
// RUN: mkdir -p %t/MacOSX15.1.sdk/usr/include
-// RUN: %clang -xc %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck --check-prefix=CHECK-C %s
+// RUN: %clang -xc %s -target arm64-apple-macosx15.1 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck --check-prefix=CHECK-C %s
//
// CHECK-C: -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
// CHECK-C: #include <...> search starts here:
@@ -17,7 +17,7 @@
// CHECK-C: [[PATH]]/System/Library/Frameworks (framework directory)
// CHECK-C: [[PATH]]/System/Library/SubFrameworks (framework directory)
-// RUN: %clang -xc++ %s -target arm64-apple-darwin13.0 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck --check-prefix=CHECK-CXX %s
+// RUN: %clang -xc++ %s -target arm64-apple-macosx15.1 -isysroot %t/MacOSX15.1.sdk -E -v 2>&1 | FileCheck --check-prefix=CHECK-CXX %s
//
// CHECK-CXX: -isysroot [[PATH:[^ ]*/MacOSX15.1.sdk]]
// CHECK-CXX: #include <...> search starts here:
More information about the cfe-commits
mailing list