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

Support labeling targets with minimum-compatible C++ version #22264

Open
cramertj opened this issue May 6, 2024 · 3 comments
Open

Support labeling targets with minimum-compatible C++ version #22264

cramertj opened this issue May 6, 2024 · 3 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability Issues for Configurability team team-Rules-CPP Issues for C++ rules type: feature request

Comments

@cramertj
Copy link

cramertj commented May 6, 2024

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.

@github-actions github-actions bot added team-Core Skyframe, bazel query, BEP, options parsing, bazelrc team-Rules-CPP Issues for C++ rules labels May 6, 2024
@tpudlik
Copy link
Contributor

tpudlik commented May 6, 2024

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

@trybka
Copy link
Contributor

trybka commented May 7, 2024

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 places1, and also the possibility to negate the flag with nocopts2
    • 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.

Footnotes

  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.

@comius
Copy link
Contributor

comius commented May 15, 2024

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 comius added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels May 15, 2024
@haxorz haxorz added team-Configurability Issues for Configurability team and removed team-Core Skyframe, bazel query, BEP, options parsing, bazelrc labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability Issues for Configurability team team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

8 participants