Skip to content

Commit

Permalink
[tools] Modernize Bazel spelling for selecting compiler flags
Browse files Browse the repository at this point in the history
Deprecate the 'cc' external.

Keeping this repository rule intact with bzlmod is certainly possible,
but is somewhat of a pain point and hassle. Better to just use what
rules_cc already offers for compiler selection as best we can.
  • Loading branch information
jwnimmer-tri committed Dec 19, 2024
1 parent ee25dbb commit 81973c7
Show file tree
Hide file tree
Showing 9 changed files with 182 additions and 67 deletions.
7 changes: 0 additions & 7 deletions tools/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand Down
115 changes: 113 additions & 2 deletions tools/cc_toolchain/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion tools/cc_toolchain/bazel.rc
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 1 addition & 3 deletions tools/lint/library_lint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
89 changes: 45 additions & 44 deletions tools/skylark/drake_cc.bzl
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -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 + [
Expand All @@ -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",
Expand All @@ -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):
Expand Down
22 changes: 12 additions & 10 deletions tools/skylark/pybind.bzl
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
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")
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,
Expand Down
6 changes: 6 additions & 0 deletions tools/ubuntu-noble.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 3 additions & 0 deletions tools/workspace/cc/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {}
Expand Down
1 change: 1 addition & 0 deletions tools/workspace/default.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 81973c7

Please sign in to comment.