diff --git a/tools/BUILD.bazel b/tools/BUILD.bazel index 560308f73fad..d3b97724873a 100644 --- a/tools/BUILD.bazel +++ b/tools/BUILD.bazel @@ -44,13 +44,6 @@ alias( # === config_setting rules === -# When this is set, a Drake build will promote some warnings to errors. -# See drake/tools/cc_toolchain/bazel.rc for details. -config_setting( - name = "drake_werror", - values = {"define": "DRAKE_WERROR=ON"}, -) - config_setting( name = "with_gurobi", values = {"define": "WITH_GUROBI=ON"}, diff --git a/tools/cc_toolchain/BUILD.bazel b/tools/cc_toolchain/BUILD.bazel index b90fbfea5f70..42d5f647f34e 100644 --- a/tools/cc_toolchain/BUILD.bazel +++ b/tools/cc_toolchain/BUILD.bazel @@ -1,21 +1,132 @@ +load("@bazel_skylib//lib:selects.bzl", "selects") +load("@bazel_skylib//rules:common_settings.bzl", "int_flag", "string_flag") load("//tools/lint:lint.bzl", "add_lint_tests") load("//tools/skylark:sh.bzl", "sh_binary") package(default_visibility = ["//visibility:public"]) +# When this is set to "error", a Drake build will promote some warnings of our +# choice to errors. See drake/tools/cc_toolchain/bazel.rc for details. This is +# intended for internal use only, not for users to configure. +string_flag( + name = "error_severity", + build_setting_default = "warning", + values = [ + "warning", + "error", + ], +) + config_setting( name = "apple", constraint_values = ["@platforms//os:osx"], ) +config_setting( + name = "linux", + constraint_values = ["@platforms//os:linux"], +) + +config_setting( + name = "error_severity_warning", + flag_values = {":error_severity": "warning"}, +) + +config_setting( + name = "error_severity_error", + flag_values = {":error_severity": "error"}, +) + +selects.config_setting_group( + name = "apple_clang_with_warnings", + match_all = [ + ":apple", + "@rules_cc//cc/compiler:clang", + ":error_severity_warning", + ], +) + +selects.config_setting_group( + name = "apple_clang_with_errors", + match_all = [ + ":apple", + "@rules_cc//cc/compiler:clang", + ":error_severity_error", + ], +) + +selects.config_setting_group( + name = "gcc_with_warnings", + match_all = [ + "@rules_cc//cc/compiler:gcc", + ":error_severity_warning", + ], +) + +selects.config_setting_group( + name = "gcc_with_errors", + match_all = [ + "@rules_cc//cc/compiler:gcc", + ":error_severity_error", + ], +) + +selects.config_setting_group( + name = "linux_clang_with_warnings", + match_all = [ + ":linux", + "@rules_cc//cc/compiler:clang", + ":error_severity_warning", + ], +) + +selects.config_setting_group( + name = "linux_clang_with_errors", + match_all = [ + ":linux", + "@rules_cc//cc/compiler:clang", + ":error_severity_error", + ], +) + config_setting( name = "debug", values = {"compilation_mode": "dbg"}, ) +# In our developer builds, we use this as an optional hint for configuring +# compiler warnings. It's neither necessary (nor typically set) when Drake +# is installed with CMake or used as a Bazel external. This is intended for +# internal use only, not for users to configure. +# +# TODO(jwnimmer-tri) Ideally, rules_cc would provide this as a toolchain +# attribute that we could query, but it doesn't seem to exist there yet. +int_flag( + name = "compiler_major", + build_setting_default = 0, +) + config_setting( - name = "linux", - constraint_values = ["@platforms//os:linux"], + name = "compiler_major_13", + flag_values = {":compiler_major": "13"}, +) + +selects.config_setting_group( + name = "gcc_13_with_warnings", + match_all = [ + "@rules_cc//cc/compiler:gcc", + ":compiler_major_13", + ":error_severity_warning", + ], +) + +selects.config_setting_group( + name = "gcc_13_with_errors", + match_all = [ + "@rules_cc//cc/compiler:gcc", + ":compiler_major_13", + ":error_severity_error", + ], ) # Capture some bazel cc_toolchain variables into a bash environment file. These diff --git a/tools/cc_toolchain/bazel.rc b/tools/cc_toolchain/bazel.rc index 8fdcb87b6751..53391ae29389 100644 --- a/tools/cc_toolchain/bazel.rc +++ b/tools/cc_toolchain/bazel.rc @@ -11,4 +11,4 @@ build --action_env=CCACHE_DISABLE=1 # # When compiilng Drake as an external package, this rcfile is not loaded and we # won't promote warnings to errors by default. -build --define=DRAKE_WERROR=ON +build --@drake//tools/cc_toolchain:error_severity=error diff --git a/tools/lint/library_lint.bzl b/tools/lint/library_lint.bzl index 79df42ea5a28..936729545be5 100644 --- a/tools/lint/library_lint.bzl +++ b/tools/lint/library_lint.bzl @@ -93,11 +93,9 @@ def library_lint( ]) # Find libraries that are deps of the package_library but shouldn't be. - extra_deps_expression = "deps({}, 1) except ({}) except {}".format( + extra_deps_expression = "deps({}, 1) except ({})".format( package_name + ":" + short_package_name, correct_deps_expression, - # This is fine (it's a dependency of our copt select() statement). - "//tools:drake_werror", ) # Find libraries that should be deps of the package_library but aren't. diff --git a/tools/skylark/drake_cc.bzl b/tools/skylark/drake_cc.bzl index 6b5f53004d42..e5a14366159e 100644 --- a/tools/skylark/drake_cc.bzl +++ b/tools/skylark/drake_cc.bzl @@ -1,4 +1,3 @@ -load("@cc//:compiler.bzl", "COMPILER_ID", "COMPILER_VERSION_MAJOR") load("//tools/skylark:cc.bzl", "CcInfo", "cc_binary", "cc_library", "cc_test") load( "//tools/skylark:kwargs.bzl", @@ -43,23 +42,10 @@ CLANG_FLAGS = CXX_FLAGS + [ "-Werror=unqualified-std-cast-call", ] -# The CLANG_VERSION_SPECIFIC_FLAGS will be enabled for all C++ rules in the -# project when building with a Clang compiler of the specified major -# version (excluding the Apple LLVM compiler, see -# APPLECLANG_VERSION_SPECIFIC_FLAGS below). -CLANG_VERSION_SPECIFIC_FLAGS = { -} - # The APPLECLANG_FLAGS will be enabled for all C++ rules in the project when # building with the Apple LLVM compiler. APPLECLANG_FLAGS = CLANG_FLAGS -# The APPLECLANG_VERSION_SPECIFIC_FLAGS will be enabled for all C++ rules in -# the project when building with an Apple LLVM compiler of the specified major -# version. -APPLECLANG_VERSION_SPECIFIC_FLAGS = { -} - # The GCC_FLAGS will be enabled for all C++ rules in the project when # building with gcc. GCC_FLAGS = CXX_FLAGS + [ @@ -79,6 +65,11 @@ GCC_CC_TEST_FLAGS = [ "-Wno-unused-parameter", ] +# The GCC_VERSION_SPECIFIC_FLAGS will be enabled for all C++ rules in the +# project when building with gcc of the specified major version, but only if +# the --@drake//tools/cc_toolchain:compiler_major=NN flag has been set on the +# command line or in an rcfile. (It typically will be except when Drake is used +# as a Bazel external.) GCC_VERSION_SPECIFIC_FLAGS = { 13: [ "-Werror=pessimizing-move", @@ -93,39 +84,49 @@ GCC_VERSION_SPECIFIC_FLAGS = { ], } +def _defang(flags): + """Given a list of copts, demote all -Werror into just plain -W.""" + return [ + x.replace("-Werror=", "-W") + for x in flags + ] + +# The BASE_COPTS are used for all drake_cc_{binary,library,test} rules. +BASE_COPTS = select({ + "//tools/cc_toolchain:apple_clang_with_errors": APPLECLANG_FLAGS, + "//tools/cc_toolchain:apple_clang_with_warnings": _defang(APPLECLANG_FLAGS), # noqa + "//tools/cc_toolchain:gcc_with_errors": GCC_FLAGS, + "//tools/cc_toolchain:gcc_with_warnings": _defang(GCC_FLAGS), + "//tools/cc_toolchain:linux_clang_with_errors": CLANG_FLAGS, + "//tools/cc_toolchain:linux_clang_with_warnings": _defang(CLANG_FLAGS), + "//conditions:default": _defang(CXX_FLAGS), +}) + select(dict([ + ("//tools/cc_toolchain:gcc_{}_with_errors".format(major_ver), flags) + for major_ver, flags in GCC_VERSION_SPECIFIC_FLAGS.items() +] + [ + ("//tools/cc_toolchain:gcc_{}_with_warnings".format(major_ver), _defang(flags)) # noqa + for major_ver, flags in GCC_VERSION_SPECIFIC_FLAGS.items() +] + [ + ("//conditions:default", []), +])) + def _platform_copts(rule_copts, rule_gcc_copts, rule_clang_copts, cc_test = 0): - """Returns both the rule_copts (plus rule_{cc}_copts iff under the - specified compiler), and platform-specific copts. + """Returns the concatenation of Drake's platform-specific BASE_COPTS, + plus the passed-in rule_copts, plus the passed-in plus rule_{cc}_copts iff + building with the specified compiler). - When cc_test=1, the GCC_CC_TEST_FLAGS will be added. It should only be set - to 1 from cc_test rules or rules that are boil down to cc_test rules. + When cc_test=1, the GCC_CC_TEST_FLAGS will also be added. It should only be + used from cc_test rules or rules that are boil down to cc_test rules. """ - if COMPILER_ID == "AppleClang": - result = APPLECLANG_FLAGS + rule_copts + rule_clang_copts - if COMPILER_VERSION_MAJOR in APPLECLANG_VERSION_SPECIFIC_FLAGS: - result += APPLECLANG_VERSION_SPECIFIC_FLAGS[COMPILER_VERSION_MAJOR] - elif COMPILER_ID == "Clang": - result = CLANG_FLAGS + rule_copts + rule_clang_copts - if COMPILER_VERSION_MAJOR in CLANG_VERSION_SPECIFIC_FLAGS: - result += CLANG_VERSION_SPECIFIC_FLAGS[COMPILER_VERSION_MAJOR] - elif COMPILER_ID == "GNU": - extra_gcc_flags = GCC_CC_TEST_FLAGS if cc_test else [] - result = GCC_FLAGS + extra_gcc_flags + rule_copts + rule_gcc_copts - if COMPILER_VERSION_MAJOR in GCC_VERSION_SPECIFIC_FLAGS: - result += GCC_VERSION_SPECIFIC_FLAGS[COMPILER_VERSION_MAJOR] - else: - result = rule_copts - - # We can't handle select() yet. - # TODO(jwnimmer-tri) We should handle select. - if type(result) != "list": - return result - return select({ - "//tools:drake_werror": result, - "//conditions:default": [ - x.replace("-Werror=", "-W") - for x in result - ], + if not any([rule_copts, rule_gcc_copts, rule_clang_copts, cc_test]): + # In the case of no special arguments at all, we can save Bazel the + # hassle of concatenating a bunch of empty stuff. + return BASE_COPTS + test_gcc_copts = GCC_CC_TEST_FLAGS if cc_test else [] + return BASE_COPTS + (rule_copts or []) + select({ + "@rules_cc//cc/compiler:gcc": rule_gcc_copts + test_gcc_copts, + "@rules_cc//cc/compiler:clang": rule_clang_copts, + "//conditions:default": [], }) def _check_library_deps_blacklist(name, deps): diff --git a/tools/skylark/pybind.bzl b/tools/skylark/pybind.bzl index 7fb31ed79259..f60aeccca086 100644 --- a/tools/skylark/pybind.bzl +++ b/tools/skylark/pybind.bzl @@ -1,4 +1,3 @@ -load("@cc//:compiler.bzl", "COMPILER_ID") load("@python//:version.bzl", "PYTHON_EXTENSION_SUFFIX") load("//tools/install:install.bzl", "install") load("//tools/skylark:cc.bzl", "CcInfo", "cc_binary") @@ -6,15 +5,18 @@ load("//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest") load("//tools/skylark:drake_py.bzl", "drake_py_library", "drake_py_test") load("//tools/skylark:py.bzl", "py_library") -EXTRA_PYBIND_COPTS = [ - # GCC and Clang don't always agree / succeed when inferring storage - # duration (#9600). Workaround it for now. - "-Wno-unused-lambda-capture", - # pybind11's operator overloading (e.g., .def(py::self + py::self)) - # spuriously triggers this warning, so we'll suppress it anytime we're - # compiling pybind11 modules. - "-Wno-self-assign-overloaded", -] if COMPILER_ID.endswith("Clang") else [] +EXTRA_PYBIND_COPTS = select({ + "@rules_cc//cc/compiler:clang": [ + # GCC and Clang don't always agree / succeed when inferring storage + # duration (#9600). Workaround it for now. + "-Wno-unused-lambda-capture", + # pybind11's operator overloading (e.g., .def(py::self + py::self)) + # spuriously triggers this warning, so we'll suppress it anytime we're + # compiling pybind11 modules. + "-Wno-self-assign-overloaded", + ], + "//conditions:default": [], +}) def pybind_py_library( name, diff --git a/tools/ubuntu-noble.bazelrc b/tools/ubuntu-noble.bazelrc index b41be418d8e2..9e4e1d0efced 100644 --- a/tools/ubuntu-noble.bazelrc +++ b/tools/ubuntu-noble.bazelrc @@ -8,3 +8,9 @@ build:clang --host_action_env=CC=clang-15 build:clang --host_action_env=CXX=clang++-15 build --define=UBUNTU_VERSION=24.04 + +# This flag tells our drake_cc rules which GCC version they should expect to +# see, so that the rules can tweak our default warning flags to be nicer. +# Note that this is not *changing* which compiler the build will use, it is +# only telling what to expect. +build --@drake//tools/cc_toolchain:compiler_major=13 diff --git a/tools/workspace/cc/repository.bzl b/tools/workspace/cc/repository.bzl index c22ddfe57371..2d26e5617b4c 100644 --- a/tools/workspace/cc/repository.bzl +++ b/tools/workspace/cc/repository.bzl @@ -142,6 +142,9 @@ def _impl(repository_ctx): file_content = """# DO NOT EDIT: generated by cc_repository() +print("DRAKE DEPRECATED: The @cc external is deprecated " + + "and will be removed from Drake on or after 2024-05-01.") + COMPILER_ID = "{}" COMPILER_VERSION_MAJOR = {} diff --git a/tools/workspace/default.bzl b/tools/workspace/default.bzl index a4b36d5c8825..f7cfbade1df1 100644 --- a/tools/workspace/default.bzl +++ b/tools/workspace/default.bzl @@ -159,6 +159,7 @@ def add_default_repositories( if "buildifier" not in excludes: buildifier_repository(name = "buildifier", mirrors = mirrors) if "cc" not in excludes: + # Deprecated 2025-04-01. cc_repository(name = "cc") if "ccd_internal" not in excludes: ccd_internal_repository(name = "ccd_internal", mirrors = mirrors)