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 hassle. Better to
use what rules_cc already offers for compiler selection.

The command line option --define=DRAKE_WERROR=ON no longer has any
effect. (That flag was intended to be internal use only so hopefully
nobody will care. It's replacement is now explicitly documented to be
internal use only.)

Remove some vestigial cc_toolchain diagnostic programs.

Also fix some warning Noble GCC spam from FCL, noticed while debugging.
  • Loading branch information
jwnimmer-tri committed Dec 19, 2024
1 parent 21c5485 commit 1b8349f
Show file tree
Hide file tree
Showing 12 changed files with 211 additions and 93 deletions.
8 changes: 8 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,14 @@ if(C_COMPILER_NAME STREQUAL ccache OR CXX_COMPILER_NAME STREQUAL ccache)
)
endif()

# Get the full C++ compiler major version for our cmake/bazel.rc.in.
string(REGEX MATCH "^([0-9]+)"
DRAKE_CC_TOOLCHAIN_COMPILER_MAJOR
"${CMAKE_CXX_COMPILER_VERSION}")
if(NOT DRAKE_CC_TOOLCHAIN_COMPILER_MAJOR)
set(DRAKE_CC_TOOLCHAIN_COMPILER_MAJOR "0")
endif()

# The minimum compiler versions should match those listed in both
# doc/_pages/from_source.md and tools/workspace/cc/repository.bzl.
set(MINIMUM_APPLE_CLANG_VERSION 14)
Expand Down
3 changes: 3 additions & 0 deletions cmake/bazel.rc.in
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ build:MinSizeRel --compilation_mode=opt --copt=-Os --host_copt=-Os
build:Release --compilation_mode=opt
build:RelWithDebInfo --compilation_mode=opt --fission=no --copt=-g --host_copt=-g

# Pass along the compiler version.
build --@drake//tools/cc_toolchain:compiler_major=@DRAKE_CC_TOOLCHAIN_COMPILER_MAJOR@

# Match CMAKE_BUILD_TYPE as well as the WITH_FOO customizations.
build @BAZEL_CONFIG@

Expand Down
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
146 changes: 122 additions & 24 deletions tools/cc_toolchain/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,44 +1,143 @@
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"])

# This cc_toolchain package provides labels that can be used by `select()`
# statements in our build rules to tweak settings based on which C++ compiler
# is being used. For example uses, see drake/tools/skylark/drake_cc.bzl.

# 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"},
)

# Capture some bazel cc_toolchain variables into a bash environment file. These
# values reflect the compiler and flags that Bazel's compilation actions will
# use. Note that this might refer to Bazel's compiler wrapper, `cc_wrapper.sh`,
# and thus may only indirectly identify the actual compiler in use. Note also
# that `$(CC)` may refer to the canonicalized location of the compiler; for
# example, on Ubuntu, Clang may be `/usr/lib/llvm-<version>/bin/clang`,
# canonicalized from `/usr/bin/clang-<version>`.
genrule(
name = "capture_cc",
outs = ["capture_cc.env"],
cmd = "cat <<EOF > '$@'\n" +
"BAZEL_CC=\"$(CC)\"\n" +
"BAZEL_CC_FLAGS=\"$(CC_FLAGS)\"\n" +
"EOF",
toolchains = [
"@bazel_tools//tools/cpp:cc_flags",
"@bazel_tools//tools/cpp:current_cc_toolchain",
selects.config_setting_group(
name = "gcc_13_with_warnings",
match_all = [
"@rules_cc//cc/compiler:gcc",
":compiler_major_13",
":error_severity_warning",
],
visibility = ["//common:__pkg__"],
)

selects.config_setting_group(
name = "gcc_13_with_errors",
match_all = [
"@rules_cc//cc/compiler:gcc",
":compiler_major_13",
":error_severity_error",
],
)

# *****************************************************************************
# The following sh_binary (and its prerequisite genrule) are used by drake-ci's
# CMake setup logic to sniff out Drake's selected compiler. They are not used
# by Drake's normal Bazel nor CMake builds.

# Capture bazel's action environment variables into a bash environment file.
# These values reflect ONLY the command-line (or rcfile) `--action_env`
# settings that Bazel uses when running its toolchain configuration, e.g.
Expand All @@ -53,7 +152,7 @@ genrule(
"CC=\"$${CC:-}\"\n" +
"CXX=\"$${CXX:-}\"\n" +
"EOF",
visibility = ["//common:__pkg__"],
visibility = ["//visibility:private"],
)

# Utility script to indicate which compiler is selected by use of
Expand All @@ -65,9 +164,8 @@ sh_binary(
name = "print_compiler_config",
srcs = ["print_compiler_config.sh"],
args = ["$(location :capture_compiler_config.env)"],
data = [
":capture_compiler_config.env",
],
data = [":capture_compiler_config.env"],
visibility = ["//visibility:private"],
)

add_lint_tests()
8 changes: 4 additions & 4 deletions tools/cc_toolchain/bazel.rc
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
# Disable ccache due to incompatibility with Bazel.
build --action_env=CCACHE_DISABLE=1

# When compiling with Drake as the main WORKSPACE (i.e., if and only if this
# rcfile is loaded), we enable -Werror by default for Drake's *own* targets,
# but not for our externals.
# When compiling with Drake as the main module (i.e., if and only if this rcfile
# is loaded), we enable -Werror by default for Drake's *own* targets, but not
# for our externals.
#
# Developers may still disable errors locally by passing --copt=-w on the
# command line, or promote *any* warnings even from externals to errors via
# --copt=-Werror.
#
# 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 + 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
Loading

0 comments on commit 1b8349f

Please sign in to comment.