[clang] [clang][Darwin] Remove legacy framework search path logic in the frontend (PR #75841)

Louis Dionne via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 18 14:39:50 PST 2023


https://github.com/ldionne updated https://github.com/llvm/llvm-project/pull/75841

>From ca2a85880cec704225c0b1d5241cf299d5dcad41 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 18 Dec 2023 12:53:23 -0500
Subject: [PATCH 1/2] [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.

This is intended to be a pure refactor without any functional change
visible to users, since the search paths before and after should be the
same, and in the same order. The change in the tests is necessary because
we would previously add the DriverKit framework search path in the frontend
regardless of whether we actually need to, which we now handle correctly
because the driver checks for ld64-605.1+.

Fixes #75638
---
 clang/lib/Driver/ToolChains/Darwin.cpp | 35 ++++++++++++++++----------
 clang/lib/Lex/InitHeaderSearch.cpp     | 18 +++----------
 clang/test/Driver/driverkit-path.c     |  1 -
 3 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 65846cace461e3..f76a42d2d8e7e3 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -758,9 +758,14 @@ 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>/Library/Frameworks
   {
     bool NonStandardSearchPath = false;
     const auto &Triple = getToolChain().getTriple();
@@ -771,18 +776,22 @@ 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", "/Library/Frameworks");
       }
     }
   }
diff --git a/clang/lib/Lex/InitHeaderSearch.cpp b/clang/lib/Lex/InitHeaderSearch.cpp
index 2218db15013d92..1350fa5f01a578 100644
--- a/clang/lib/Lex/InitHeaderSearch.cpp
+++ b/clang/lib/Lex/InitHeaderSearch.cpp
@@ -324,6 +324,9 @@ bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
     break;
   }
 
+  if (triple.isOSDarwin())
+    return false;
+
   return true; // Everything else uses AddDefaultIncludePaths().
 }
 
@@ -338,21 +341,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("/Library/Frameworks", System, true);
-      }
-    }
-    return;
-  }
-
   if (Lang.CPlusPlus && !Lang.AsmPreprocessor &&
       HSOpts.UseStandardCXXIncludes && HSOpts.UseStandardSystemIncludes) {
     if (HSOpts.UseLibcxx) {
diff --git a/clang/test/Driver/driverkit-path.c b/clang/test/Driver/driverkit-path.c
index 9699b9c01f4e81..43e5aa40fc6f31 100644
--- a/clang/test/Driver/driverkit-path.c
+++ b/clang/test/Driver/driverkit-path.c
@@ -31,4 +31,3 @@ int main() { return 0; }
 // 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)

>From ee38b41f9bd3c6549d4d5bf38c3ff734e3fe9091 Mon Sep 17 00:00:00 2001
From: Louis Dionne <ldionne.2 at gmail.com>
Date: Mon, 18 Dec 2023 17:39:40 -0500
Subject: [PATCH 2/2] Tentatively remove cuda test that is not relevant anymore
 since the linker on Apple platforms already handles these paths

---
 clang/test/Preprocessor/cuda-macos-includes.cu | 13 -------------
 1 file changed, 13 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 6ef94b0e453520..00000000000000
--- 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"



More information about the cfe-commits mailing list