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.

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

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


3 Files Affected:

- (modified) clang/lib/Driver/ToolChains/Darwin.cpp (+22-13) 
- (modified) clang/lib/Lex/InitHeaderSearch.cpp (+3-15) 
- (modified) clang/test/Driver/driverkit-path.c (-1) 


``````````diff
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)

``````````

</details>


https://github.com/llvm/llvm-project/pull/75841
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to