This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by mhjacobson on Nov 16 2022, 10:07 PM.
Tokens
"Like" token, awarded by RandomDSdevel.

Details

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.

Diff Detail

Event Timeline

mhjacobson created this revision.Nov 16 2022, 10:07 PM
mhjacobson requested review of this revision.Nov 16 2022, 10:07 PM

Added test.

Run clang-format.

Clarify comment.

MaskRay added inline comments.Nov 16 2022, 11:46 PM
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

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

mhjacobson added inline comments.Nov 16 2022, 11:56 PM
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 added inline comments.Nov 17 2022, 12:06 AM
clang/test/Driver/freebsd.c
217

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.

Test for more paths; test C++.

Fix a couple warnings.

MaskRay added inline comments.Nov 17 2022, 9:04 AM
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

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

Style fixes.

Ensure include paths are sequential.

mhjacobson marked 6 inline comments as done.Nov 17 2022, 7:46 PM

Make the tests play nice with Windows path separators.

MaskRay accepted this revision.Nov 17 2022, 10:16 PM
This revision is now accepted and ready to land.Nov 17 2022, 10:16 PM

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