Skip to content

[clang][Darwin] Remove legacy framework search path logic in the frontend #138234

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ian-twilightcoder
Copy link
Contributor

@ian-twilightcoder ian-twilightcoder commented May 2, 2025

Move the Darwin framework search path logic from InitHeaderSearch::AddDefaultIncludePaths to DarwinClang::AddClangSystemIncludeArgs. Add a new -internal-iframework cc1 argument to support the tool chain adding these paths.
Now that the tool chain is adding search paths via cc1 flag, they're only added if they exist, so the Preprocessor/cuda-macos-includes.cu test is no longer relevant.
Change Driver/driverkit-path.c and Driver/darwin-subframeworks.c to do -### style testing similar to the darwin-header-search and darwin-embedded-search-paths tests. Rename darwin-subframeworks.c to darwin-framework-search-paths.c and have it test all framework search paths, not just SubFrameworks.
Add a unit test to validate that the myriad of search path flags result in the expected search path list.

Fixes #75638

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 2, 2025
@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-clang

Author: Ian Anderson (ian-twilightcoder)

Changes

Move the Darwin framework search path logic from InitHeaderSearch::AddDefaultIncludePaths to DarwinClang::AddClangSystemIncludeArgs. Add a new -internal-iframework cc1 argument to support the tool chain adding these paths. Now that the tool chain is adding search paths via cc1 flag, they're only added if they exist, so the Preprocessor/cuda-macos-includes.cu test is no longer relevant. Change Driver/driverkit-path.c and Driver/darwin-subframeworks.c to do -### style testing similar to the darwin-header-search and darwin-embedded-search-paths tests. Rename darwin-subframeworks.c to darwin-framework-search-paths.c and have it test all framework search paths, not just SubFrameworks. Add a unit test to validate that the myriad of search path flags result in the expected search path list.

Fixes #75638


Patch is 21.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138234.diff

17 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+5)
  • (modified) clang/include/clang/Driver/ToolChain.h (+6)
  • (modified) clang/lib/Driver/Job.cpp (+1-1)
  • (modified) clang/lib/Driver/ToolChain.cpp (+20)
  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+21)
  • (modified) clang/lib/Driver/ToolChains/Darwin.h (+4)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+4)
  • (modified) clang/lib/Lex/InitHeaderSearch.cpp (+3-16)
  • (added) clang/test/Driver/Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks/.keep ()
  • (added) clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibraryFrameworks/.keep ()
  • (added) clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibrarySubFrameworks/.keep ()
  • (added) clang/test/Driver/darwin-framework-search-paths.c (+23)
  • (removed) clang/test/Driver/darwin-subframeworks.c (-18)
  • (modified) clang/test/Driver/driverkit-path.c (+9-7)
  • (removed) clang/test/Preprocessor/cuda-macos-includes.cu (-13)
  • (modified) clang/unittests/Frontend/CMakeLists.txt (+1)
  • (added) clang/unittests/Frontend/SearchPathTest.cpp (+157)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 561b0498c549c..7d2b847dd3830 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8413,6 +8413,11 @@ def objc_isystem : Separate<["-"], "objc-isystem">,
 def objcxx_isystem : Separate<["-"], "objcxx-isystem">,
   MetaVarName<"<directory>">,
   HelpText<"Add directory to the ObjC++ SYSTEM include search path">;
+def internal_iframework : Separate<["-"], "internal-iframework">,
+  MetaVarName<"<directory>">,
+  HelpText<"Add directory to the internal system framework search path; these "
+           "are assumed to not be user-provided and are used to model system "
+           "and standard frameworks' paths.">;
 def internal_isystem : Separate<["-"], "internal-isystem">,
   MetaVarName<"<directory>">,
   HelpText<"Add directory to the internal system include search path; these "
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index d0059673d6a67..58edf2b3887b0 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -226,6 +226,9 @@ class ToolChain {
 
   /// \name Utilities for implementing subclasses.
   ///@{
+  static void addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
+                                        llvm::opt::ArgStringList &CC1Args,
+                                        const Twine &Path);
   static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
                                llvm::opt::ArgStringList &CC1Args,
                                const Twine &Path);
@@ -236,6 +239,9 @@ class ToolChain {
       addExternCSystemIncludeIfExists(const llvm::opt::ArgList &DriverArgs,
                                       llvm::opt::ArgStringList &CC1Args,
                                       const Twine &Path);
+  static void addSystemFrameworkIncludes(const llvm::opt::ArgList &DriverArgs,
+                                         llvm::opt::ArgStringList &CC1Args,
+                                         ArrayRef<StringRef> Paths);
   static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
                                 llvm::opt::ArgStringList &CC1Args,
                                 ArrayRef<StringRef> Paths);
diff --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp
index 4619b8c1887be..60f219f564abb 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/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 5cd5755e01587..d18799016e853 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1366,6 +1366,15 @@ ToolChain::CXXStdlibType ToolChain::GetCXXStdlibType(const ArgList &Args) const{
   return *cxxStdlibType;
 }
 
+/// Utility function to add a system framework directory to CC1 arguments.
+/*static*/ void
+ToolChain::addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
+                                     llvm::opt::ArgStringList &CC1Args,
+                                     const Twine &Path) {
+  CC1Args.push_back("-internal-iframework");
+  CC1Args.push_back(DriverArgs.MakeArgString(Path));
+}
+
 /// Utility function to add a system include directory to CC1 arguments.
 /*static*/ void ToolChain::addSystemInclude(const ArgList &DriverArgs,
                                             ArgStringList &CC1Args,
@@ -1396,6 +1405,17 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
     addExternCSystemInclude(DriverArgs, CC1Args, Path);
 }
 
