search

teslamotors / fixed-containers

C++ Fixed Containers
MIT License
361 stars 31 forks source link

Clang: Add -Wno-poison-system-directories #31

Closed Quuxplusone closed 1 year ago

Quuxplusone commented 1 year ago

Before this patch, Clang trunk gives:

[  2%] Building CXX object CMakeFiles/string_literal_test.dir/test/string_literal_test.cpp.o
error: include location '/usr/local/include' is unsafe for cross-compilation
[-Werror,-Wpoison-system-directories]
1 error generated.

Better would be to replace -Weverything with -Wall -Wextra (https://quuxplusone.github.io/blog/2018/12/06/dont-use-weverything/)

alexkaratarakis commented 1 year ago

Amended to also exclude in bazel. However, I have been unable to reproduce this with clang-trunk (Ubuntu clang version 17.0.0 (++20230417095441+43ac269bdd00-1~exp1~20230417215605.872), which hash are you using?

-Weverything

Valid points! The primary goal is to have an "opt-out" as opposed to an "opt-in" for diagnostics and I could not find a way to do so without -Weverything. It is used here for tests only and doesn't propagate to production code, but I am open to sanctioned alternatives. In the discussions linked in the blog, I see there are considerations towards that!

Quuxplusone commented 1 year ago

I have been unable to reproduce this with clang-trunk (Ubuntu clang version 17.0.0 (++20230417095441+43ac269bdd00-1~exp1~20230417215605.872), which hash are you using?

I was using (my fork currently based on top of) be93256655def7026166865859b54082ef579363; but I'm sure it doesn't have to do with any recent code changes, I think it just has to do with what paths CMake (v3.21.2) decided to put into -I/-isystem, which depends on hard-to-reproduce environment stuff. (I'm on OSX, having done brew install magic_enum which doesn't seem to install a new enough version anyway — thus my PR to remove magic_enum from as many codepaths as possible. I'm afraid I won't be much help digging further than that.)

alexkaratarakis commented 1 year ago

Ok, happy to take the change since it will improve developer iteration.

If you are ok with installing bazel, there is a one-liner that can reliably fetch deps and do the build.

bazel: CC=clang++-13 bazel build --config=clang ...

or with cmake/vcpkg:

mkdir build && cd build
cmake .. -DCMAKE_C_COMPILER=/bin/clang-13 -DCMAKE_CXX_COMPILER=/bin/clang++-13 -DCMAKE_TOOLCHAIN_FILE=/path/to/vcpkg/scripts/buildsystems/vcpkg.cmake
cmake --build .