Skip to content

Commit

Permalink
Add support for parse_headers
Browse files Browse the repository at this point in the history
This feature enables bazel to validate "C++" header files through clang.
Without this you only get this type of validating when you actually
include the library elsewhere.

Similar to layering_check we have to validate this outside of normal
bazel because we are expecting build failures and need to parse the logs

This mirrors bazelbuild/bazel#21560
  • Loading branch information
keith committed Jun 28, 2024
1 parent facfb55 commit e7925d9
Show file tree
Hide file tree
Showing 8 changed files with 113 additions and 2 deletions.
5 changes: 3 additions & 2 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,14 @@ tasks:
bazel: latest
<<: *common

macos_latest_layering_check:
name: "Current layering_check"
macos_latest_shell_scripts:
name: "Current layering_check and header parsing"
platform: macos_arm64
xcode_version: "15.2"
bazel: latest
shell_commands:
- test/shell/layering_check_test.sh
- test/shell/header_parsing_test.sh

macos_last_green:
name: "Last Green Bazel"
Expand Down
1 change: 1 addition & 0 deletions crosstool/BUILD.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ filegroup(
linker_files = ":osx_tools_" + arch,
objcopy_files = ":empty",
strip_files = ":osx_tools_" + arch,
supports_header_parsing = 1,
supports_param_files = 1,
toolchain_config = arch,
toolchain_identifier = arch,
Expand Down
34 changes: 34 additions & 0 deletions crosstool/cc_toolchain_config.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,21 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new
tools = [tool(path = "/usr/bin/strip")],
)

header_parsing_env_feature = feature(
name = "header_parsing_env",
env_sets = [
env_set(
actions = [ACTION_NAMES.cpp_header_parsing],
env_entries = [
env_entry(
key = "HEADER_PARSING_OUTPUT",
value = "%{output_file}",
),
],
),
],
)

cpp_header_parsing_action = action_config(
action_name = ACTION_NAMES.cpp_header_parsing,
implies = [
Expand All @@ -214,6 +229,23 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new
"compiler_input_flags",
"compiler_output_flags",
"unfiltered_cxx_flags",
"header_parsing_env",
],
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",
],
),
],
),
],
tools = [
tool(
Expand Down Expand Up @@ -2619,6 +2651,7 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new
feature(name = "no_legacy_features"),
feature(name = "only_doth_headers_in_module_maps"),
feature(name = "opt"),
feature(name = "parse_headers"),

# Features with more configuration
link_libcpp_feature,
Expand Down Expand Up @@ -2689,6 +2722,7 @@ please file an issue at https://github.com/bazelbuild/apple_support/issues/new
no_warn_duplicate_libraries_feature,
layering_check_feature,
external_include_paths_feature,
header_parsing_env_feature,
]

if (ctx.attr.cpu == "darwin_x86_64" or
Expand Down
6 changes: 6 additions & 0 deletions crosstool/wrapped_clang.cc
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,12 @@ int main(int argc, char *argv[]) {
return 1;
}

const char *header_parsing_output = getenv("HEADER_PARSING_OUTPUT");
if (header_parsing_output != nullptr) {
std::ofstream output(header_parsing_output);
output.close();
}

if (!postprocess) {
return 0;
}
Expand Down
40 changes: 40 additions & 0 deletions test/header_parsing/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
load("@bazel_skylib//rules:build_test.bzl", "build_test")

package(features = ["parse_headers"])

cc_library(
name = "invalid_header",
hdrs = ["invalid_header.h"],
tags = ["manual"],
)

objc_library(
name = "invalid_header_objc",
hdrs = ["invalid_header.h"],
tags = ["manual"],
)

cc_library(
name = "invalid_header_feature_disabled",
hdrs = ["invalid_header.h"],
features = ["-parse_headers"],
)

cc_library(
name = "valid_header",
hdrs = ["valid_header.h"],
)

objc_library(
name = "valid_header_objc",
hdrs = ["valid_header.h"],
)

build_test(
name = "test",
targets = [
":invalid_header_feature_disabled",
":valid_header",
":valid_header_objc",
],
)
2 changes: 2 additions & 0 deletions test/header_parsing/invalid_header.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Missing include of cstdint, which defines uint8_t.
uint8_t foo();
3 changes: 3 additions & 0 deletions test/header_parsing/valid_header.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#include <cstdint>

uint8_t foo();
24 changes: 24 additions & 0 deletions test/shell/header_parsing_test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
#!/bin/bash

set -euo pipefail

script_path="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
source "$script_path"/unittest.bash

bazel="${BAZEL:-bazel}"

function test_good_header_parsing() {
"$bazel" test --process_headers_in_dependencies -- //test/header_parsing/... &>"$TEST_log"
}

function test_bad_header_parsing() {
! "$bazel" test --process_headers_in_dependencies -- //test/header_parsing:invalid_header &> "$TEST_log" || fail "Expected build failure"
expect_log "test/header_parsing/invalid_header.h:2:1: error: unknown type name 'uint8_t'"
}

function test_bad_header_parsing_objc() {
! "$bazel" test --process_headers_in_dependencies -- //test/header_parsing:invalid_header_objc &> "$TEST_log" || fail "Expected build failure"
expect_log "test/header_parsing/invalid_header.h:2:1: error: unknown type name 'uint8_t'"
}

run_suite "header_parsing tests"

0 comments on commit e7925d9

Please sign in to comment.