Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
sfc-gh-mhazy opened this issue Aug 13, 2024 · 3 comments
Open

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

sfc-gh-mhazy opened this issue Aug 13, 2024 · 3 comments

Comments

@sfc-gh-mhazy
Copy link

sfc-gh-mhazy commented Aug 13, 2024

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 google/googletest#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
Copy link
Member

fmeum commented Aug 13, 2024

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
Copy link
Member

jsharpe commented Aug 13, 2024

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

@sfc-gh-mhazy
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants