bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.2k stars 4.06k forks source link

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

Open jungleraptor opened 2 years ago

jungleraptor commented 2 years ago

Description of the feature request:

Proposal to add a handful of new "features" to the default c++ toolchain that enables setting C/C++ standards (names tentative):

I've got an initial implementation of this capability that currently works on linux & macos, and does not break backwards compatibility: https://github.com/bazelbuild/bazel/pull/16552

I'd like to get some feedback from the Bazel team on if it's possible to get this merged before doing whatever else is necessary to meet the contribution requirements (design docs, testing, etc...).

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

The original issue I was trying to solve was to get some code to build with bazel that mixes C/C++ code in the same target:

cc_library(
    name = "example",
    srcs = [
        "src/foo.cpp", # requires compiling with C++14
        "src/bar.c"
    ],
    copts = [
        "-Werror",
        "-std=c++14",
    ]
)

It's currently not possible to build that target with Bazel. This is because there's no way to distinguish between C and C++ flags at the cc_* rules level. The compiler will issue a warning when -std=c++14 is passed while compiling the C file, and -Werror will cause that warning to be propagated as a build error.

There are various alternatives but none of them are desirable:

I think that the most elegant solution to this problem is to just expose a new set of features in the default toolchain that allow setting the standard. The features API enables distinguishing between C/C++ actions and avoids the scope issues above. Every target can mix and match C/C++ and decide the standard for each.

The POC branch I've linked shows that it should be possible to do this without breaking backwards compatibility of existing behavior, assuming that the order of flags is not part of the API.

Which operating system are you running Bazel on?

Windows, Linux, and Mac

What is the output of bazel info release?

5.3.2

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

Lots of research, still prefer my suggestion above.

Any other information, logs, or outputs that you want to share?

No response

jungleraptor commented 2 years ago

@oquenchil Sorry don't know who else to ping. Any objections to this feature? Happy to do the remaining work if it can be accepted.

My initial PR that implements this feature is here: https://github.com/bazelbuild/bazel/pull/16552

Mizux commented 1 year ago

My 2 cents,

  1. for C "only" you can use --conlyopt=... or --action_env=BAZEL_CONLYOPTS=.. ed: copt was a wrong naming i.e. closer to CPPFLAGS (C PreProcessor) behaviour aka injected for C and C++ rules

ref: https://github.com/bazelbuild/bazel/blob/e97f62d54585c759735487bec78cc2bb6a7d215b/tools/cpp/unix_cc_configure.bzl#L409-L421 EDIT: https://github.com/bazelbuild/bazel/commit/b272cef7d4b2d1be4e3f82b737c3c14d5e768aea only in preview 7 ref: https://www.gnu.org/software/make/manual/make.html#Implicit-Variables

  1. Adding c++ dialect in rules using copt in BUILD/WORKSPACE is against abseil-cpp policy ref: https://github.com/abseil/abseil-cpp/blob/master/FAQ.md#how-to-i-set-the-c-dialect-used-to-build-abseil
fmeum commented 1 year ago

At least for cc_* targets in external repositories, https://docs.google.com/document/u/1/d/132vEDnQZY0PtF9ko6HoLQ2dqk6meZnSrUfc1gsKuBkk/edit#heading=h.5mcn15i0e1ch could solve this as well.

junyer commented 1 year ago

RTYI, @jsharpe. :)

github-actions[bot] commented 2 months ago

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

phst commented 2 months ago

I think this is still relevant.