From 61def7a42c0451f74bd3ae26c9a2b15c53de6048 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 25 Mar 2024 15:52:38 -0700 Subject: [PATCH] Improve errors in variable definitions by adding labels to the variables. BEGIN_PUBLIC Improve errors in variable definitions by adding labels to the variables. END_PUBLIC PiperOrigin-RevId: 618984216 Change-Id: I5d6d11ba2b72f426b9f01bcbb528b0914c98c964 --- cc/toolchains/cc_toolchain_info.bzl | 3 ++- cc/toolchains/impl/variables.bzl | 8 +++----- tests/rule_based_toolchain/variables/variables_test.bzl | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cc/toolchains/cc_toolchain_info.bzl b/cc/toolchains/cc_toolchain_info.bzl index 42227d4..0429ad2 100644 --- a/cc/toolchains/cc_toolchain_info.bzl +++ b/cc/toolchains/cc_toolchain_info.bzl @@ -50,6 +50,7 @@ VariableInfo = provider( # @unsorted-dict-items fields = { "name": "(str) The variable name", + "label": "(Label) The label defining this provider. Place in error messages to simplify debugging", "actions": "(Optional[depset[ActionTypeInfo]]) The actions this variable is available for", "type": "A type constructed using variables.types.*", }, @@ -58,7 +59,7 @@ VariableInfo = provider( BuiltinVariablesInfo = provider( doc = "The builtin variables", fields = { - "variables": "(dict[str, struct(type=type, actions=Optional[depset[ActionTypeInfo]]) A mapping from variable name to variable metadata.", + "variables": "(dict[str, VariableInfo]) A mapping from variable name to variable metadata.", }, ) diff --git a/cc/toolchains/impl/variables.bzl b/cc/toolchains/impl/variables.bzl index 6577b34..c2820f3 100644 --- a/cc/toolchains/impl/variables.bzl +++ b/cc/toolchains/impl/variables.bzl @@ -52,6 +52,7 @@ types = struct( def _cc_variable_impl(ctx): return [VariableInfo( name = ctx.label.name, + label = ctx.label, type = json.decode(ctx.attr.type), actions = collect_action_types(ctx.attr.actions) if ctx.attr.actions else None, )] @@ -84,10 +85,7 @@ def cc_variable(name, type, **kwargs): def _cc_builtin_variables_impl(ctx): return [BuiltinVariablesInfo(variables = { - variable.name: struct( - actions = variable.actions, - type = variable.type, - ) + variable.name: variable for variable in collect_provider(ctx.attr.srcs, VariableInfo) })] @@ -131,7 +129,7 @@ def get_type(*, name, variables, overrides, actions, args_label, nested_label, f for action in actions: if action not in valid_actions: fail("The variable {var} is inaccessible from the action {action}. This is required because it is referenced in {nested_label}, which is included by {args_label}, which references that action".format( - var = outer, + var = variables[outer].label, nested_label = nested_label, args_label = args_label, action = action.label, diff --git a/tests/rule_based_toolchain/variables/variables_test.bzl b/tests/rule_based_toolchain/variables/variables_test.bzl index be15a45..a3cf843 100644 --- a/tests/rule_based_toolchain/variables/variables_test.bzl +++ b/tests/rule_based_toolchain/variables/variables_test.bzl @@ -84,7 +84,7 @@ nested_str_list: List[string]""") expect_type("struct", actions = [c_compile]).ok() expect_type("struct", actions = [c_compile, cpp_compile]).err().equals( - "The variable struct is inaccessible from the action %s. This is required because it is referenced in %s, which is included by %s, which references that action" % (cpp_compile.label, _NESTED_LABEL, _ARGS_LABEL), + "The variable %s is inaccessible from the action %s. This is required because it is referenced in %s, which is included by %s, which references that action" % (targets.struct.label, cpp_compile.label, _NESTED_LABEL, _ARGS_LABEL), ) expect_type("struct.nested_str_list", actions = [c_compile]).ok()