+/// Utility function to add a list of system framework directories to CC1.
+/*static*/ void
+ToolChain::addSystemFrameworkIncludes(const ArgList &DriverArgs,
+                                      ArgStringList &CC1Args,
+                                      ArrayRef<StringRef> Paths) {
+  for (const auto &Path : Paths) {
+    CC1Args.push_back("-internal-iframework");
+    CC1Args.push_back(DriverArgs.MakeArgString(Path));
+  }
+}
+
 /// Utility function to add a list of system include directories to CC1.
 /*static*/ void ToolChain::addSystemIncludes(const ArgList &DriverArgs,
                                              ArgStringList &CC1Args,
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 4735dc3ad30ee..76fa2d121db2a 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2577,6 +2577,27 @@ void AppleMachO::AddClangSystemIncludeArgs(
   }
 }
 
+void DarwinClang::AddClangSystemIncludeArgs(
+    const llvm::opt::ArgList &DriverArgs,
+    llvm::opt::ArgStringList &CC1Args) const {
+  AppleMachO::AddClangSystemIncludeArgs(DriverArgs, CC1Args);
+
+  if (DriverArgs.hasArg(options::OPT_nostdinc) ||
+      DriverArgs.hasArg(options::OPT_nostdlibinc))
+    return;
+
+  llvm::SmallString<128> Sysroot = GetEffectiveSysroot(DriverArgs);
+
+  // Add <sysroot>/System/Library/Frameworks
+  // Add <sysroot>/System/Library/SubFrameworks
+  // Add <sysroot>/Library/Frameworks
+  SmallString<128> P1(Sysroot), P2(Sysroot), P3(Sysroot);
+  llvm::sys::path::append(P1, "System", "Library", "Frameworks");
+  llvm::sys::path::append(P2, "System", "Library", "SubFrameworks");
+  llvm::sys::path::append(P3, "Library", "Frameworks");
+  addSystemFrameworkIncludes(DriverArgs, CC1Args, {P1, P2, P3});
+}
+
 bool DarwinClang::AddGnuCPlusPlusIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                               llvm::opt::ArgStringList &CC1Args,
                                               llvm::SmallString<128> Base,
diff --git a/clang/lib/Driver/ToolChains/Darwin.h b/clang/lib/Driver/ToolChains/Darwin.h
index 76523d636ce07..b38bfe6d1e554 100644
--- a/clang/lib/Driver/ToolChains/Darwin.h
+++ b/clang/lib/Driver/ToolChains/Darwin.h
@@ -647,6 +647,10 @@ class LLVM_LIBRARY_VISIBILITY DarwinClang : public Darwin {
   /// @name Apple ToolChain Implementation
   /// {
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+                            llvm::opt::ArgStringList &CC1Args) const override;
+
   RuntimeLibType GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
 
   void AddLinkRuntimeLibArgs(const llvm::opt::ArgList &Args,
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index c7d11e6027ccf..9ad4324be789b 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3364,6 +3364,8 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
                            : OPT_internal_externc_isystem;
     GenerateArg(Consumer, Opt, It->Path);
   }
+  for (; It < End && Matches(*It, {frontend::System}, true, true); ++It)
+    GenerateArg(Consumer, OPT_internal_iframework, It->Path);
 
   assert(It == End && "Unhandled HeaderSearchOption::Entry.");
 
@@ -3496,6 +3498,8 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
       Group = frontend::ExternCSystem;
     Opts.AddPath(A->getValue(), Group, false, true);
   }
+  for (const auto *A : Args.filtered(OPT_internal_iframework))
+    Opts.AddPath(A->getValue(), frontend::System, true, true);
 
   // Add the path prefixes which are implicitly treated as being system headers.
   for (const auto *A :
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index bb2a21356fa8f..df45c96725fe3 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -321,6 +321,9 @@ bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
     break;
   }
 
+  if (triple.isOSDarwin())
+    return false;
+
   return true; // Everything else uses AddDefaultIncludePaths().
 }
 
