[Driver] move FreeBSD header search path management to the driver

[mhjacobson]

Summary

This matches OpenBSD, and it supports Swift's use of clang for its C interop functionality. Recent changes to Swift use AddClangSystemIncludeArgs() to inspect the cc1 args; this doesn't work for platforms where cc1 adds standard include paths implicitly.

Also, clean up InitHeaderSearch, making it clearer which targets manage header search paths in the driver.

Add test.

[mhjacobson]

Added test.

[mhjacobson]

Run clang-format.

[mhjacobson]

Clarify comment.

[MaskRay]

clang/lib/Driver/ToolChains/FreeBSD.cpp

442

I think Fuchsia way of checking if (!D.SysRoot.empty()) { before adding /usr/include is probably better. Is it /include or /usr/include ?

clang/lib/Lex/InitHeaderSearch.cpp

103

Drop ShouldAddDefaultIncludePaths - . It is not recommended for new code.

104

multiple spaces => one space

clang/test/Driver/freebsd.c

217 ↗

(On Diff #476018)

Is this sufficient? For Linux linux-cross.cpp checks a lot of directories to facilitate code refactoring.

[mhjacobson]

clang/lib/Driver/ToolChains/FreeBSD.cpp

442

It's /usr/include on FreeBSD.

I'm confused by the Fuchsia code—does it not add /include if the sysroot is empty? Appears not...

$ clang-15 -### -c -target arm64-fuchsia -xc /dev/null         
clang version 15.0.3
Target: arm64-unknown-fuchsia
Thread model: posix
InstalledDir: /opt/local/libexec/llvm-15/bin
 (in-process)
 "/opt/local/libexec/llvm-15/bin/clang" "-cc1" "-triple" "arm64-unknown-fuchsia" "-emit-obj" "-mrelax-all" "--mrelax-relocations" "-disable-free" "-clear-ast-before-backend" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "null" "-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie" "-mframe-pointer=non-leaf" "-ffp-contract=on" "-fno-rounding-math" "-mconstructor-aliases" "-funwind-tables=2" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8a" "-target-abi" "aapcs" "-fallow-half-arguments-and-returns" "-mllvm" "-treat-scalable-fixed-error-as-warning" "-debugger-tuning=gdb" "-target-linker-version" "556.6" "-fcoverage-compilation-dir=/Users/matt" "-resource-dir" "/opt/local/libexec/llvm-15/lib/clang/15.0.3" "-I/usr/local/include" "-internal-isystem" "/opt/local/libexec/llvm-15/lib/clang/15.0.3/include" "-fdebug-compilation-dir=/Users/matt" "-ferror-limit" "19" "-fsanitize=shadow-call-stack" "-stack-protector" "2" "-fno-signed-char" "-fgnuc-version=4.2.1" "-fcolor-diagnostics" "-target-feature" "+outline-atomics" "-faddrsig" "-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o" "null.o" "-x" "c" "/dev/null"

$ clang-15 -### -c -target arm64-fuchsia --sysroot /foo -xc /dev/null
clang version 15.0.3
Target: arm64-unknown-fuchsia
Thread model: posix
InstalledDir: /opt/local/libexec/llvm-15/bin
 (in-process)
 "/opt/local/libexec/llvm-15/bin/clang" "-cc1" "-triple" "arm64-unknown-fuchsia" "-emit-obj" "-mrelax-all" "--mrelax-relocations" "-disable-free" "-clear-ast-before-backend" "-disable-llvm-verifier" "-discard-value-names" "-main-file-name" "null" "-mrelocation-model" "pic" "-pic-level" "2" "-pic-is-pie" "-mframe-pointer=non-leaf" "-ffp-contract=on" "-fno-rounding-math" "-mconstructor-aliases" "-funwind-tables=2" "-target-cpu" "generic" "-target-feature" "+neon" "-target-feature" "+v8a" "-target-abi" "aapcs" "-fallow-half-arguments-and-returns" "-mllvm" "-treat-scalable-fixed-error-as-warning" "-debugger-tuning=gdb" "-target-linker-version" "556.6" "-fcoverage-compilation-dir=/Users/matt" "-resource-dir" "/opt/local/libexec/llvm-15/lib/clang/15.0.3" "-isysroot" "/foo" "-internal-isystem" "/opt/local/libexec/llvm-15/lib/clang/15.0.3/include" "-internal-externc-isystem" "/foo/include" "-fdebug-compilation-dir=/Users/matt" "-ferror-limit" "19" "-fsanitize=shadow-call-stack" "-stack-protector" "2" "-fno-signed-char" "-fgnuc-version=4.2.1" "-fcolor-diagnostics" "-target-feature" "+outline-atomics" "-faddrsig" "-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o" "null.o" "-x" "c" "/dev/null"

I don't know anything about Fuchsia, but isn't that wrong? Certainly for FreeBSD we'd want to add an unprefixed /usr/include normally...

[mhjacobson]

clang/test/Driver/freebsd.c

217 ↗

(On Diff #476018)

On FreeBSD, the only directories included by default are

• /usr/include/c++/v1 (in C++ mode)
• <resource dir>/include
• /usr/include

I suppose I can add checks for the first two.

[mhjacobson]

Test for more paths; test C++.

[mhjacobson]

Fix a couple warnings.

[MaskRay]

clang/lib/Lex/InitHeaderSearch.cpp

104

not done. /// => ///

234

drop braces around simple single statements

391

move the variable immediately before switch os and move the comment before the variable? Avoid unneeded blank lines

clang/test/Driver/freebsd.cpp

48 ↗

(On Diff #476042)

Place "-internal-isystem on the same line to ensure there are no extra includes in between.

Alternatively, use the -NEXT: {{^}} xxx pattern I use in linux-cross.cpp

[mhjacobson]

Style fixes.

Ensure include paths are sequential.

[mhjacobson]

Make the tests play nice with Windows path separators.

[MaskRay]

This matches OpenBSD, and it supports Swift's use of clang for its C interop functionality. Recent changes to Swift use AddClangSystemIncludeArgs() to inspect the cc1 args; this doesn't work for platforms where cc1 adds standard include paths implicitly.

Use a commit link if known.

Also, clean up InitHeaderSearch, making it clearer which targets manage header search paths in the driver.

Add test.

"Add test" is unneeded in the commit message:)

clang/lib/Driver/ToolChains/FreeBSD.h

 public:
   FreeBSD(const Driver &D, const llvm::Triple &Triple,
           const llvm::opt::ArgList &Args);
   bool HasNativeLLVMSupport() const override;
 
   bool IsMathErrnoDefault() const override { return false; }
   bool IsObjCNonFragileABIDefault() const override { return true; }
 
+  void
+  AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+                            llvm::opt::ArgStringList &CC1Args) const override;
   CXXStdlibType GetDefaultCXXStdlibType() const override;
   void addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                              llvm::opt::ArgStringList &CC1Args) const override;
   void
   addLibStdCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                            llvm::opt::ArgStringList &CC1Args) const override;
   void AddCXXStdlibLibArgs(const llvm::opt::ArgList &Args,
                            llvm::opt::ArgStringList &CmdArgs) const override;

