search

bazel-contrib / toolchains_llvm

LLVM toolchain for bazel
Apache License 2.0
301 stars 217 forks source link

c++-compile action should use clang++ instead of clang. #372

Open sfc-gh-mhazy opened 2 months ago

sfc-gh-mhazy commented 2 months ago

Hi, Thank you for the work on the project so far. It makes cc toolchain configuration much easier.

I've noticed that some builds fail with linker errors, for instance, compiling cc_test with com_google_google_test fails with

ld.lld: error: undefined reference due to --no-allow-shlib-undefined: nextafter

This is related to https://github.com/google/googletest/issues/3878

I think the problem is as described in the issue above - compiling c++ should use clang++.

This might be, however, problematic as it goes down to bazel implementation of default actions that is using gcc instead of g++.

It is possible to override the action configs in cc_common.create_cc_toolchain_config_info, but unix_cc_toolchain_config does not expose it.

I did some rough experiment and replaced clang with clang++ in https://github.com/bazel-contrib/toolchains_llvm/blob/master/toolchain/cc_wrapper.sh.tpl and that did the trick. This is not a generic solution though, as it would fail c compilation.

On the bright side, the workaround is to add -lm to cc_test linkopts or link_flags of cc toolchain configuration.

Perhaps we should start with exposing action_configs in unix_cc_toolchain_config and then override the cpp actions in toolchains_llvm. Alternative would be to inline unix_cc_toolchain_config into toolchains_llvm.

Looking forward to your feedback.

fmeum commented 2 months ago

With all these small inconsistencies between Bazel and the rest of the world, I wonder whether we shouldn't just make Bazel's default toolchain use clang for C and clang++ for C++. Bazel 8 could be a good time for that as it may not be doable in a fully backwards compatible way.

jsharpe commented 2 months ago

+1! This would make rules_foreign_cc a bit easier to work with too

sfc-gh-mhazy commented 2 months ago

Thank you for responses. Long term definitely changing the defaults is the best choice.

But would be good to have some improvement short / middle term.

  1. I don't know how feasible it is to add -lm by default for linux compilation.
  2. Maybe lets start a "Known issues" section in readme to increase visibility? It takes some time to debug such issues so that might help future strugglers.

Side note: The original post was about linux, on macos I had to add "-lstdc++" to link flags.