@@ -335,22 +338,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/Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks/.keep b/clang/test/Driver/Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibraryFrameworks/.keep b/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibraryFrameworks/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibrarySubFrameworks/.keep b/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibrarySubFrameworks/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
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 0000000000000..1cb4dc420c536
--- /dev/null
+++ b/clang/test/Driver/darwin-framework-search-paths.c
@@ -0,0 +1,23 @@
+// UNSUPPORTED: system-windows
+//   Windows is unsupported because we use the Unix path separator `/` in the test.
+
+// RUN: %clang %s -target arm64-apple-macosx15.1 -isysroot %S/Inputs/MacOSX15.1.sdk -c %s -### 2>&1 \
+// RUN: | FileCheck -DSDKROOT=%S/Inputs/MacOSX15.1.sdk %s
+//
+// CHECK: "-cc1"
+// CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]*]]"
+// CHECK-SAME: "-internal-iframework" "[[SDKROOT]]/System/Library/Frameworks"
+// CHECK-SAME: "-internal-iframework" "[[SDKROOT]]/System/Library/SubFrameworks"
+// CHECK-SAME: "-internal-iframework" "[[SDKROOT]]/Library/Frameworks"
+
+// Verify that -nostdlibinc and -nostdinc removes the default search paths.
+//
+// RUN: %clang %s -target arm64-apple-macosx15.1 -isysroot %S/Inputs/MacOSX15.1.sdk -nostdinc -c %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-NOSTD -DSDKROOT=%S/Inputs/MacOSX15.1.sdk %s
+//
+// RUN: %clang %s -target arm64-apple-macosx15.1 -isysroot %S/Inputs/MacOSX15.1.sdk -nostdlibinc -c %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-NOSTD -DSDKROOT=%S/Inputs/MacOSX15.1.sdk %s
+//
+// CHECK-NOSTD: "-cc1"
+// CHECK-NOSTD: "-resource-dir" "[[RESOURCE_DIR:[^"]*]]"
+// CHECK-NOSTD-NOT: "-internal-iframework"
diff --git a/clang/test/Driver/darwin-subframeworks.c b/clang/test/Driver/darwin-subframeworks.c
deleted file mode 100644
index 1a7a095c64292..0000000000000
--- 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/Driver/driverkit-path.c b/clang/test/Driver/driverkit-path.c
index 3caae382d65bb..32f0a6721ab77 100644
--- a/clang/test/Driver/driverkit-path.c
+++ b/clang/test/Driver/driverkit-path.c
@@ -21,13 +21,15 @@ int main() { return 0; }
 // LD64-NEW: "-isysroot" "[[PATH:[^"]*]]Inputs/DriverKit19.0.sdk"
 // LD64-NEW-NOT: "-L[[PATH]]Inputs/DriverKit19.0.sdk/System/DriverKit/usr/lib"
 // LD64-NEW-NOT: "-F[[PATH]]Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/Frameworks"
+// LD64-NEW-NOT: "-F[[PATH]]Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks"
 
 
-// RUN: %clang %s -target x86_64-apple-driverkit19.0 -isysroot %S/Inputs/DriverKit19.0.sdk -E -v -x c++ 2>&1 | FileCheck %s --check-prefix=INC
+// RUN: %clang %s -target x86_64-apple-driverkit19.0 -isysroot %S/Inputs/DriverKit19.0.sdk -x c++ -### 2>&1 \
+// RUN: | FileCheck %s -DSDKROOT=%S/Inputs/DriverKit19.0.sdk --check-prefix=INC
 //
