[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 llvm#75841
which was reverted in d34901f 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 llvm#75638

Author: ldionne

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]
 

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;

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,

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.

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) {

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)

clang/test/Driver/darwin-subframeworks.c

@@ -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"