From 368679f56b7746574322f3c16bb3aafc8b09f9e3 Mon Sep 17 00:00:00 2001 From: David Zbarsky Date: Sun, 2 Aug 2026 17:29:59 -0400 Subject: [PATCH] Compose C++ toolchain variables in Starlark --- cc/private/compile/compile.bzl | 32 ++--- .../compile/compile_action_templates.bzl | 5 +- .../compile/compile_build_variables.bzl | 17 ++- cc/private/link/lto_backends.bzl | 32 ++--- .../common/compile_build_variables_tests.bzl | 117 +++++++++++++++--- 5 files changed, 134 insertions(+), 69 deletions(-) diff --git a/cc/private/compile/compile.bzl b/cc/private/compile/compile.bzl index d1ca4ad1f..f31bc06c9 100644 --- a/cc/private/compile/compile.bzl +++ b/cc/private/compile/compile.bzl @@ -562,10 +562,7 @@ def _create_scan_deps_action( override_extension = extensions.CC_SOURCE[0], ), ) - compile_variables = _cc_internal.combine_cc_toolchain_variables( - common_toolchain_variables, - specific_compile_build_variables, - ) + compile_variables = common_toolchain_variables | specific_compile_build_variables _create_compile_action( action_construction_context = action_construction_context, cc_compilation_context = cc_compilation_context, @@ -1371,10 +1368,7 @@ def _create_cc_compile_actions( label = source_label, ), ) - compile_variables = _cc_internal.combine_cc_toolchain_variables( - common_compile_build_variables, - specific_compile_build_variables, - ) + compile_variables = common_compile_build_variables | specific_compile_build_variables # This creates the action to parse a header file. # If we generate pic actions, we prefer the header actions to use the pic artifacts. @@ -1709,10 +1703,7 @@ def _create_compile_source_action( use_pic = use_pic, lto_indexing_file = lto_indexing_file, action_name = action_name, - compile_build_variables = _cc_internal.combine_cc_toolchain_variables( - common_compile_variables, - compile_variables, - ), + compile_build_variables = common_compile_variables | compile_variables, needs_include_validation = _starlark_cc_semantics.needs_include_validation(language), toolchain_type = _starlark_cc_semantics.toolchain, progress_message_prefix = progress_message_prefix, @@ -1891,10 +1882,7 @@ def _create_temps_action( dotd_file = preprocess_dotd_file, diagnostics_file = preprocess_diagnostics_file, use_pic = use_pic, - compile_build_variables = _cc_internal.combine_cc_toolchain_variables( - common_compile_variables, - preprocess_compile_variables, - ), + compile_build_variables = common_compile_variables | preprocess_compile_variables, action_name = action_name, needs_include_validation = _starlark_cc_semantics.needs_include_validation(language), toolchain_type = _starlark_cc_semantics.toolchain, @@ -1913,10 +1901,7 @@ def _create_temps_action( dotd_file = assembly_dotd_file, diagnostics_file = assembly_diagnostics_file, use_pic = use_pic, - compile_build_variables = _cc_internal.combine_cc_toolchain_variables( - common_compile_variables, - assembly_compile_variables, - ), + compile_build_variables = common_compile_variables | assembly_compile_variables, action_name = action_name, needs_include_validation = _starlark_cc_semantics.needs_include_validation(language), toolchain_type = _starlark_cc_semantics.toolchain, @@ -2043,10 +2028,7 @@ def _create_module_codegen_action( fdo_build_variables = fdo_build_variables, additional_build_variables = {}, ) - compile_variables = _cc_internal.combine_cc_toolchain_variables( - common_toolchain_variables, - specific_compile_build_variables, - ) + compile_variables = common_toolchain_variables | specific_compile_build_variables additional_inputs = [] @@ -2344,7 +2326,7 @@ def _create_compile_action( additional_include_scanning_roots = additional_include_scanning_roots, cc_compilation_context = cc_compilation_context, cc_toolchain = cc_toolchain, - compile_build_variables = compile_build_variables, + compile_build_variables = _cc_internal.cc_toolchain_variables(vars = compile_build_variables), configuration = configuration, copts_filter = copts_filter, diagnostics_file = diagnostics_file, diff --git a/cc/private/compile/compile_action_templates.bzl b/cc/private/compile/compile_action_templates.bzl index b31efda24..920337da6 100644 --- a/cc/private/compile/compile_action_templates.bzl +++ b/cc/private/compile/compile_action_templates.bzl @@ -201,9 +201,8 @@ def _create_compile_action_template( cc_toolchain = cc_toolchain, configuration = configuration, feature_configuration = feature_configuration, - compile_build_variables = _cc_internal.combine_cc_toolchain_variables( - common_compile_build_variables, - specific_compile_build_variables, + compile_build_variables = _cc_internal.cc_toolchain_variables( + vars = common_compile_build_variables | specific_compile_build_variables, ), source = source_dir, additional_compilation_inputs = additional_compilation_inputs, diff --git a/cc/private/compile/compile_build_variables.bzl b/cc/private/compile/compile_build_variables.bzl index 29a59b953..638c70821 100644 --- a/cc/private/compile/compile_build_variables.bzl +++ b/cc/private/compile/compile_build_variables.bzl @@ -183,7 +183,12 @@ def create_compile_variables( additional_build_variables = additional_build_variables, user_compile_flags = user_compile_flags or [], ) - return _cc_internal.combine_cc_toolchain_variables(cc_toolchain._build_variables, common_vars, variables) + duplicate_variables = sorted([key for key in variables if key in common_vars]) + if duplicate_variables: + fail("Cannot overwrite existing variables: [" + ", ".join(duplicate_variables) + "]") + return _cc_internal.cc_toolchain_variables( + vars = cc_toolchain._build_variables_dict | common_vars | variables, + ) # buildifier: disable=function-docstring def setup_common_compile_build_variables( @@ -208,7 +213,7 @@ def setup_common_compile_build_variables( local_defines = cc_compilation_context.local_defines, external_include_dirs = cc_compilation_context.external_includes, ) - return _cc_internal.combine_cc_toolchain_variables(cc_toolchain._build_variables, common_vars) + return cc_toolchain._build_variables_dict | common_vars def _setup_common_compile_build_variables_internal( *, @@ -266,7 +271,7 @@ def _setup_common_compile_build_variables_internal( if external_include_dirs: result[_VARS.EXTERNAL_INCLUDE_PATHS] = external_include_dirs - return _cc_internal.cc_toolchain_variables(vars = result) + return result # Note: this method is side-effect free, callers should add fdo inputs to # cc_compile_action_builder themselves @@ -290,7 +295,7 @@ def get_specific_compile_build_variables( user_compile_flags = [], additional_build_variables = {}, fdo_build_variables = {}): - """Creates a CcToolchainVariables instance + """Creates a dictionary of compile build variables. Args: feature_configuration: (FeatureConfiguration) @@ -314,7 +319,7 @@ def get_specific_compile_build_variables( fdo_build_variables: (dict{str,str}) Returns: - (Variables) + (dict{str, object}) """ result = {} @@ -360,7 +365,7 @@ def get_specific_compile_build_variables( result[_VARS.PIC] = "" result = result | additional_build_variables result = result | fdo_build_variables - return _cc_internal.cc_toolchain_variables(vars = result) + return result _SOURCE_TYPES_FOR_CXXOPTS = set( extensions.CC_SOURCE + diff --git a/cc/private/link/lto_backends.bzl b/cc/private/link/lto_backends.bzl index 2cef9da1b..4a9aae318 100644 --- a/cc/private/link/lto_backends.bzl +++ b/cc/private/link/lto_backends.bzl @@ -268,7 +268,7 @@ def setup_common_lto_variables( feature_configuration: (feature_configuration) The feature configuration. Returns: - A CcToolchainVariables provider and a list[File] of additional inputs. + A dictionary of build variables and a list[File] of additional inputs. """ build_variables = {} @@ -285,12 +285,7 @@ def setup_common_lto_variables( if feature_configuration.is_enabled("cs_fdo_instrument"): build_variables["cs_fdo_instrument_path"] = cc_toolchain._cpp_configuration.cs_fdo_instrument() - build_variables = _cc_internal.combine_cc_toolchain_variables( - cc_toolchain._build_variables, - _cc_internal.cc_toolchain_variables(vars = build_variables), - ) - - return build_variables, additional_inputs + return cc_toolchain._build_variables_dict | build_variables, additional_inputs def create_lto_backend_artifacts( *, @@ -327,7 +322,7 @@ def create_lto_backend_artifacts( cc_toolchain: (CcToolchainInfo) The C++ toolchain. use_pic: (bool) Whether to use PIC. should_create_per_object_debug_info: (bool) Whether to create per-object debug info. - build_variables: (CcToolchainVariables) Toolchain variables to use for argument expansion. + build_variables: (dict) Toolchain variables to use for argument expansion. additional_inputs: list[File] Additional file inputs required for generated actions. argv: (list[str]) The command line arguments to pass to the LTO backend. @@ -340,17 +335,15 @@ def create_lto_backend_artifacts( create_shared_non_lto = all_bitcode_files == None - build_variables = _cc_internal.combine_cc_toolchain_variables( - build_variables, - _cc_internal.cc_toolchain_variables(vars = { - "user_compile_flags": _cc_internal.intern_string_sequence_variable_value(argv), - }), - ) + build_variables = build_variables | { + "user_compile_flags": _cc_internal.intern_string_sequence_variable_value(argv), + } + native_build_variables = _cc_internal.cc_toolchain_variables(vars = build_variables) env = _cc_common_internal.get_environment_variables( feature_configuration = feature_configuration, action_name = "lto-backend", - variables = build_variables, + variables = native_build_variables, ) obj = lto_obj_root_prefix + "/" + bitcode_file.path @@ -378,7 +371,7 @@ def create_lto_backend_artifacts( feature_configuration = feature_configuration, additional_inputs = additional_inputs, env = env, - build_variables = build_variables, + build_variables = native_build_variables, use_pic = use_pic, all_bitcode_files = all_bitcode_files, index = index, @@ -481,13 +474,12 @@ def _create_lto_backend_action( dwo_file, bitcode_file_path if bitcode_file_path != None else bitcode_artifact, ) - _path_variables = _cc_internal.cc_toolchain_variables(vars = _path_variables) - build_variables = _cc_internal.combine_cc_toolchain_variables(build_variables, _path_variables) - _cc_internal.create_lto_backend_action( actions = actions, feature_configuration = feature_configuration, - build_variables = build_variables, + build_variables = _cc_internal.cc_toolchain_variables( + vars = build_variables | _path_variables, + ), use_pic = use_pic, inputs = inputs, all_bitcode_files = bitcode_files, diff --git a/tests/cc/common/compile_build_variables_tests.bzl b/tests/cc/common/compile_build_variables_tests.bzl index 886abfaf2..a4c7913eb 100644 --- a/tests/cc/common/compile_build_variables_tests.bzl +++ b/tests/cc/common/compile_build_variables_tests.bzl @@ -1,12 +1,48 @@ """Tests for compile build variables.""" +load("@bazel_features//:features.bzl", "bazel_features") load("@rules_testing//lib:analysis_test.bzl", "test_suite") load("@rules_testing//lib:truth.bzl", "matching", "subjects") load("@rules_testing//lib:util.bzl", "TestingAspectInfo", "util") +load("//cc:action_names.bzl", "ACTION_NAMES") load("//cc:cc_binary.bzl", _actual_cc_binary = "cc_binary") +load("//cc:find_cc_toolchain.bzl", "CC_TOOLCHAIN_ATTRS", "find_cpp_toolchain", "use_cc_toolchain") +load("//cc/common:cc_common.bzl", "cc_common") load("//tests/cc/testutil:cc_analysis_test.bzl", "cc_analysis_test") load("//tests/cc/testutil:link_action_subject.bzl", "link_action_subject") +_CompileVariablesInfo = provider( + doc = "Arguments expanded from cc_common.create_compile_variables.", + fields = ["arguments"], +) + +def _compile_variables_test_rule_impl(ctx): + cc_toolchain = find_cpp_toolchain(ctx) + feature_configuration = cc_common.configure_features( + ctx = ctx, + cc_toolchain = cc_toolchain, + requested_features = ["debug_variables"], + ) + variables = cc_common.create_compile_variables( + cc_toolchain = cc_toolchain, + feature_configuration = feature_configuration, + source_file = "test.cc", + output_file = "test.o", + variables_extension = ctx.attr.variables_extension, + ) + return [_CompileVariablesInfo(arguments = cc_common.get_memory_inefficient_command_line( + feature_configuration = feature_configuration, + action_name = ACTION_NAMES.cpp_compile, + variables = variables, + ))] + +_compile_variables_test_rule = rule( + implementation = _compile_variables_test_rule_impl, + attrs = CC_TOOLCHAIN_ATTRS | {"variables_extension": attr.string_dict()}, + fragments = ["cpp"], + toolchains = use_cc_toolchain(), +) + # Wrap cc_binary to mock out common dependencies. def cc_binary(name, **kwargs): if "malloc" not in kwargs: @@ -232,6 +268,51 @@ def _test_presence_of_sysroot_build_variable_impl(env, target): compile_action = _compile_action(env, target, "bin") _variable(compile_action, "sysroot").equals("/usr/grte/v1") +def _test_compile_variables_extension_overrides_toolchain_variable(name, **kwargs): + util.helper_target( + _compile_variables_test_rule, + name = name + "/variables", + variables_extension = {"sysroot": "/overridden/sysroot"}, + ) + cc_analysis_test( + name = name, + impl = _test_compile_variables_extension_overrides_toolchain_variable_impl, + target = name + "/variables", + **kwargs + ) + +def _test_compile_variables_extension_overrides_toolchain_variable_impl(env, target): + env.expect.that_collection(target[_CompileVariablesInfo].arguments).contains( + "--debug-var:sysroot=/overridden/sysroot", + ) + +def _test_compile_variables_extension_rejects_duplicate_variables(name, **kwargs): + util.helper_target( + _compile_variables_test_rule, + name = name + "/variables", + variables_extension = { + "user_compile_flags": "duplicate flags", + "output_file": "duplicate output", + "source_file": "duplicate source", + }, + ) + cc_analysis_test( + name = name, + impl = _test_compile_variables_extension_rejects_duplicate_variables_impl, + target = name + "/variables", + expect_failure = True, + **kwargs + ) + +def _test_compile_variables_extension_rejects_duplicate_variables_impl(env, target): + expected_error = "Cannot overwrite existing variables: [output_file, source_file, user_compile_flags]" + env.expect.that_target(target).failures().contains_predicate( + matching.custom( + "contains '{}'".format(expected_error), + lambda actual: expected_error in actual, + ), + ) + def _test_target_sysroot_without_platforms(name, **kwargs): util.helper_target( cc_binary, @@ -390,21 +471,27 @@ def compile_build_variables_tests(name): Args: name: The name of the test suite. """ + tests = [ + _test_presence_of_basic_variables, + _test_presence_of_configuration_compile_flags, + _test_presence_of_conly_flags, + _test_cxx_flags_order, + _test_per_file_copts_are_in_user_compile_flags, + _test_host_per_file_copts_are_in_user_compile_flags, + _test_presence_of_sysroot_build_variable, + _test_target_sysroot_without_platforms, + _test_target_sysroot_with_platforms, + _test_presence_of_is_using_fission_variable, + _test_presence_of_is_using_fission_and_per_debug_object_file_variables_with_thinlto, + _test_presence_of_per_object_debug_file_build_variable, + _test_presence_of_min_os_version_build_variable, + ] + if bazel_features.cc.cc_common_is_in_rules_cc: + tests.extend([ + _test_compile_variables_extension_overrides_toolchain_variable, + _test_compile_variables_extension_rejects_duplicate_variables, + ]) test_suite( name = name, - tests = [ - _test_presence_of_basic_variables, - _test_presence_of_configuration_compile_flags, - _test_presence_of_conly_flags, - _test_cxx_flags_order, - _test_per_file_copts_are_in_user_compile_flags, - _test_host_per_file_copts_are_in_user_compile_flags, - _test_presence_of_sysroot_build_variable, - _test_target_sysroot_without_platforms, - _test_target_sysroot_with_platforms, - _test_presence_of_is_using_fission_variable, - _test_presence_of_is_using_fission_and_per_debug_object_file_variables_with_thinlto, - _test_presence_of_per_object_debug_file_build_variable, - _test_presence_of_min_os_version_build_variable, - ], + tests = tests, )