-// INC:       -isysroot [[PATH:[^ ]*/Inputs/DriverKit19.0.sdk]]
-// INC-LABEL: #include <...> search starts here:
-// INC:       [[PATH]]/System/DriverKit/usr/local/include
-// INC:       /lib{{(64)?}}/clang/{{[^/ ]+}}/include
-// INC:       [[PATH]]/System/DriverKit/usr/include
-// INC:       [[PATH]]/System/DriverKit/System/Library/Frameworks (framework directory)
+// INC: "-isysroot" "[[SDKROOT]]"
+// INC: "-internal-isystem" "[[SDKROOT]]/System/DriverKit/usr/local/include"
+// INC: "-internal-isystem" "{{.+}}/lib{{(64)?}}/clang/{{[^/ ]+}}/include"
+// INC: "-internal-externc-isystem" "[[SDKROOT]]/System/DriverKit/usr/include"
+// INC: "-internal-iframework" "[[SDKROOT]]/System/DriverKit/System/Library/Frameworks"
+// INC: "-internal-iframework" "[[SDKROOT]]/System/DriverKit/System/Library/SubFrameworks"
diff --git a/clang/test/Preprocessor/cuda-macos-includes.cu b/clang/test/Preprocessor/cuda-macos-includes.cu
deleted file mode 100644
index 6ef94b0e45352..0000000000000
--- a/clang/test/Preprocessor/cuda-macos-includes.cu
+++ /dev/null
@@ -1,13 +0,0 @@
-// 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"
diff --git a/clang/unittests/Frontend/CMakeLists.txt b/clang/unittests/Frontend/CMakeLists.txt
index bbf0396014fa9..0d9fc714212d5 100644
--- a/clang/unittests/Frontend/CMakeLists.txt
+++ b/clang/unittests/Frontend/CMakeLists.txt
@@ -10,6 +10,7 @@ add_clang_unittest(FrontendTests
   PCHPreambleTest.cpp
   ReparseWorkingDirTest.cpp
   OutputStreamTest.cpp
+  SearchPathTest.cpp
   TextDiagnosticTest.cpp
   UtilsTest.cpp
   CLANG_LIBS
diff --git a/clang/unittests/Frontend/SearchPathTest.cpp b/clang/unittests/Frontend/SearchPathTest.cpp
new file mode 100644
index 0000000000000..5d382a4ee20a8
--- /dev/null
+++ b/clang/unittests/Frontend/SearchPathTest.cpp
@@ -0,0 +1,157 @@
+//====-- unittests/Frontend/SearchPathTest.cpp - FrontendAction tests -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/HeaderSearchOptions.h"
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Option/Option.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include "gtest/gtest.h"
+
+#include <memory>
+#include <optional>
+#include <tuple>
+#include <vector>
+
+namespace clang {
+namespace {
+
+class SearchPathTest : public ::testing::Test {
+protected:
+  SearchPathTest()
+      : Diags(new DiagnosticIDs(), new DiagnosticOptions,
+              new IgnoringDiagConsumer()),
+        VFS(new llvm::vfs::InMemoryFileSystem),
+        FileMgr(FileSystemOptions(), VFS), SourceMgr(Diags, FileMgr),
+        Invocation(std::make_unique<CompilerInvocation>()) {}
+
+  DiagnosticsEngine Diags;
+  IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS;
+  FileManager FileMgr;
+  SourceManager SourceMgr;
+  std::unique_ptr<CompilerInvocation> Invocation;
+
+  void addDirectories(ArrayRef<StringRef> Dirs) {
+    for (StringRef Dir : Dirs) {
+      VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""),
+                   /*User=*/std::nullopt, /*Group=*/std::nullopt,
+                   llvm::sys::fs::file_type::directory_file);
+    }
+  }
+
+  std::unique_ptr<HeaderSearch>
+  makeHeaderSearchFromCC1Args(llvm::opt::ArgStringList Args) {
+    CompilerInvocation::CreateFromArgs(*Invocation, Args, Diags);
+    HeaderSearchOptions HSOpts = Invocation->getHeaderSearchOpts();
+    LangOptions LangOpts = Invocation->getLangOpts();
+    TargetInfo *Target =
+        TargetInfo::CreateTargetInfo(Diags, Invocation->getTargetOpts());
+    auto HeaderInfo = std::make_unique<HeaderSearch>(HSOpts, SourceMgr, Diags,
+                                                     LangOpts, Target);
+    ApplyHeaderSearchOptions(*HeaderInfo, HSOpts, LangOpts,
+                             Target->getTriple());
+    return HeaderInfo;
+  }
+};
+
+TEST_F(SearchPathTest, SearchPathOrder) {
+  addDirectories({"One", "Two", "Three", "Four", "Five", "Six", "Seven",
+                  "Eight", "Nine", "Ten", "Eleven", "Twelve", "Thirteen",
+                  "Fourteen", "Fifteen", "Sixteen", "Seventeen"});
+  llvm::opt::ArgStringList Args = {
+      // Make sure to use a triple and language that don't automatically add any
+      // search paths.
+      "-triple", "arm64-apple-darwin24.4.0", "-x", "c",
+
+      // clang-format off
+      "-internal-isystem", "One",
+      "-iwithsysroot", "Two",
+      "-c-isystem", "Three",
+      "-IFour",
+      "-idirafter", "Five",
+      "-internal-externc-isystem", "Six",
+      "-iwithprefix", "Seven",
+      "-FEight",
+      "-idirafter", "Nine",
+      "-iframeworkwithsysroot", "Ten",
+      "-internal-iframework", "Eleven",
+      "-iframework", "Twelve",
+      "-iwithprefixbefore", "Thirteen",
+      "-internal-isystem", "Fourteen",
+      "-isystem", "Fifteen",
+      "-ISixteen",
+      "-iwithsysroot", "Seventeen",
+      // clang-format on
+  };
+
+  // The search path arguments get categorized by IncludeDirGroup, but
+  // ultimately are sorted with some groups mixed together and some flags sorted
+  // very specifically within their group. The conceptual groups below don't
+  // exactly correspond to IncludeDirGroup.
+  const std::vector<StringRef> expected = {
+      // User paths: -I and -F mixed together, -iwithprefixbefore.
+      /*-I*/ "Four",
+      /*-F*/ "Eight",
+      /*-I*/ "Sixteen",
+      /*-iwithprefixbefore*/ "Thirteen",
+
+      // System paths: -isystem and -iwithsysroot, -iframework,
+      // -iframeworkwithsysroot, one of {-c-isystem, -cxx-isystem,
+      // -objc-isystem, -objcxx-isystem}
+      /*-iwithsysroot*/ "Two",
+      /*-isystem*/ "Fifteen",
+      /*-iwithsysroot*/ "Seventeen",
+      /*-iframework*/ "Twelve",
+      /*...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented May 2, 2025

@llvm/pr-subscribers-clang-driver

Author: Ian Anderson (ian-twilightcoder)

Changes

Move the Darwin framework search path logic from InitHeaderSearch::AddDefaultIncludePaths to DarwinClang::AddClangSystemIncludeArgs. Add a new -internal-iframework cc1 argument to support the tool chain adding these paths. Now that the tool chain is adding search paths via cc1 flag, they're only added if they exist, so the Preprocessor/cuda-macos-includes.cu test is no longer relevant. Change Driver/driverkit-path.c and Driver/darwin-subframeworks.c to do -### style testing similar to the darwin-header-search and darwin-embedded-search-paths tests. Rename darwin-subframeworks.c to darwin-framework-search-paths.c and have it test all framework search paths, not just SubFrameworks. Add a unit test to validate that the myriad of search path flags result in the expected search path list.

Fixes #75638


Patch is 21.07 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/138234.diff

17 Files Affected:

  • (modified) clang/include/clang/Driver/Options.td (+5)
  • (modified) clang/include/clang/Driver/ToolChain.h (+6)
  • (modified) clang/lib/Driver/Job.cpp (+1-1)
  • (modified) clang/lib/Driver/ToolChain.cpp (+20)
  • (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+21)
  • (modified) clang/lib/Driver/ToolChains/Darwin.h (+4)
  • (modified) clang/lib/Frontend/CompilerInvocation.cpp (+4)
  • (modified) clang/lib/Lex/InitHeaderSearch.cpp (+3-16)
  • (added) clang/test/Driver/Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks/.keep ()
  • (added) clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibraryFrameworks/.keep ()
  • (added) clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibrarySubFrameworks/.keep ()
  • (added) clang/test/Driver/darwin-framework-search-paths.c (+23)
  • (removed) clang/test/Driver/darwin-subframeworks.c (-18)
  • (modified) clang/test/Driver/driverkit-path.c (+9-7)
  • (removed) clang/test/Preprocessor/cuda-macos-includes.cu (-13)
  • (modified) clang/unittests/Frontend/CMakeLists.txt (+1)
  • (added) clang/unittests/Frontend/SearchPathTest.cpp (+157)
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 561b0498c549c..7d2b847dd3830 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8413,6 +8413,11 @@ def objc_isystem : Separate<["-"], "objc-isystem">,
 def objcxx_isystem : Separate<["-"], "objcxx-isystem">,
   MetaVarName<"<directory>">,
   HelpText<"Add directory to the ObjC++ SYSTEM include search path">;
+def internal_iframework : Separate<["-"], "internal-iframework">,
+  MetaVarName<"<directory>">,
+  HelpText<"Add directory to the internal system framework search path; these "
+           "are assumed to not be user-provided and are used to model system "
+           "and standard frameworks' paths.">;
 def internal_isystem : Separate<["-"], "internal-isystem">,
   MetaVarName<"<directory>">,
   HelpText<"Add directory to the internal system include search path; these "
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index d0059673d6a67..58edf2b3887b0 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -226,6 +226,9 @@ class ToolChain {
 
   /// \name Utilities for implementing subclasses.
   ///@{
+  static void addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
+                                        llvm::opt::ArgStringList &CC1Args,
+                                        const Twine &Path);
   static void addSystemInclude(const llvm::opt::ArgList &DriverArgs,
                                llvm::opt::ArgStringList &CC1Args,
                                const Twine &Path);
@@ -236,6 +239,9 @@ class ToolChain {
       addExternCSystemIncludeIfExists(const llvm::opt::ArgList &DriverArgs,
                                       llvm::opt::ArgStringList &CC1Args,
                                       const Twine &Path);
+  static void addSystemFrameworkIncludes(const llvm::opt::ArgList &DriverArgs,
+                                         llvm::opt::ArgStringList &CC1Args,
+                                         ArrayRef<StringRef> Paths);
   static void addSystemIncludes(const llvm::opt::ArgList &DriverArgs,
                                 llvm::opt::ArgStringList &CC1Args,
                                 ArrayRef<StringRef> Paths);
diff --git a/clang/lib/Driver/Job.cpp b/clang/lib/Driver/Job.cpp
index 4619b8c1887be..60f219f564abb 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/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 5cd5755e01587..d18799016e853 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1366,6 +1366,15 @@ ToolChain::CXXStdlibType ToolChain::GetCXXStdlibType(const ArgList &Args) const{
   return *cxxStdlibType;
 }
 
+/// Utility function to add a system framework directory to CC1 arguments.
+/*static*/ void
+ToolChain::addSystemFrameworkInclude(const llvm::opt::ArgList &DriverArgs,
+                                     llvm::opt::ArgStringList &CC1Args,
+                                     const Twine &Path) {
+  CC1Args.push_back("-internal-iframework");
+  CC1Args.push_back(DriverArgs.MakeArgString(Path));
+}
+
 /// Utility function to add a system include directory to CC1 arguments.
 /*static*/ void ToolChain::addSystemInclude(const ArgList &DriverArgs,
                                             ArgStringList &CC1Args,
@@ -1396,6 +1405,17 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
     addExternCSystemInclude(DriverArgs, CC1Args, Path);
 }
 
+/// Utility function to add a list of system framework directories to CC1.
+/*static*/ void
+ToolChain::addSystemFrameworkIncludes(const ArgList &DriverArgs,
+                                      ArgStringList &CC1Args,
+                                      ArrayRef<StringRef> Paths) {
+  for (const auto &Path : Paths) {
+    CC1Args.push_back("-internal-iframework");
+    CC1Args.push_back(DriverArgs.MakeArgString(Path));
+  }
+}
+
 /// Utility function to add a list of system include directories to CC1.
 /*static*/ void ToolChain::addSystemIncludes(const ArgList &DriverArgs,
                                              ArgStringList &CC1Args,
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 4735dc3ad30ee..76fa2d121db2a 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2577,6 +2577,27 @@ void AppleMachO::AddClangSystemIncludeArgs(
   }
 }
 
+void DarwinClang::AddClangSystemIncludeArgs(
+    const llvm::opt::ArgList &DriverArgs,
+    llvm::opt::ArgStringList &CC1Args) const {
+  AppleMachO::AddClangSystemIncludeArgs(DriverArgs, CC1Args);
+
+  if (DriverArgs.hasArg(options::OPT_nostdinc) ||
+      DriverArgs.hasArg(options::OPT_nostdlibinc))
+    return;
+
+  llvm::SmallString<128> Sysroot = GetEffectiveSysroot(DriverArgs);
+
+  // Add <sysroot>/System/Library/Frameworks
+  // Add <sysroot>/System/Library/SubFrameworks
+  // Add <sysroot>/Library/Frameworks
+  SmallString<128> P1(Sysroot), P2(Sysroot), P3(Sysroot);
+  llvm::sys::path::append(P1, "System", "Library", "Frameworks");
+  llvm::sys::path::append(P2, "System", "Library", "SubFrameworks");
+  llvm::sys::path::append(P3, "Library", "Frameworks");
+  addSystemFrameworkIncludes(DriverArgs, CC1Args, {P1, P2, P3});
+}
+
 bool DarwinClang::AddGnuCPlusPlusIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                               llvm::opt::ArgStringList &CC1Args,
                                               llvm::SmallString<128> Base,
diff --git a/clang/lib/Driver/ToolChains/Darwin.h b/clang/lib/Driver/ToolChains/Darwin.h
index 76523d636ce07..b38bfe6d1e554 100644
--- a/clang/lib/Driver/ToolChains/Darwin.h
+++ b/clang/lib/Driver/ToolChains/Darwin.h
@@ -647,6 +647,10 @@ class LLVM_LIBRARY_VISIBILITY DarwinClang : public Darwin {
   /// @name Apple ToolChain Implementation
   /// {
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+                            llvm::opt::ArgStringList &CC1Args) const override;
+
   RuntimeLibType GetRuntimeLibType(const llvm::opt::ArgList &Args) const override;
 
   void AddLinkRuntimeLibArgs(const llvm::opt::ArgList &Args,
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index c7d11e6027ccf..9ad4324be789b 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -3364,6 +3364,8 @@ static void GenerateHeaderSearchArgs(const HeaderSearchOptions &Opts,
                            : OPT_internal_externc_isystem;
     GenerateArg(Consumer, Opt, It->Path);
   }
+  for (; It < End && Matches(*It, {frontend::System}, true, true); ++It)
+    GenerateArg(Consumer, OPT_internal_iframework, It->Path);
 
   assert(It == End && "Unhandled HeaderSearchOption::Entry.");
 
@@ -3496,6 +3498,8 @@ static bool ParseHeaderSearchArgs(HeaderSearchOptions &Opts, ArgList &Args,
       Group = frontend::ExternCSystem;
     Opts.AddPath(A->getValue(), Group, false, true);
   }
+  for (const auto *A : Args.filtered(OPT_internal_iframework))
+    Opts.AddPath(A->getValue(), frontend::System, true, true);
 
   // Add the path prefixes which are implicitly treated as being system headers.
   for (const auto *A :
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index bb2a21356fa8f..df45c96725fe3 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -321,6 +321,9 @@ bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
     break;
   }
 
+  if (triple.isOSDarwin())
+    return false;
+
   return true; // Everything else uses AddDefaultIncludePaths().
 }
 
@@ -335,22 +338,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/Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks/.keep b/clang/test/Driver/Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibraryFrameworks/.keep b/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibraryFrameworks/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
diff --git a/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibrarySubFrameworks/.keep b/clang/test/Driver/Inputs/MacOSX15.1.sdk/System/LibrarySubFrameworks/.keep
new file mode 100644
index 0000000000000..e69de29bb2d1d
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 0000000000000..1cb4dc420c536
--- /dev/null
+++ b/clang/test/Driver/darwin-framework-search-paths.c
@@ -0,0 +1,23 @@
+// UNSUPPORTED: system-windows
+//   Windows is unsupported because we use the Unix path separator `/` in the test.
+
+// RUN: %clang %s -target arm64-apple-macosx15.1 -isysroot %S/Inputs/MacOSX15.1.sdk -c %s -### 2>&1 \
+// RUN: | FileCheck -DSDKROOT=%S/Inputs/MacOSX15.1.sdk %s
+//
+// CHECK: "-cc1"
+// CHECK: "-resource-dir" "[[RESOURCE_DIR:[^"]*]]"
+// CHECK-SAME: "-internal-iframework" "[[SDKROOT]]/System/Library/Frameworks"
+// CHECK-SAME: "-internal-iframework" "[[SDKROOT]]/System/Library/SubFrameworks"
+// CHECK-SAME: "-internal-iframework" "[[SDKROOT]]/Library/Frameworks"
+
+// Verify that -nostdlibinc and -nostdinc removes the default search paths.
+//
+// RUN: %clang %s -target arm64-apple-macosx15.1 -isysroot %S/Inputs/MacOSX15.1.sdk -nostdinc -c %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-NOSTD -DSDKROOT=%S/Inputs/MacOSX15.1.sdk %s
+//
+// RUN: %clang %s -target arm64-apple-macosx15.1 -isysroot %S/Inputs/MacOSX15.1.sdk -nostdlibinc -c %s -### 2>&1 \
+// RUN: | FileCheck --check-prefix=CHECK-NOSTD -DSDKROOT=%S/Inputs/MacOSX15.1.sdk %s
+//
+// CHECK-NOSTD: "-cc1"
+// CHECK-NOSTD: "-resource-dir" "[[RESOURCE_DIR:[^"]*]]"
+// CHECK-NOSTD-NOT: "-internal-iframework"
diff --git a/clang/test/Driver/darwin-subframeworks.c b/clang/test/Driver/darwin-subframeworks.c
deleted file mode 100644
index 1a7a095c64292..0000000000000
--- 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/Driver/driverkit-path.c b/clang/test/Driver/driverkit-path.c
index 3caae382d65bb..32f0a6721ab77 100644
--- a/clang/test/Driver/driverkit-path.c
+++ b/clang/test/Driver/driverkit-path.c
@@ -21,13 +21,15 @@ int main() { return 0; }
 // LD64-NEW: "-isysroot" "[[PATH:[^"]*]]Inputs/DriverKit19.0.sdk"
 // LD64-NEW-NOT: "-L[[PATH]]Inputs/DriverKit19.0.sdk/System/DriverKit/usr/lib"
 // LD64-NEW-NOT: "-F[[PATH]]Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/Frameworks"
+// LD64-NEW-NOT: "-F[[PATH]]Inputs/DriverKit19.0.sdk/System/DriverKit/System/Library/SubFrameworks"
 
 
-// RUN: %clang %s -target x86_64-apple-driverkit19.0 -isysroot %S/Inputs/DriverKit19.0.sdk -E -v -x c++ 2>&1 | FileCheck %s --check-prefix=INC
+// RUN: %clang %s -target x86_64-apple-driverkit19.0 -isysroot %S/Inputs/DriverKit19.0.sdk -x c++ -### 2>&1 \
+// RUN: | FileCheck %s -DSDKROOT=%S/Inputs/DriverKit19.0.sdk --check-prefix=INC
 //
-// INC:       -isysroot [[PATH:[^ ]*/Inputs/DriverKit19.0.sdk]]
-// INC-LABEL: #include <...> search starts here:
-// INC:       [[PATH]]/System/DriverKit/usr/local/include
-// INC:       /lib{{(64)?}}/clang/{{[^/ ]+}}/include
-// INC:       [[PATH]]/System/DriverKit/usr/include
-// INC:       [[PATH]]/System/DriverKit/System/Library/Frameworks (framework directory)
+// INC: "-isysroot" "[[SDKROOT]]"
+// INC: "-internal-isystem" "[[SDKROOT]]/System/DriverKit/usr/local/include"
+// INC: "-internal-isystem" "{{.+}}/lib{{(64)?}}/clang/{{[^/ ]+}}/include"
+// INC: "-internal-externc-isystem" "[[SDKROOT]]/System/DriverKit/usr/include"
+// INC: "-internal-iframework" "[[SDKROOT]]/System/DriverKit/System/Library/Frameworks"
+// INC: "-internal-iframework" "[[SDKROOT]]/System/DriverKit/System/Library/SubFrameworks"
diff --git a/clang/test/Preprocessor/cuda-macos-includes.cu b/clang/test/Preprocessor/cuda-macos-includes.cu
deleted file mode 100644
index 6ef94b0e45352..0000000000000
--- a/clang/test/Preprocessor/cuda-macos-includes.cu
+++ /dev/null
@@ -1,13 +0,0 @@
-// 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"
diff --git a/clang/unittests/Frontend/CMakeLists.txt b/clang/unittests/Frontend/CMakeLists.txt
index bbf0396014fa9..0d9fc714212d5 100644
--- a/clang/unittests/Frontend/CMakeLists.txt
+++ b/clang/unittests/Frontend/CMakeLists.txt
@@ -10,6 +10,7 @@ add_clang_unittest(FrontendTests
   PCHPreambleTest.cpp
   ReparseWorkingDirTest.cpp
   OutputStreamTest.cpp
+  SearchPathTest.cpp
   TextDiagnosticTest.cpp
   UtilsTest.cpp
   CLANG_LIBS
diff --git a/clang/unittests/Frontend/SearchPathTest.cpp b/clang/unittests/Frontend/SearchPathTest.cpp
new file mode 100644
index 0000000000000..5d382a4ee20a8
--- /dev/null
+++ b/clang/unittests/Frontend/SearchPathTest.cpp
@@ -0,0 +1,157 @@
+//====-- unittests/Frontend/SearchPathTest.cpp - FrontendAction tests -----===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemOptions.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/HeaderSearchOptions.h"
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Option/Option.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
+
+#include "gtest/gtest.h"
+
+#include <memory>
+#include <optional>
+#include <tuple>
+#include <vector>
+
+namespace clang {
+namespace {
+
+class SearchPathTest : public ::testing::Test {
+protected:
+  SearchPathTest()
+      : Diags(new DiagnosticIDs(), new DiagnosticOptions,
+              new IgnoringDiagConsumer()),
+        VFS(new llvm::vfs::InMemoryFileSystem),
+        FileMgr(FileSystemOptions(), VFS), SourceMgr(Diags, FileMgr),
+        Invocation(std::make_unique<CompilerInvocation>()) {}
+
+  DiagnosticsEngine Diags;
+  IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS;
+  FileManager FileMgr;
+  SourceManager SourceMgr;
+  std::unique_ptr<CompilerInvocation> Invocation;
+
+  void addDirectories(ArrayRef<StringRef> Dirs) {
+    for (StringRef Dir : Dirs) {
+      VFS->addFile(Dir, 0, llvm::MemoryBuffer::getMemBuffer(""),
+                   /*User=*/std::nullopt, /*Group=*/std::nullopt,
+                   llvm::sys::fs::file_type::directory_file);
+    }
+  }
+
+  std::unique_ptr<HeaderSearch>
+  makeHeaderSearchFromCC1Args(llvm::opt::ArgStringList Args) {
+    CompilerInvocation::CreateFromArgs(*Invocation, Args, Diags);
+    HeaderSearchOptions HSOpts = Invocation->getHeaderSearchOpts();
+    LangOptions LangOpts = Invocation->getLangOpts();
+    TargetInfo *Target =
+        TargetInfo::CreateTargetInfo(Diags, Invocation->getTargetOpts());
+    auto HeaderInfo = std::make_unique<HeaderSearch>(HSOpts, SourceMgr, Diags,
+                                                     LangOpts, Target);
+    ApplyHeaderSearchOptions(*HeaderInfo, HSOpts, LangOpts,
+                             Target->getTriple());
+    return HeaderInfo;
+  }
+};
+
+TEST_F(SearchPathTest, SearchPathOrder) {
+  addDirectories({"One", "Two", "Three", "Four", "Five", "Six", "Seven",
+                  "Eight", "Nine", "Ten", "Eleven", "Twelve", "Thirteen",
+                  "Fourteen", "Fifteen", "Sixteen", "Seventeen"});
+  llvm::opt::ArgStringList Args = {
+      // Make sure to use a triple and language that don't automatically add any
+      // search paths.
+      "-triple", "arm64-apple-darwin24.4.0", "-x", "c",
+
+      // clang-format off
+      "-internal-isystem", "One",
+      "-iwithsysroot", "Two",
+      "-c-isystem", "Three",
+      "-IFour",
+      "-idirafter", "Five",
+      "-internal-externc-isystem", "Six",
+      "-iwithprefix", "Seven",
+      "-FEight",
+      "-idirafter", "Nine",
+      "-iframeworkwithsysroot", "Ten",
+      "-internal-iframework", "Eleven",
+      "-iframework", "Twelve",
+      "-iwithprefixbefore", "Thirteen",
+      "-internal-isystem", "Fourteen",
+      "-isystem", "Fifteen",
+      "-ISixteen",
+      "-iwithsysroot", "Seventeen",
+      // clang-format on
+  };
+
+  // The search path arguments get categorized by IncludeDirGroup, but
+  // ultimately are sorted with some groups mixed together and some flags sorted
+  // very specifically within their group. The conceptual groups below don't
+  // exactly correspond to IncludeDirGroup.
+  const std::vector<StringRef> expected = {
+      // User paths: -I and -F mixed together, -iwithprefixbefore.
+      /*-I*/ "Four",
+      /*-F*/ "Eight",
+      /*-I*/ "Sixteen",
+      /*-iwithprefixbefore*/ "Thirteen",
+
+      // System paths: -isystem and -iwithsysroot, -iframework,
+      // -iframeworkwithsysroot, one of {-c-isystem, -cxx-isystem,
+      // -objc-isystem, -objcxx-isystem}
+      /*-iwithsysroot*/ "Two",
+      /*-isystem*/ "Fifteen",
+      /*-iwithsysroot*/ "Seventeen",
+      /*-iframework*/ "Twelve",
+      /*...
[truncated]

@ian-twilightcoder
Copy link
Contributor Author

This is a takeover of #120149 because Louis is out, I'm not a maintainer so I can't push to his fork, and I redid most of it anyway.

…tend

Move the Darwin framework search path logic from InitHeaderSearch::AddDefaultIncludePaths to DarwinClang::AddClangSystemIncludeArgs. Add a new -internal-iframework cc1 argument to support the tool chain adding these paths.
Now that the tool chain is adding search paths via cc1 flag, they're only added if they exist, so the Preprocessor/cuda-macos-includes.cu test is no longer relevant.
Change Driver/driverkit-path.c and Driver/darwin-subframeworks.c to do -### style testing similar to the darwin-header-search and darwin-embedded-search-paths tests. Rename darwin-subframeworks.c to darwin-framework-search-paths.c and have it test all framework search paths, not just SubFrameworks.
Add a unit test to validate that the myriad of search path flags result in the expected search path list.

Fixes llvm#75638
@ian-twilightcoder ian-twilightcoder force-pushed the darwin-framework-search-paths branch from 59f8d68 to 6c55939 Compare May 2, 2025 06:01
@@ -1366,6 +1366,15 @@ ToolChain::CXXStdlibType ToolChain::GetCXXStdlibType(const ArgList &Args) const{
return *cxxStdlibType;
}

/// Utility function to add a system framework directory to CC1 arguments.
/*static*/ void
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/*static*/ void
void

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this from addSystemInclude for consistency. I can still change it you think it looks dumb.

@@ -1396,6 +1405,17 @@ void ToolChain::addExternCSystemIncludeIfExists(const ArgList &DriverArgs,
addExternCSystemInclude(DriverArgs, CC1Args, Path);
}

/// Utility function to add a list of system framework directories to CC1.
/*static*/ void
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/*static*/ void
void

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here from addSystemIncludes.

@@ -1,13 +0,0 @@
// RUN: %clang -cc1 -fcuda-is-device -isysroot /var/empty \
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this test but set the -isysroot to %S/Inputs/MacOSX15.1.sdk ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need it because the ignoring nonexistent directory are no longer produced. Those paths get pruned before they reach the HeaderSearch now. The old code unconditionally added those directly, the new code goes through the cc1 arguments and those get checked if they exist before they're added to the HeaderSearch.

llvm::opt::ArgStringList &CC1Args) const {
AppleMachO::AddClangSystemIncludeArgs(DriverArgs, CC1Args);

if (DriverArgs.hasArg(options::OPT_nostdinc) ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a change in behavior. Previoiusly -Xclang -nostdsysteminc would additionally remove the framework search paths. However, I wanted to make the default framework include paths be consistent with the default header include paths. I don't believe there's a practical use case to keep the default header search paths but remove the default framework search paths, which is what -Xclang -nostdsysteminc would do. I think the previous code used HeaderSearchOptions.UseStandardSystemIncludes because that's all you get at that level of the code.

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:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Driver][Darwin] Move the remaining bits for header path handling over to the Driver.
3 participants