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

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/FreeBSD.cpp:442
+
+  addExternCSystemInclude(DriverArgs, CC1Args,
+                          concat(D.SysRoot, "/usr/include"));
----------------
I think Fuchsia way of checking `if (!D.SysRoot.empty()) {` before adding `/usr/include` is probably better. Is it /include or /usr/include ?


================
Comment at: clang/lib/Lex/InitHeaderSearch.cpp:103
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
----------------
Drop `ShouldAddDefaultIncludePaths - `. It is not recommended for new code.




================
Comment at: clang/lib/Lex/InitHeaderSearch.cpp:104
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
----------------
multiple spaces => one space


================
Comment at: clang/test/Driver/freebsd.c:217
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
----------------
Is this  sufficient? For Linux `linux-cross.cpp` checks a lot of directories to facilitate code refactoring.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138183/new/

https://reviews.llvm.org/D138183