bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Enable setting C/C++ standards as a toolchain feature

Open jungleraptor opened this issue 3 years ago • 13 comments

Adds a handful of new features to the default toolchain that enables setting the C/C++ standards, with or without gnu extensions (-std=gnu) at the target level.

The issue associated with the PR is linked, and goes into further detail about the motivation for introducing this feature.

https://github.com/bazelbuild/bazel/issues/16551

~~This is an initial implementation. I'd like to get feedback on whether it's possible to accept this change before I add tests/documentation etc..~~

Usage

# build with -std=c++17
bazel build --features=c++17

# build with -std=gnu++17
bazel build --features=c++17 --features=gnu_extensions

Testing

Testing was done using this project: https://github.com/isaactorz/bazel-features-test

Let me know if there's an appropriate place in this repo to add these tests.

Testing done w/

  • gcc-11, gcc-12, and clang-14 on ubuntu 22.04
  • apple clang on intel macos (note that I add to bump the -mmacos-version-min flag to get the c++17 tests to build as the default was to low to use optional api.)

TODO

Assuming this can be merged I'd think the next steps would be:

  • [x] Add equivalent features for setting C standard
  • [x] Add tests
  • [x] Finalize the names of the feature
  • [ ] Mingw support
  • [x] Testing with msvc

jungleraptor avatar Oct 26 '22 01:10 jungleraptor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Oct 26 '22 01:10 google-cla[bot]

If one really doesn't want duplicate options then I guess they could set BAZEL_CXXOPTS="".

monika566 avatar Nov 30 '22 14:11 monika566

Hey, is this ready for a review?

buildbreaker2021 avatar Jan 12 '23 13:01 buildbreaker2021

Hey, is this ready for a review?

Whoops. Sorry for letting this go stale.

See the issue I linked for the background on why I initially proposed this. Internally we ended up just settling on splitting up building c and c++ code because we also need to set things like -fno-exceptions and -fno-rtti. If you're curious on our struggles see: https://github.com/swift-nav/rules_swiftnav/pull/52

However I still think this is a good and useful change for Bazel in general. I'll rebase and test this weekend and get it ready for review.

Are there any tests I should be adding for this stuff someone can point me to?

jungleraptor avatar Mar 31 '23 18:03 jungleraptor

You may want to see https://docs.google.com/document/d/132vEDnQZY0PtF9ko6HoLQ2dqk6meZnSrUfc1gsKuBkk/edit?usp=share_link

@fmeum

Wyverald avatar Mar 31 '23 19:03 Wyverald

Hi @isaactorz, Lets us know once it is ready for review. Thanks!

sgowroji avatar May 16 '23 12:05 sgowroji

Hi @isaactorz, Lets us know once it is ready for review. Thanks!

Will do. Been very busy lately. Only thing left I want to do is add support for this feature to the mingw toolchain.

jungleraptor avatar May 16 '23 16:05 jungleraptor

RTYI, @jsharpe. :)

junyer avatar Jun 30 '23 17:06 junyer

Why not adding C++ 23 and C++ 26 ? they have already landed in CMake and it will take months (years?) to have bazel 7.0 widely available so....

23
New in version 3.20.
C++23

26
New in version 3.25. 
C++26. CMake 3.25 and later recognize 26 as a valid value, no version has support for any compiler.

ref: https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD.html

Also you have C 23

23
New in version 3.21.
C23

see: https://cmake.org/cmake/help/latest/prop_tgt/C_STANDARD.html

ps: Nov 16, 2022: v3.25.0 Mars 23, 2021: v3.20.0 ref: https://github.com/Kitware/CMake/releases

Mizux avatar Jul 03 '23 06:07 Mizux

Why not adding C++ 23 and C++ 26 ? they have already landed in CMake and it will take months (years?) to have bazel 7.0 widely available so....

23
New in version 3.20.
C++23

26
New in version 3.25. 
C++26. CMake 3.25 and later recognize 26 as a valid value, no version has support for any compiler.

ref: https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD.html

Also you have C 23

23
New in version 3.21.
C23

see: https://cmake.org/cmake/help/latest/prop_tgt/C_STANDARD.html

ps: Nov 16, 2022: v3.25.0 Mars 23, 2021: v3.20.0 ref: https://github.com/Kitware/CMake/releases

I was going for a consistent set of features to expose across the "big 3" (gcc/clang/msvc)

with gcc and clang this would probably be fine since they have explicit codenames for the newer standards (i.e. std=c++2b, std=c++2c, etc..), but what to do for msvc? What /std:c++latest means now may not be what it means in the future. I guess the simple option would be to just not enable support for those features for the msvc toolchain.

Side note: maybe we even should consider using -std=c++2a instead of -std=c++20 to support a wider range of clang/gcc

jungleraptor avatar Jul 05 '23 20:07 jungleraptor

with gcc and clang this would probably be fine since they have explicit codenames for the newer standards (i.e. std=c++2b, std=c++2c, etc..), but what to do for msvc? What /std:c++latest means now may not be what it means in the future. I guess the simple option would be to just not enable support for those features for the msvc toolchain.

Side note: maybe we even should consider using -std=c++2a instead of -std=c++20 to support a wider range of clang/gcc

FYI CMake adapt what it would pass to the command line according to the compiler version... (which means bazel need to detect VS 2022 first...) https://gitlab.kitware.com/cmake/cmake/-/blob/0183956d30283212bc66cde17d9756f18bc5db27/Modules/Compiler/Clang.cmake

Mizux avatar Jul 05 '23 20:07 Mizux

Hi @jungleraptor what is the status of this, it is a sorely missed feature!

peakschris avatar Jun 23 '24 08:06 peakschris

Hi @jungleraptor what is the status of this, it is a sorely missed feature!

Hello!

At work we ended up forking unix_cc_toolchain_config.bzl and implementing this ourselves so this fell off my radar, but it is annoying that it doesn't work with the autoconfigured toolchain.

@sgowroji @buildbreaker2021 @Mizux Is this something we could land in mostly it's current shape ?

jungleraptor avatar Jun 28 '24 17:06 jungleraptor

@buildbreaker2021 @sgowroji @Mizux friendly ping

Recently came across another case in rules_rust that's currently having to work around this issue: https://github.com/bazelbuild/rules_rust/blob/bad49c4a1139afdb257fed0af6bd1dc20a6c21a1/bindgen/3rdparty/patches/llvm-project.cxx17.patch#L403

Also tagging @keith since you did the initial review.

jungleraptor avatar Sep 13 '24 15:09 jungleraptor

Is there really no way of having the actual C/CXX standard specified in a string (which may be parsed in some .bzl function) instead of having individual boolean features? We just about get away with it here with the number of standard revisions, but for other controls/flags which may hold arbitrary N values, this won't scale.

Summoning @fmeum on this point because it's an architectural question.

gr1mpatr0n avatar Apr 23 '25 02:04 gr1mpatr0n

Is there really no way of having the actual C/CXX standard specified in a string (which may be parsed in some .bzl function) instead of having individual boolean features?

This could probably be realized as an API, but in the end that would still need to be just syntactic sugar on top of boolean features: If you want to match on the availability of a certain feature in c++11 with a select or dependent feature, you would need e.g. c++17 to imply c++11 so that the check succeeds. There is no way to check on a range of features or anything like that.

fmeum avatar Apr 25 '25 17:04 fmeum