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

jungleraptor · 2022-10-26T01:47:06Z

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.

#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:

google-cla · 2022-10-26T01:47:09Z

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.

tools/cpp/unix_cc_toolchain_config.bzl

keith · 2022-10-31T23:38:51Z

tools/cpp/unix_cc_configure.bzl

@@ -528,6 +521,12 @@ def configure_unix_toolchain(repository_ctx, cpu_value, overriden_tools):
                cpu_value,
                False,
            )),
+            "%{cxx_standard}": escape_string(get_env_var(
+                repository_ctx,
+                "BAZEL_CXXOPTS",


I don't think this is a safe change as previously this env var could have contained much more than just this. I think to keep compatibility you'll have to move this back up to where it was, just default it to nothing, and hardcode the C++ default here (although I guess it also could have affected the CXX include directories output? I'm not sure if that's good or bad)

Ah, of course 🤦🏼 .

So since that's the case I've left everything in unix_cc_configure.bzl alone and only added the new features that set the standard. No more default_cxx_standard enabled by default either.

If the features are activated then they take precedent over the -std=c++0x opt defined in cxx_opts (since they come later on the cmd line).

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

Does that sound better? Certainly simpler implementation wise.

tools/cpp/unix_cc_configure.bzl

monika566 · 2022-11-30T14:04:35Z

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

buildbreaker2021 · 2023-01-12T13:34:04Z

Hey, is this ready for a review?

jungleraptor · 2023-03-31T18:49:29Z

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: swift-nav/rules_swiftnav#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?

Wyverald · 2023-03-31T19:35:26Z

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

@fmeum

Adds a handful of new features to the default toolchain that enables setting the c++ standard. This enables setting the standard on a target by target basis.

sgowroji · 2023-05-16T12:23:40Z

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

jungleraptor · 2023-05-16T16:12:28Z

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.

junyer · 2023-06-30T17:28:38Z

RTYI, @jsharpe. :)

Mizux · 2023-07-03T06:16:18Z

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

jungleraptor · 2023-07-05T20:12:55Z

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

Mizux · 2023-07-05T20:38:18Z

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

peakschris · 2024-06-23T08:41:25Z

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

jungleraptor · 2024-06-28T17:06:29Z

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 · 2024-09-13T15:47:37Z

@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.

sgowroji added team-Rules-CPP Issues for C++ rules awaiting-user-response Awaiting a response from the author labels Oct 26, 2022

jungleraptor mentioned this pull request Oct 26, 2022

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

Open

jungleraptor force-pushed the isaactorz/cc-standard-feature branch from 44261bb to 3cbda3a Compare October 26, 2022 17:51

sgowroji added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Oct 28, 2022

keith reviewed Oct 31, 2022

View reviewed changes

tools/cpp/unix_cc_toolchain_config.bzl Outdated Show resolved Hide resolved

keith reviewed Oct 31, 2022

View reviewed changes

jungleraptor force-pushed the isaactorz/cc-standard-feature branch from 5a332fc to 43daa1f Compare November 1, 2022 01:38

keith approved these changes Nov 1, 2022

View reviewed changes

tools/cpp/unix_cc_configure.bzl Outdated Show resolved Hide resolved

keith mentioned this pull request Jan 6, 2023

Update default or provide easy mechaism to change C++ version bazelbuild/rules_apple#1801

Open

buildbreaker2021 self-assigned this Jan 12, 2023

buildbreaker2021 added the awaiting-user-response Awaiting a response from the author label Jan 12, 2023

sgowroji removed the awaiting-review PR is awaiting review from an assigned reviewer label Jan 16, 2023

cc: enable setting c++ standard with toolchain feature

0896d3c

Adds a handful of new features to the default toolchain that enables setting the c++ standard. This enables setting the standard on a target by target basis.

jungleraptor force-pushed the isaactorz/cc-standard-feature branch from 6073e36 to 0896d3c Compare April 1, 2023 17:26

jungleraptor added 4 commits April 1, 2023 14:23

more

cb661fa

c standards

c876b1e

drop support for bleeding edge standards

59e6bbb

Merge branch 'master' into isaactorz/cc-standard-feature

81a6370

jungleraptor requested a review from keith September 13, 2024 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jungleraptor commented Oct 26, 2022 •

edited

Loading

google-cla bot commented Oct 26, 2022

keith Oct 31, 2022

jungleraptor Nov 1, 2022 •

edited

Loading

keith Nov 1, 2022

monika566 commented Nov 30, 2022

buildbreaker2021 commented Jan 12, 2023

jungleraptor commented Mar 31, 2023

Wyverald commented Mar 31, 2023

sgowroji commented May 16, 2023

jungleraptor commented May 16, 2023

junyer commented Jun 30, 2023

Mizux commented Jul 3, 2023 •

edited

Loading

jungleraptor commented Jul 5, 2023

Mizux commented Jul 5, 2023 •

edited

Loading

peakschris commented Jun 23, 2024

jungleraptor commented Jun 28, 2024 •

edited

Loading

jungleraptor commented Sep 13, 2024

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

Are you sure you want to change the base?

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

Conversation

jungleraptor commented Oct 26, 2022 • edited Loading

Usage

Testing

TODO

google-cla bot commented Oct 26, 2022

keith Oct 31, 2022

Choose a reason for hiding this comment

jungleraptor Nov 1, 2022 • edited Loading

Choose a reason for hiding this comment

keith Nov 1, 2022

Choose a reason for hiding this comment

monika566 commented Nov 30, 2022

buildbreaker2021 commented Jan 12, 2023

jungleraptor commented Mar 31, 2023

Wyverald commented Mar 31, 2023

sgowroji commented May 16, 2023

jungleraptor commented May 16, 2023

junyer commented Jun 30, 2023

Mizux commented Jul 3, 2023 • edited Loading

jungleraptor commented Jul 5, 2023

Mizux commented Jul 5, 2023 • edited Loading

peakschris commented Jun 23, 2024

jungleraptor commented Jun 28, 2024 • edited Loading

jungleraptor commented Sep 13, 2024

jungleraptor commented Oct 26, 2022 •

edited

Loading

jungleraptor Nov 1, 2022 •

edited

Loading

Mizux commented Jul 3, 2023 •

edited

Loading

Mizux commented Jul 5, 2023 •

edited

Loading

jungleraptor commented Jun 28, 2024 •

edited

Loading