Rollforward of f7946d0: Propagate graph_node_aspect over a computed d…

…efault attribute

Set computed default to deps only when we need to propagate graph_node aspect.

This change makes it possible to collapse cc_binary and cc_test into a single rule.

NEW: using initializers setting a private field (instead of computed default that caused a regression). This is a workaround that propagates an aspect optionally. Allowing initializers set a private field is a potential replacement for computed default. Current implementation of initializers allows this only in builtins - we need to be careful, because implicit dependencies potentially affect other system.
PiperOrigin-RevId: 580118899
Change-Id: I4625c0c4f903b19e697f8d5ae09bbea682d78a13

src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl

@@ -14,6 +14,8 @@
 
 """cc_binary Starlark implementation replacing native"""
 
+load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs")
+load(":common/cc/cc_shared_library.bzl", "cc_shared_library_initializer")
 load(":common/cc/cc_common.bzl", "cc_common")
 load(":common/cc/cc_helper.bzl", "cc_helper", "linker_mode")
 load(":common/cc/cc_info.bzl", "CcInfo")
@@ -340,7 +342,7 @@ def _filter_libraries_that_are_linked_dynamically(ctx, feature_configuration, cc
     static_linker_inputs = []
     linker_inputs = cc_linking_context.linker_inputs.to_list()
 
-    all_deps = ctx.attr.deps + semantics.get_cc_runtimes(ctx, _is_link_shared(ctx))
+    all_deps = ctx.attr._deps_analyzed_by_graph_structure_aspect
     graph_structure_aspect_nodes = [dep[GraphNodeInfo] for dep in all_deps if GraphNodeInfo in dep]
 
     can_be_linked_dynamically = {}
@@ -908,21 +910,20 @@ def _impl(ctx):
 
     return providers
 
-def make_cc_binary(cc_binary_attrs, **kwargs):
-    return rule(
-        implementation = _impl,
-        attrs = cc_binary_attrs,
-        outputs = {
-            "stripped_binary": "%{name}.stripped",
-            "dwp_file": "%{name}.dwp",
-        },
-        fragments = ["cpp"] + semantics.additional_fragments(),
-        exec_groups = {
-            "cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
-        },
-        toolchains = cc_helper.use_cpp_toolchain() +
-                     semantics.get_runtimes_toolchain(),
-        executable = True,
-        provides = [CcInfo],
-        **kwargs
-    )
+cc_binary = rule(
+    implementation = _impl,
+    initializer = cc_shared_library_initializer,
+    attrs = cc_binary_attrs,
+    outputs = {
+        "stripped_binary": "%{name}.stripped",
+        "dwp_file": "%{name}.dwp",
+    },
+    fragments = ["cpp"] + semantics.additional_fragments(),
+    exec_groups = {
+        "cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
+    },
+    toolchains = cc_helper.use_cpp_toolchain() +
+                 semantics.get_runtimes_toolchain(),
+    provides = [CcInfo],
+    executable = True,
+)

src/main/starlark/builtins_bzl/common/cc/cc_binary_attrs.bzl

@@ -16,12 +16,12 @@
 """
 
 load(":common/cc/cc_info.bzl", "CcInfo")
-load(":common/cc/cc_shared_library.bzl", "CcSharedLibraryInfo", "graph_structure_aspect")
+load(":common/cc/cc_shared_library.bzl", "dynamic_deps_attrs")
 load(":common/cc/semantics.bzl", "semantics")
 
 cc_internal = _builtins.internal.cc_internal
 
-cc_binary_attrs_with_aspects = {
+cc_binary_attrs = {
     "srcs": attr.label_list(
         flags = ["DIRECT_COMPILE_TIME_INPUT"],
         allow_files = True,
@@ -54,26 +54,19 @@ cc_binary_attrs_with_aspects = {
         allow_rules = semantics.ALLOWED_RULES_IN_DEPS + semantics.ALLOWED_RULES_WITH_WARNINGS_IN_DEPS,
         flags = ["SKIP_ANALYSIS_TIME_FILETYPE_CHECK"],
         providers = [CcInfo],
-        aspects = [graph_structure_aspect],
-    ),
-    "dynamic_deps": attr.label_list(
-        allow_files = False,
-        providers = [CcSharedLibraryInfo],
     ),
     "malloc": attr.label(
         default = Label("@" + semantics.get_repo() + "//tools/cpp:malloc"),
         allow_files = False,
         providers = [CcInfo],
-        aspects = [graph_structure_aspect],
+        allow_rules = ["cc_library"],
     ),
     "_default_malloc": attr.label(
         default = configuration_field(fragment = "cpp", name = "custom_malloc"),
-        aspects = [graph_structure_aspect],
     ),
     "link_extra_lib": attr.label(
         default = Label("@" + semantics.get_repo() + "//tools/cpp:link_extra_lib"),
         providers = [CcInfo],
-        aspects = [graph_structure_aspect],
     ),
     "stamp": attr.int(
         values = [-1, 0, 1],
@@ -98,24 +91,5 @@ cc_binary_attrs_with_aspects = {
     "_use_auto_exec_groups": attr.bool(default = True),
 }
 
-cc_binary_attrs_with_aspects.update(semantics.get_distribs_attr())
-
-# Update attributes to contain no aspect implementation.
-cc_binary_attrs_without_aspects = dict(cc_binary_attrs_with_aspects)
-cc_binary_attrs_without_aspects["deps"] = attr.label_list(
-    allow_files = semantics.ALLOWED_FILES_IN_DEPS,
-    allow_rules = semantics.ALLOWED_RULES_IN_DEPS + semantics.ALLOWED_RULES_WITH_WARNINGS_IN_DEPS,
-    flags = ["SKIP_ANALYSIS_TIME_FILETYPE_CHECK"],
-    providers = [CcInfo],
-)
-cc_binary_attrs_without_aspects["malloc"] = attr.label(
-    default = Label("@" + semantics.get_repo() + "//tools/cpp:malloc"),
-    allow_files = False,
-    allow_rules = ["cc_library"],
-)
-cc_binary_attrs_without_aspects["_default_malloc"] = attr.label(
-    default = configuration_field(fragment = "cpp", name = "custom_malloc"),
-)
-cc_binary_attrs_without_aspects["link_extra_lib"] = attr.label(
-    default = Label("@" + semantics.get_repo() + "//tools/cpp:link_extra_lib"),
-)
+cc_binary_attrs.update(dynamic_deps_attrs)
+cc_binary_attrs.update(semantics.get_distribs_attr())

src/main/starlark/builtins_bzl/common/cc/cc_shared_library.bzl

@@ -840,6 +840,34 @@ cc_shared_library = rule(
     fragments = ["cpp"] + semantics.additional_fragments(),
 )
 
+def cc_shared_library_initializer(**kwargs):
+    """Initializes dynamic_deps_attrs"""
+    if "dynamic_deps" in kwargs and cc_helper.is_non_empty_list_or_select(kwargs["dynamic_deps"], "dynamic_deps"):
+        # Propagate an aspect if dynamic_deps attribute is specified.
+        all_deps = []
+        if "deps" in kwargs:
+            all_deps.extend(kwargs["deps"])
+
+        if "linkshared" not in kwargs or not kwargs["linkshared"]:
+            if "link_extra_lib" in kwargs:
+                all_deps.append(kwargs["link_extra_lib"])
+            if "malloc" in kwargs:
+                all_deps.append(kwargs["malloc"])
+
+        return kwargs | {"_deps_analyzed_by_graph_structure_aspect": all_deps}
+    return kwargs
+
+dynamic_deps_attrs = {
+    "dynamic_deps": attr.label_list(
+        allow_files = False,
+        providers = [CcSharedLibraryInfo],
+    ),
+    "_deps_analyzed_by_graph_structure_aspect": attr.label_list(
+        providers = [CcInfo],
+        aspects = [graph_structure_aspect],
+    ),
+}
+
 for_testing_dont_use_check_if_target_under_path = _check_if_target_under_path
 merge_cc_shared_library_infos = _merge_cc_shared_library_infos
 build_link_once_static_libs_map = _build_link_once_static_libs_map

src/main/starlark/builtins_bzl/common/cc/cc_test.bzl

@@ -15,7 +15,8 @@
 """cc_test Starlark implementation."""
 
 load(":common/cc/cc_binary.bzl", "cc_binary_impl")
-load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs_with_aspects", "cc_binary_attrs_without_aspects")
+load(":common/cc/cc_binary_attrs.bzl", "cc_binary_attrs")
+load(":common/cc/cc_shared_library.bzl", "cc_shared_library_initializer")
 load(":common/cc/cc_helper.bzl", "cc_helper")
 load(":common/cc/semantics.bzl", "semantics")
 load(":common/paths.bzl", "paths")
@@ -77,58 +78,44 @@ def _impl(ctx):
     providers.extend(test_providers)
     return providers
 
-def make_cc_test(with_aspects = False):
-    """Makes one of the cc_test rule variants.
-
-    This function shall only be used internally in CC ruleset.
-
-    Args:
-      with_aspects: Attaches graph_structure_aspect to `deps` attribute and
-        implicit deps.
-    Returns:
-      A cc_test rule class.
-    """
-    _cc_test_attrs = None
-    if with_aspects:
-        _cc_test_attrs = dict(cc_binary_attrs_with_aspects)
-    else:
-        _cc_test_attrs = dict(cc_binary_attrs_without_aspects)
-
-    # Update cc_test defaults:
-    _cc_test_attrs.update(
-        _is_test = attr.bool(default = True),
-        _apple_constraints = attr.label_list(
-            default = [
-                "@" + paths.join(semantics.get_platforms_root(), "os:ios"),
-                "@" + paths.join(semantics.get_platforms_root(), "os:macos"),
-                "@" + paths.join(semantics.get_platforms_root(), "os:tvos"),
-                "@" + paths.join(semantics.get_platforms_root(), "os:watchos"),
-            ],
-        ),
-        # Starlark tests don't get `env_inherit` by default.
-        env_inherit = attr.string_list(),
-        stamp = attr.int(values = [-1, 0, 1], default = 0),
-        linkstatic = attr.bool(default = False),
-    )
-    _cc_test_attrs.update(semantics.get_test_malloc_attr())
-    _cc_test_attrs.update(semantics.get_coverage_attrs())
-
-    return rule(
-        implementation = _impl,
-        attrs = _cc_test_attrs,
-        outputs = {
-            # TODO(b/198254254): Handle case for windows.
-            "stripped_binary": "%{name}.stripped",
-            "dwp_file": "%{name}.dwp",
-        },
-        fragments = ["cpp", "coverage"] + semantics.additional_fragments(),
-        exec_groups = {
-            "cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
-            # testing.ExecutionInfo defaults to an exec_group of "test".
-            "test": exec_group(toolchains = [config_common.toolchain_type(_CC_TEST_TOOLCHAIN_TYPE, mandatory = False)]),
-        },
-        toolchains = [] +
-                     cc_helper.use_cpp_toolchain() +
-                     semantics.get_runtimes_toolchain(),
-        test = True,
-    )
+_cc_test_attrs = dict(cc_binary_attrs)
+
+# Update cc_test defaults:
+_cc_test_attrs.update(
+    _is_test = attr.bool(default = True),
+    _apple_constraints = attr.label_list(
+        default = [
+            "@" + paths.join(semantics.get_platforms_root(), "os:ios"),
+            "@" + paths.join(semantics.get_platforms_root(), "os:macos"),
+            "@" + paths.join(semantics.get_platforms_root(), "os:tvos"),
+            "@" + paths.join(semantics.get_platforms_root(), "os:watchos"),
+        ],
+    ),
+    # Starlark tests don't get `env_inherit` by default.
+    env_inherit = attr.string_list(),
+    stamp = attr.int(values = [-1, 0, 1], default = 0),
+    linkstatic = attr.bool(default = False),
+)
+_cc_test_attrs.update(semantics.get_test_malloc_attr())
+_cc_test_attrs.update(semantics.get_coverage_attrs())
+
+cc_test = rule(
+    initializer = cc_shared_library_initializer,
+    implementation = _impl,
+    attrs = _cc_test_attrs,
+    outputs = {
+        # TODO(b/198254254): Handle case for windows.
+        "stripped_binary": "%{name}.stripped",
+        "dwp_file": "%{name}.dwp",
+    },
+    fragments = ["cpp", "coverage"] + semantics.additional_fragments(),
+    exec_groups = {
+        "cpp_link": exec_group(toolchains = cc_helper.use_cpp_toolchain()),
+        # testing.ExecutionInfo defaults to an exec_group of "test".
+        "test": exec_group(toolchains = [config_common.toolchain_type(_CC_TEST_TOOLCHAIN_TYPE, mandatory = False)]),
+    },
+    toolchains = [] +
+                 cc_helper.use_cpp_toolchain() +
+                 semantics.get_runtimes_toolchain(),
+    test = True,
+)