clang/lib/Driver/ToolChains/FreeBSD.cpp

 //===--- FreeBSD.cpp - FreeBSD ToolChain Implementations --------*- C++ -*-===//
 //
 // 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 "FreeBSD.h"
 #include "Arch/ARM.h"
 #include "Arch/Mips.h"
 #include "Arch/Sparc.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 using namespace clang::driver;

 }
 
 unsigned FreeBSD::GetDefaultDwarfVersion() const {
   if (getTriple().getOSMajorVersion() < 12)
     return 2;
   return 4;
 }
 
+void FreeBSD::AddClangSystemIncludeArgs(
+    const llvm::opt::ArgList &DriverArgs,
+    llvm::opt::ArgStringList &CC1Args) const {
+  const Driver &D = getDriver();
+
+  if (DriverArgs.hasArg(clang::driver::options::OPT_nostdinc))
+    return;
+
+  if (!DriverArgs.hasArg(options::OPT_nobuiltininc)) {
+    SmallString<128> Dir(D.ResourceDir);
+    llvm::sys::path::append(Dir, "include");
+    addSystemInclude(DriverArgs, CC1Args, Dir.str());
+  }
+
+  if (DriverArgs.hasArg(options::OPT_nostdlibinc))
+    return;
+
+  // Check for configure-time C include directories.
+  StringRef CIncludeDirs(C_INCLUDE_DIRS);
+  if (CIncludeDirs != "") {
+    SmallVector<StringRef, 5> dirs;
+    CIncludeDirs.split(dirs, ":");
+    for (StringRef dir : dirs) {
+      StringRef Prefix =
+          llvm::sys::path::is_absolute(dir) ? StringRef(D.SysRoot) : "";
+      addExternCSystemInclude(DriverArgs, CC1Args, Prefix + dir);
+    }
+    return;
+  }
+
+  addExternCSystemInclude(DriverArgs, CC1Args,
+                          concat(D.SysRoot, "/usr/include"));
+}
+
 void FreeBSD::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                     llvm::opt::ArgStringList &CC1Args) const {
   addSystemInclude(DriverArgs, CC1Args,
                    concat(getDriver().SysRoot, "/usr/include/c++/v1"));
 }
 
 void FreeBSD::addLibStdCxxIncludePaths(
     const llvm::opt::ArgList &DriverArgs,

clang/lib/Lex/InitHeaderSearch.cpp

void AddDefaultCIncludePaths(const llvm::Triple &triple,
                                const HeaderSearchOptions &HSOpts);
 
   // AddDefaultCPlusPlusIncludePaths -  Add paths that should be searched when
   //  compiling c++.
   void AddDefaultCPlusPlusIncludePaths(const LangOptions &LangOpts,
                                        const llvm::Triple &triple,
                                        const HeaderSearchOptions &HSOpts);
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
+  bool ShouldAddDefaultIncludePaths(const llvm::Triple &triple);
+
   /// AddDefaultSystemIncludePaths - Adds the default system include paths so
   ///  that e.g. stdio.h is found.
   void AddDefaultIncludePaths(const LangOptions &Lang,
                               const llvm::Triple &triple,
                               const HeaderSearchOptions &HSOpts);
 
   /// Realize - Merges all search path lists into one list and send it to
   /// HeaderSearch.
void InitHeaderSearch::AddMinGWCPlusPlusIncludePaths(StringRef Base,
   AddPath(Base + "/" + Arch + "/" + Version + "/include/c++/" + Arch,
           CXXSystem, false);
   AddPath(Base + "/" + Arch + "/" + Version + "/include/c++/backward",
           CXXSystem, false);
 }
 
 void InitHeaderSearch::AddDefaultCIncludePaths(const llvm::Triple &triple,
                                             const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
     llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
   if (HSOpts.UseStandardSystemIncludes) {
     switch (os) {
     case llvm::Triple::CloudABI:
-    case llvm::Triple::FreeBSD:
     case llvm::Triple::NetBSD:
-    case llvm::Triple::OpenBSD:
     case llvm::Triple::NaCl:
     case llvm::Triple::PS4:
     case llvm::Triple::PS5:
     case llvm::Triple::ELFIAMCU:
     case llvm::Triple::Fuchsia:
       break;
     case llvm::Triple::Win32:
       if (triple.getEnvironment() != llvm::Triple::Cygnus)
if (CIncludeDirs != "") {
     SmallVector<StringRef, 5> dirs;
     CIncludeDirs.split(dirs, ":");
     for (StringRef dir : dirs)
       AddPath(dir, ExternCSystem, false);
     return;
   }
 
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::OpenBSD:
-    llvm_unreachable("Include management is handled in the driver.");
-
   case llvm::Triple::CloudABI: {
     // <sysroot>/<triple>/include
     SmallString<128> P = StringRef(HSOpts.ResourceDir);
     llvm::sys::path::append(P, "../../..", triple.str(), "include");
     AddPath(P, System, false);
     break;
   }
 
default:
     AddPath("/usr/include", ExternCSystem, false);
     break;
   }
 }
 
 void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
     const LangOptions &LangOpts, const llvm::Triple &triple,
     const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-  // FIXME: temporary hack: hard-coded paths.
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
     llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
+  // FIXME: temporary hack: hard-coded paths.
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::AIX:
-    llvm_unreachable("Include management is handled in the driver.");
-    break;
   case llvm::Triple::Win32:
     switch (triple.getEnvironment()) {
     default: llvm_unreachable("Include management is handled in the driver.");
     case llvm::Triple::Cygnus:
       // Cygwin-1.7
       AddMinGWCPlusPlusIncludePaths("/usr/lib/gcc", "i686-pc-cygwin", "4.7.3");
       AddMinGWCPlusPlusIncludePaths("/usr/lib/gcc", "i686-pc-cygwin", "4.5.3");
       AddMinGWCPlusPlusIncludePaths("/usr/lib/gcc", "i686-pc-cygwin", "4.3.4");
case llvm::Triple::Minix:
     AddGnuCPlusPlusIncludePaths("/usr/gnu/include/c++/4.4.3",
                                 "", "", "", triple);
     break;
   default:
     break;
   }
 }
 
-void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
-                                              const llvm::Triple &triple,
-                                            const HeaderSearchOptions &HSOpts) {
-  // NB: This code path is going away. All of the logic is moving into the
-  // driver which has the information necessary to do target-specific
-  // selections of default include paths. Each target which moves there will be
-  // exempted from this logic here until we can delete the entire pile of code.
-  switch (triple.getOS()) {
-  default:
-    break; // Everything else continues to use this routine's logic.
+bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
+    const llvm::Triple &triple) {
+  llvm::Triple::OSType os = triple.getOS();
 
+  switch (triple.getOS()) {
+  case llvm::Triple::AIX:
   case llvm::Triple::Emscripten:
-  case llvm::Triple::Linux:
+  case llvm::Triple::FreeBSD:
   case llvm::Triple::Hurd:
+  case llvm::Triple::Linux:
   case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
-  case llvm::Triple::AIX:
-    return;
+    return false;
 
   case llvm::Triple::Win32:
     if (triple.getEnvironment() != llvm::Triple::Cygnus ||
         triple.isOSBinFormatMachO())
-      return;
+      return false;
     break;
 
   case llvm::Triple::UnknownOS:
     if (triple.isWasm())
-      return;
+      return false;
     break;
   }
 
-  // All header search logic is handled in the Driver for Darwin.
+  return true; // Everything else continues to use this routine's logic.
+}
+
+void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
+                                              const llvm::Triple &triple,
+                                            const HeaderSearchOptions &HSOpts) {
+  // NB: This code path is going away. All of the logic is moving into the
+  // driver which has the information necessary to do target-specific
+  // selections of default include paths. Each target which moves there will be
+  // exempted from this logic in ShouldAddDefaultIncludePaths() until we can
+  // delete the entire pile of code.
+  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 {
+      } else {
         AddPath("/System/Library/Frameworks", System, true);
         AddPath("/Library/Frameworks", System, true);
       }
     }
     return;
   }
 
   if (Lang.CPlusPlus && !Lang.AsmPreprocessor &&