bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Support labeling targets with minimum-compatible C++ version

Open cramertj opened this issue 1 year ago • 3 comments

Description of the feature request:

I would like a standard way to tag a target (library, binary, or test) as requiring a minimum C++ version (e.g. C++20 and above).

Related features:

  • target_compatible_with: C++ version isn't quite a property of the platform, and there isn't a standard way to detect it, since it may be configured via any of: (1) target copts (2) command-line --copts=-std=c++14 (3) copts in the toolchain definition. Even when working with a single one of these, there's no general mechanism for select-ing for "C++20 or greater."
  • Enable setting C/C++ standards as a toolchain feature. This feature doesn't make it clear how to avoid building the (e.g.) C++20-only target when building the rest of the repo with a C++14 toolchain.
  • C/C++ standard resolution for Bazel modules (cc @fmeum). This is a bzlmod-module-level mechanism, so it only works if the whole module declares a minimum version. I'd like something more granular (target-level).

cc @tpudlik @trybka

Which category does this issue belong to?

C++ Rules, Core

What underlying problem are you trying to solve with this feature?

I work on a project that supports C++17. However, we'd like to offer C++20 coroutine support for users that are using C++20. I'd like to be able to declare a coroutine-compatibility target which only builds (and is only tested) when C++20 or later is enabled. Those particular library and test targets should not be built or run when using C++17.

One solution would be to #ifdef out the entire files such that the tests and library target still built and ran on C++17, but this would create the mistaken impression that the tests were building and running successfully, which I don't want. All dependent targets would also have to #ifdef themselves out of existence. Additionally, users attempting to depend on my target from C++17 would encounter an opaque error (missing symbols on import) rather than a useful one.

cramertj avatar May 06 '24 23:05 cramertj

@comius for the rules_cc perspective, @katre since it's (at least) Configurability-adjacent.

tpudlik avatar May 06 '24 23:05 tpudlik

There is some prior art here in how CMake handles things: https://cmake.org/cmake/help/latest/manual/cmake-compile-features.7.html#id6

There are lots of interesting questions about the semantics of this:

  • what does it mean for a target to set min_cc_std?
    • should this actually modify build flags? (i.e. if you require c++20 for a library, but your top-level config is c++17, should this set --std=c++20 for a given target) -- I believe this is what CMake does
    • should this throw an error if an incompatible std= flag is encountered in the myriad copt entry points?
    • should this behave like target_compatible_with when interacting with top-level flag configuration?
      • i.e. if you say, bazel test //:all --@rules_cc//lang:min_std=17 does that skip incompatible targets?
    • how does this propagate up and down the BUILD graph?
      • you can imagine top-level targets (e.g. binary, test) would want to enforce that any deps meet the minimum criteria (i.e. target_compatible_with semantics). would library deps act the same way?
  • how can we semantically set the language standard version at the top-level and inspect it -- given the proliferation of copts in various places[^1], and also the possibility to negate the flag with nocopts[^2]
    • Probably a build setting, but we still have the problem of what to do if we encounter the flag in some permutation of copt

Thinking of the above questions, I can see a couple of over-arching themes of how this could work:

  • requirement: this acts like a tag or constraint, other targets (which depend upon or are depended on by) would have to meet or exceed the requirement. Unlike most constraints in Bazel, these would not be just boolean--in theory these should support some kind of "at least" semantics. Kind of like Android's targetSdkVersion vs. minSdkVersion, though I do not know if android_library supports those as first-class attributes yet, either.
    • "at least" here is also tricky--how do you handle gnu++ vs. c++ ?
    • Also the versions aren't numeric -- incomplete standards are referenced with letters, e.g. 1y or 2c.
  • setting: this overrides the settings of the top-level config, possibly up (like cc_library.defines) or down (like transition()) the graph.

In both cases I think we have to figure out how to interact with copts wherever they originate:

  • One proposal might be that we filter std= out aggressively, preferring the library attributes and/or top-level flags. This is probably tricky unless you put that filtering way down in cc_common
  • Or your cc_toolchain could have some variable that it populates with the appropriately determined value, and sets it after user copts -- that's probably the cleanest way to ignore them.
    • e.g. flag_group ( flags = ["-std=%{std_value}"] ) [^3]
  • You could try to inspect the flags and parse them but that feels worse somehow, needing to juggle the attributes with all the copts.

[^1]: (command-line: --copt, --cxxopt, --per_file_copt, --features which set -std; cc_toolchain: either feature-gated or just as a bare flag_set, as well as BUILD attributes: cc_library.{features,copts} [^2]: While I am aware of https://github.com/bazelbuild/bazel/issues/8706 I do not think this has been completed, so folks (including our own internal repository) can set --noincompatible_disable_nocopts [^3]: I was tempted to split out std_prefix (i.e. gnu++ or c++) and std_value, but I have no idea if cc_toolchain can expand two variables in a single flag group.

trybka avatar May 07 '24 00:05 trybka

I had very similar problem with Java where I felt language version should be configured on a target level (with possible defaults for package/module).

I haven’t solved it for Java, resorting to a global flag —java_language_version. But at least in Java, higher versions are usually backwards compatible.

in C++ world “features” already support such granularity, so that’s probably the best approach.

I wish to have some inputs from configurability team. Cc @katre @gregestren

comius avatar May 15 '24 10:05 comius

I wound up emulating this in this CL by using a string_flag to select the C++ version, then some config_settings to detect which version was selected.

cramertj avatar Jul 25 '24 17:07 cramertj

@cramertj Could you share a public link for posterity?

fmeum avatar Jul 25 '24 19:07 fmeum

http://pwrev.dev/221453 is the publicly available link to the CL Taylor mentioned. I do think we should consider upstreaming to rules_cc the //pw_toolchain/cc:cxx_standard string_flag and the associated config_settings, and maybe even the minimum_cxx_20 target-compatibility helper defined in pw_build/compatibility.bzl.

tpudlik avatar Jul 26 '24 22:07 tpudlik

This looks good to me. In the future, we could extend it with an auto default value and a way for modules to declare their minimum required C++ standard, with rules_cc translating auto into the maximum of the required minimum standard levels. This would solve the problem of having to keep bumping the standard over time.

fmeum avatar Jul 27 '24 16:07 fmeum

In today's episode of "things I discovered were possible but may be ill-advised" I found a way to create config_settings that are driven by C/C++ toolchain features. It's roughly implemented as:

  • Scan for features prefixed with cpp_std_version_.
  • Extract the integer after the prefix.
  • Do arbitrary logic to spit out a BuildSettingInfo and/or config_common.FeatureFlagInfo with your desired effect.

There's a bit of glue required in the middle, but you can boil it down to having a cpp_std_version_17 feature properly propagate to a compatible_if_cpp_std_at_least(17).

armandomontanez avatar Oct 02 '24 21:10 armandomontanez