Add `parse_headers` support to the default Unix toolchain
Also remove Ubuntu 16.04 workarounds from layering_check tests.
RELNOTES: The default Unix C++ toolchain now supports the `parse_headers` feature to validate header files with `--process_headers_in_dependencies`.
Closes #21560.
PiperOrigin-RevId: 633657012
Change-Id: Iaaa2623bcc86b219b02567eca1c7bf9e1a77ae7d
diff --git a/site/en/docs/bazel-and-cpp.md b/site/en/docs/bazel-and-cpp.md
index aab47cd..866f0f7 100644
--- a/site/en/docs/bazel-and-cpp.md
+++ b/site/en/docs/bazel-and-cpp.md
@@ -82,3 +82,22 @@
use the [`include_prefix`](/reference/be/c-cpp#cc_library.include_prefix) and
[`strip_include_prefix`](/reference/be/c-cpp#cc_library.strip_include_prefix)
arguments on the `cc_library` rule target.
+
+### Toolchain features {:#toolchain-features}
+
+The following optional [features](/docs/cc-toolchain-config-reference#features)
+can improve the hygiene of a C++ project. They can be enabled using the
+`--features` command-line flag or the `features` attribute of
+[`repo`](/external/overview#repo.bazel),
+[`package`](/reference/be/functions#package) or `cc_*` rules:
+
+* The `parse_headers` feature makes it so that the C++ compiler is used to parse
+ (but not compile) all header files in the built targets and their dependencies
+ when using the
+ [`--process_headers_in_dependencies`](/reference/command-line-reference#flag--process_headers_in_dependencies)
+ flag. This can help catch issues in header-only libraries and ensure that
+ headers are self-contained and independent of the order in which they are
+ included.
+* The `layering_check` feature enforces that targets only include headers
+ provided by their direct dependencies. The default toolchain supports this
+ feature on Linux with `clang` as the compiler.
diff --git a/src/test/shell/bazel/bazel_layering_check_test.sh b/src/test/shell/bazel/bazel_layering_check_test.sh
index bbbd680..4ccf75d 100755
--- a/src/test/shell/bazel/bazel_layering_check_test.sh
+++ b/src/test/shell/bazel/bazel_layering_check_test.sh
@@ -31,6 +31,12 @@
)
cc_library(
+ name = 'hello_header_only',
+ hdrs = ['hello_header_only.h'],
+ deps = [":hello_lib"],
+)
+
+cc_library(
name = "hello_lib",
srcs = ["hello_private.h", "hellolib.cc"],
hdrs = ["hello.h"],
@@ -116,10 +122,27 @@
}
#endif
EOF
+
+ cat >hello/hello_header_only.h <<EOF
+#ifdef private_header
+#include "hello_private.h"
+int func() {
+ return helloPrivate() - 42;
+}
+#elif defined layering_violation
+#include "base.h"
+int func() {
+ return base() - 42;
+}
+#else
+#include "hello.h"
+int func() {
+ return hello() - 42;
+}
+#endif
+EOF
}
-# TODO(hlopko): Add a test for a "toplevel" header-only library
-# once we have parse_headers support in cc_configure.
function test_bazel_layering_check() {
if is_darwin; then
echo "This test doesn't run on Darwin. Skipping."
@@ -135,7 +158,7 @@
write_files
CC="${clang_tool}" bazel build \
- //hello:hello --linkopt=-fuse-ld=gold --features=layering_check \
+ //hello:hello --features=layering_check \
&> "${TEST_log}" || fail "Build with layering_check failed"
bazel-bin/hello/hello || fail "the built binary failed to run"
@@ -148,23 +171,65 @@
fail "module map files were not generated"
fi
- # Specifying -fuse-ld=gold explicitly to override -fuse-ld=/usr/bin/ld.gold
- # passed in by cc_configure because Ubuntu-16.04 ships with an old
- # clang version that doesn't accept that.
CC="${clang_tool}" bazel build \
--copt=-D=private_header \
- //hello:hello --linkopt=-fuse-ld=gold --features=layering_check \
+ //hello:hello --features=layering_check \
&> "${TEST_log}" && fail "Build of private header violation with "\
"layering_check should have failed"
expect_log "use of private header from outside its module: 'hello_private.h'"
CC="${clang_tool}" bazel build \
--copt=-D=layering_violation \
- //hello:hello --linkopt=-fuse-ld=gold --features=layering_check \
+ //hello:hello --features=layering_check \
&> "${TEST_log}" && fail "Build of private header violation with "\
"layering_check should have failed"
expect_log "module //hello:hello does not depend on a module exporting "\
"'base.h'"
}
+function test_bazel_layering_check_header_only() {
+ if is_darwin; then
+ echo "This test doesn't run on Darwin. Skipping."
+ return
+ fi
+
+ local -r clang_tool=$(which clang)
+ if [[ ! -x ${clang_tool:-/usr/bin/clang_tool} ]]; then
+ echo "clang not installed. Skipping test."
+ return
+ fi
+
+ write_files
+
+ CC="${clang_tool}" bazel build \
+ //hello:hello_header_only --features=layering_check --features=parse_headers \
+ -s --process_headers_in_dependencies \
+ &> "${TEST_log}" || fail "Build with layering_check + parse_headers failed"
+
+ if [[ ! -e bazel-bin/hello/hello_header_only.cppmap ]]; then
+ fail "module map file for hello_header_only was not generated"
+ fi
+
+ if [[ ! -e bazel-bin/hello/hello_lib.cppmap ]]; then
+ fail "module map file for hello_lib was not generated"
+ fi
+
+ CC="${clang_tool}" bazel build \
+ --copt=-D=private_header \
+ //hello:hello_header_only --features=layering_check --features=parse_headers \
+ --process_headers_in_dependencies \
+ &> "${TEST_log}" && fail "Build of private header violation with "\
+ "layering_check + parse_headers should have failed"
+ expect_log "use of private header from outside its module: 'hello_private.h'"
+
+ CC="${clang_tool}" bazel build \
+ --copt=-D=layering_violation \
+ //hello:hello_header_only --features=layering_check --features=parse_headers \
+ --process_headers_in_dependencies \
+ &> "${TEST_log}" && fail "Build of private header violation with "\
+ "layering_check + parse_headers should have failed"
+ expect_log "module //hello:hello_header_only does not depend on a module exporting "\
+ "'base.h'"
+}
+
run_suite "test layering_check"
diff --git a/src/test/shell/bazel/cc_integration_test.sh b/src/test/shell/bazel/cc_integration_test.sh
index e5e1887..19e39f4 100755
--- a/src/test/shell/bazel/cc_integration_test.sh
+++ b/src/test/shell/bazel/cc_integration_test.sh
@@ -1909,4 +1909,39 @@
fi
}
+function test_parse_headers_unclean() {
+ mkdir pkg
+ cat > pkg/BUILD <<'EOF'
+cc_library(name = "lib", hdrs = ["lib.h"])
+EOF
+ cat > pkg/lib.h <<'EOF'
+// Missing include of cstdint, which defines uint8_t.
+uint8_t foo();
+EOF
+
+ bazel build -s --process_headers_in_dependencies --features parse_headers \
+ //pkg:lib &> "$TEST_log" && fail "Build should have failed due to unclean headers"
+ expect_log "Compiling pkg/lib.h"
+ expect_log "error:.*'uint8_t'"
+
+ bazel build -s --process_headers_in_dependencies \
+ //pkg:lib &> "$TEST_log" || fail "Build should have passed"
+}
+
+function test_parse_headers_clean() {
+ mkdir pkg
+ cat > pkg/BUILD <<'EOF'
+package(features = ["parse_headers"])
+cc_library(name = "lib", hdrs = ["lib.h"])
+EOF
+ cat > pkg/lib.h <<'EOF'
+#include <cstdint>
+uint8_t foo();
+EOF
+
+ bazel build -s --process_headers_in_dependencies \
+ //pkg:lib &> "$TEST_log" || fail "Build should have passed"
+ expect_log "Compiling pkg/lib.h"
+}
+
run_suite "cc_integration_test"
diff --git a/tools/cpp/BUILD.tpl b/tools/cpp/BUILD.tpl
index 384b63a..f99995d 100644
--- a/tools/cpp/BUILD.tpl
+++ b/tools/cpp/BUILD.tpl
@@ -84,6 +84,7 @@
linker_files = ":compiler_deps",
objcopy_files = ":empty",
strip_files = ":empty",
+ supports_header_parsing = 1,
supports_param_files = 1,
module_map = %{modulemap},
)
diff --git a/tools/cpp/linux_cc_wrapper.sh.tpl b/tools/cpp/linux_cc_wrapper.sh.tpl
index a83be50..629741e 100644
--- a/tools/cpp/linux_cc_wrapper.sh.tpl
+++ b/tools/cpp/linux_cc_wrapper.sh.tpl
@@ -18,8 +18,37 @@
#
set -eu
+OUTPUT=
+
+function parse_option() {
+ local -r opt="$1"
+ if [[ "${OUTPUT}" = "1" ]]; then
+ OUTPUT=$opt
+ elif [[ "$opt" = "-o" ]]; then
+ # output is coming
+ OUTPUT=1
+ fi
+}
+
+# let parse the option list
+for i in "$@"; do
+ if [[ "$i" = @* && -r "${i:1}" ]]; then
+ while IFS= read -r opt
+ do
+ parse_option "$opt"
+ done < "${i:1}" || exit 1
+ else
+ parse_option "$i"
+ fi
+done
+
# Set-up the environment
%{env}
# Call the C++ compiler
%{cc} "$@"
+
+# Generate an empty file if header processing succeeded.
+if [[ "${OUTPUT}" == *.h.processed ]]; then
+ echo -n > "${OUTPUT}"
+fi
diff --git a/tools/cpp/osx_cc_wrapper.sh.tpl b/tools/cpp/osx_cc_wrapper.sh.tpl
index fa60f33..e40a98b 100644
--- a/tools/cpp/osx_cc_wrapper.sh.tpl
+++ b/tools/cpp/osx_cc_wrapper.sh.tpl
@@ -71,6 +71,11 @@
# Call the C++ compiler
%{cc} "$@"
+# Generate an empty file if header processing succeeded.
+if [[ "${OUTPUT}" == *.h.processed ]]; then
+ echo -n > "${OUTPUT}"
+fi
+
function get_library_path() {
for libdir in ${LIB_DIRS}; do
if [ -f ${libdir}/lib$1.so ]; then
diff --git a/tools/cpp/unix_cc_configure.bzl b/tools/cpp/unix_cc_configure.bzl
index 365e78c..3f162bd 100644
--- a/tools/cpp/unix_cc_configure.bzl
+++ b/tools/cpp/unix_cc_configure.bzl
@@ -387,6 +387,10 @@
overriden_tools["ar"] = _find_generic(repository_ctx, "libtool", "LIBTOOL", overriden_tools)
auto_configure_warning_maybe(repository_ctx, "CC used: " + str(cc))
tool_paths = _get_tool_paths(repository_ctx, overriden_tools)
+
+ # The parse_header tool needs to be a wrapper around the compiler as it has
+ # to touch the output file.
+ tool_paths["parse_headers"] = "cc_wrapper.sh"
cc_toolchain_identifier = escape_string(get_env_var(
repository_ctx,
"CC_TOOLCHAIN_NAME",
@@ -546,9 +550,10 @@
"%{cc_toolchain_identifier}": cc_toolchain_identifier,
"%{name}": cpu_value,
"%{modulemap}": ("\":module.modulemap\"" if generate_modulemap else "None"),
- "%{cc_compiler_deps}": get_starlark_list([":builtin_include_directory_paths"] + (
- [":cc_wrapper"] if darwin else []
- )),
+ "%{cc_compiler_deps}": get_starlark_list([
+ ":builtin_include_directory_paths",
+ ":cc_wrapper",
+ ]),
"%{compiler}": escape_string(get_env_var(
repository_ctx,
"BAZEL_COMPILER",
diff --git a/tools/cpp/unix_cc_toolchain_config.bzl b/tools/cpp/unix_cc_toolchain_config.bzl
index 35e2c64..3f077a9 100644
--- a/tools/cpp/unix_cc_toolchain_config.bzl
+++ b/tools/cpp/unix_cc_toolchain_config.bzl
@@ -96,6 +96,47 @@
),
]
+def parse_headers_support(parse_headers_tool_path):
+ if not parse_headers_tool_path:
+ return [], []
+ action_configs = [
+ action_config(
+ action_name = ACTION_NAMES.cpp_header_parsing,
+ tools = [
+ tool(path = parse_headers_tool_path),
+ ],
+ flag_sets = [
+ flag_set(
+ flag_groups = [
+ flag_group(
+ flags = [
+ # Note: This treats all headers as C++ headers, which may lead to
+ # parsing failures for C headers that are not valid C++.
+ # For such headers, use features = ["-parse_headers"] to selectively
+ # disable parsing.
+ "-xc++-header",
+ "-fsyntax-only",
+ ],
+ ),
+ ],
+ ),
+ ],
+ implies = [
+ # Copied from the legacy feature definition in CppActionConfigs.java.
+ "legacy_compile_flags",
+ "user_compile_flags",
+ "sysroot",
+ "unfiltered_compile_flags",
+ "compiler_input_flags",
+ "compiler_output_flags",
+ ],
+ ),
+ ]
+ features = [
+ feature(name = "parse_headers"),
+ ]
+ return action_configs, features
+
all_compile_actions = [
ACTION_NAMES.c_compile,
ACTION_NAMES.cpp_compile,
@@ -1488,6 +1529,12 @@
generate_linkmap_feature,
]
+ parse_headers_action_configs, parse_headers_features = parse_headers_support(
+ parse_headers_tool_path = ctx.attr.tool_paths.get("parse_headers"),
+ )
+ action_configs += parse_headers_action_configs
+ features += parse_headers_features
+
return cc_common.create_cc_toolchain_config_info(
ctx = ctx,
features = features,
