[libc-commits] [libc] [libc][clang-tidy] Add llvm-header-guard to get consistant naming and prevent file copy/paste issues. (PR #66477)

[Guillaume Chatelet via libc-commits]

gchatelet wrote:

So `llvm-header-guard` should not normally flag headers coming from `isystem` but there were a lot of false positives coming from `clang` and llvm libc so I took a look at what's going on.

Looking at the `compile_commands.json` file that is generated by CMake and which [drives the `clang-tidy`](https://github.com/llvm/llvm-project/blob/main/libc/cmake/modules/LLVMLibCObjectRules.cmake#L729-L773) it seems that all the headers are appended with the `-I` option. I believe this is because the headers are added manually via a bunch of
```
set(include_dirs ${LIBC_SOURCE_DIR} ${LIBC_INCLUDE_DIR})
target_include_directories(${gpu_target_name} PRIVATE ${include_dirs})
```
instead of using the proper `libc-headers` target which defines [the `isystem` includes](https://github.com/llvm/llvm-project/blob/dd070394a3106217e0e4cdcf589f131c4946096c/libc/include/CMakeLists.txt#L535).

Example command line from `compile_commands.json` shows that all includes are done via `-I` and none via `-isystem`
```
/home/gchatelet/clang_head_install/bin/clang++ -DLIBC_NAMESPACE=__llvm_libc_18_0_0_git -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D_LIBCPP_ENABLE_HARDENED_MODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/gchatelet/build/libc_x86/projects/libc/src/fenv -I/home/gchatelet/git/llvm-project/libc/src/fenv -I/home/gchatelet/build/libc_x86/include -I/home/gchatelet/git/llvm-project/llvm/include -I/home/gchatelet/git/llvm-project/libc -I/home/gchatelet/build/libc_x86/projects/libc/include  -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -march=znver2 -O2 -fpie -ffreestanding -fno-builtin -fno-exceptions -fno-lax-vector-conversions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -Wall -Wextra -Wconversion -Wno-sign-conversion -Wimplicit-fallthrough -Wwrite-strings -Wextra-semi -Wnewline-eof -Wnonportable-system-include-path -Wstrict-prototypes -Wthread-safety -Wglobal-constructors -UNDEBUG -std=c++17 -o projects/libc/src/fenv/CMakeFiles/libc.src.fenv.feclearexcept.__internal__.dir/feclearexcept.cpp.o -c /home/gchatelet/git/llvm-project/libc/src/fenv/feclearexcept.cpp
```

I may not fully grasp how the header inclusion mechanism works but I believe that -for the purpose of `llvm-header-guard` - some of the headers should be declared `isystem`.

Also looking more closely at the above example command line, it seems to me that there are too many includes
 - `-I/home/gchatelet/build/libc_x86/projects/libc/src/fenv`
 - `-I/home/gchatelet/git/llvm-project/libc/src/fenv`
 - `-I/home/gchatelet/build/libc_x86/include`
 - `-I/home/gchatelet/git/llvm-project/llvm/include`
 - `-I/home/gchatelet/git/llvm-project/libc`
 - `-I/home/gchatelet/build/libc_x86/projects/libc/include`

@sivachandra do we expect that many includes?

On the other hand, the Bazel generated command line seems much more reasonable to me
```
/usr/lib/llvm-14/bin/clang -U_FORTIFY_SOURCE -fstack-protector -Wall -Wthread-safety -Wself-assign -Wunused-but-set-parameter -Wno-free-nonheap-object -fcolor-diagnostics -fno-omit-frame-pointer '-std=c++0x' -MD -MF bazel-out/k8-fastbuild/bin/external/llvm-project/libc/_objs/feclearexcept/feclearexcept.pic.d '-frandom-seed=bazel-out/k8-fastbuild/bin/external/llvm-project/libc/_objs/feclearexcept/feclearexcept.pic.o' -fPIC '-DLIBC_NAMESPACE=__llvm_libc_18_0_0_git' -DLIBC_COPT_PUBLIC_PACKAGING '-DLLVM_LIBC_FUNCTION_ATTR=__attribute__((visibility("default")))' '-DBAZEL_CURRENT_REPOSITORY="llvm-project"' -iquote external/llvm-project -iquote bazel-out/k8-fastbuild/bin/external/llvm-project -isystem external/llvm-project/libc -isystem bazel-out/k8-fastbuild/bin/external/llvm-project/libc -O1 -Wall -Wno-deprecated '-std=c++17' -Wno-range-loop-analysis -O3 -fno-builtin -fno-lax-vector-conversions '-fvisibility=hidden' -no-canonical-prefixes -Wno-builtin-macro-redefined '-D__DATE__="redacted"' '-D__TIMESTAMP__="redacted"' '-D__TIME__="redacted"' -c external/llvm-project/libc/src/fenv/feclearexcept.cpp -o bazel-out/k8-fastbuild/bin/external/llvm-project/libc/_objs/feclearexcept/feclearexcept.pic.o
```

https://github.com/llvm/llvm-project/pull/66477