llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Louis Dionne (ldionne) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/120149.diff 8 Files Affected: - (modified) clang/include/clang/Driver/Options.td (+5) - (modified) clang/lib/Driver/Job.cpp (+1-1) - (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+36-13) - (modified) clang/lib/Frontend/CompilerInvocation.cpp (+5-2) - (modified) clang/lib/Lex/InitHeaderSearch.cpp (+3-16) - (added) clang/test/Driver/darwin-framework-search-paths.c (+26) - (removed) clang/test/Driver/darwin-subframeworks.c (-18) - (modified) clang/test/Preprocessor/cuda-macos-includes.cu (+3-10) ``````````diff 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" `````````` </details> https://github.com/llvm/llvm-project/pull/120149 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
