Use rule extension API

examples/cc_asan_test_with_reset/BUILD.bazel → examples/cc_asan_test_with_automatic_reset/BUILD.bazel

@@ -44,7 +44,7 @@ cc_library(
 # original top-level settings, that is, without AddressSanitizer.
 cc_asan_test_reset(
     name = "large_dep_reset",
-    exports = "//cc_asan_test_with_reset/third_party:large_dep",
+    exports = "//cc_asan_test_with_automatic_reset/third_party:large_dep",
 )
 
 original_settings(

examples/cc_asan_test_with_automatic_reset/asan_test.cpp

@@ -0,0 +1,9 @@
+#include "cc_asan_test_with_automatic_reset/lib.h"
+
+#include <iostream>
+
+int main() {
+  trigger_asan();
+  std::cerr << "Did not get expected AddressSanitizer error, is the test instrumented correctly?" << std::endl;
+  return 1;
+}

examples/cc_asan_test_with_automatic_reset/lib.cpp

@@ -0,0 +1,8 @@
+#include "cc_asan_test_with_automatic_reset/lib.h"
+
+int trigger_asan() {
+  int* array = new int[100] {};
+  delete[] array;
+  // Triggers a heap-use-after-free error.
+  return array[0];
+}

examples/cc_asan_test_with_automatic_reset/third_party/BUILD.bazel

@@ -0,0 +1,7 @@
+# This is a mock of a large third-party dependency that should not be built with
+# AddressSanitizer to reduce build time.
+cc_library(
+    name = "large_dep",
+    srcs = ["large_dep.cpp"],
+    visibility = ["//cc_asan_test_with_automatic_reset:__pkg__"],
+)

examples/cc_asan_test_with_manual_reset/BUILD.bazel

@@ -0,0 +1,33 @@
+load("@with_cfg.bzl", "original_settings")
+load(":cc_asan_test.bzl", "cc_asan_test", "cc_asan_test_reset")
+
+cc_asan_test(
+    name = "asan_test",
+    srcs = ["asan_test.cpp"],
+    env = {
+        # Effectively invert the exit code so that the test passes if and only
+        # if the expected ASAN error is detected.
+        "ASAN_OPTIONS": "exitcode=0:abort_on_error=0",
+    },
+    deps = [
+        ":lib",
+    ],
+)
+
+cc_library(
+    name = "lib",
+    srcs = ["lib.cpp"],
+    hdrs = ["lib.h"],
+    deps = [":large_dep_reset"],
+)
+
+# This ensures that the "large" third-party dependency is built with the
+# original top-level settings, that is, without AddressSanitizer.
+cc_asan_test_reset(
+    name = "large_dep_reset",
+    exports = "//cc_asan_test_with_manual_reset/third_party:large_dep",
+)
+
+original_settings(
+    name = "cc_asan_test_original_settings",
+)

examples/cc_asan_test_with_reset/asan_test.cpp → examples/cc_asan_test_with_manual_reset/asan_test.cpp

@@ -1,4 +1,4 @@
-#include "cc_asan_test_with_reset/lib.h"
+#include "cc_asan_test_with_manual_reset/lib.h"
 
 #include <iostream>
 

examples/cc_asan_test_with_manual_reset/cc_asan_test.bzl

@@ -0,0 +1,11 @@
+load("@with_cfg.bzl", "with_cfg")
+
+_builder = with_cfg(native.cc_test)
+_builder.extend("copt", ["-fsanitize=address"])
+_builder.extend("linkopt", select({
+    # link.exe doesn't require or recognize -fsanitize=address and would emit a warning.
+    Label("@rules_cc//cc/compiler:msvc-cl"): [],
+    "//conditions:default": ["-fsanitize=address"],
+}))
+_builder.resettable(Label(":cc_asan_test_original_settings"))
+cc_asan_test, cc_asan_test_reset = _builder.build()

examples/cc_asan_test_with_reset/lib.cpp → examples/cc_asan_test_with_manual_reset/lib.cpp

@@ -1,4 +1,4 @@
-#include "cc_asan_test_with_reset/lib.h"
+#include "cc_asan_test_with_manual_reset/lib.h"
 
 int trigger_asan() {
   int* array = new int[100] {};

examples/cc_asan_test_with_manual_reset/lib.h

@@ -0,0 +1,3 @@
+#pragma once
+
+int trigger_asan();

examples/cc_asan_test_with_reset/third_party/BUILD.bazel → examples/cc_asan_test_with_manual_reset/third_party/BUILD.bazel

@@ -3,5 +3,5 @@
 cc_library(
     name = "large_dep",
     srcs = ["large_dep.cpp"],
-    visibility = ["//cc_asan_test_with_reset:__pkg__"],
+    visibility = ["//cc_asan_test_with_manual_reset:__pkg__"],
 )

examples/cc_asan_test_with_manual_reset/third_party/large_dep.cpp

@@ -0,0 +1,10 @@
+// This file is set up to fail when compiled with AddressSanitizer. We use it to
+// simulate a large third-party library that shouldn't be instrumented with
+// sanitizers.
+
+#ifndef __has_feature
+  #define __has_feature(x) 0  // Compatibility with non-clang compilers.
+#endif
+#if __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
+#error "large_dep.cpp should not be compiled with ASAN"
+#endif

with_cfg/private/BUILD.bazel

@@ -43,6 +43,15 @@ bzl_library(
     ],
 )
 
+bzl_library(
+    name = "extend",
+    srcs = ["extend.bzl"],
+    visibility = ["//with_cfg:__subpackages__"],
+    deps = [
+        ":setting",
+    ],
+)
+
 bzl_library(
     name = "frontend",
     srcs = ["frontend.bzl"],
@@ -138,6 +147,7 @@ bzl_library(
     visibility = ["//with_cfg:__subpackages__"],
     deps = [
         ":builder",
+        ":extend",
         ":providers",
         ":rule_defaults",
     ],

with_cfg/private/builder.bzl

@@ -1,3 +1,5 @@
+load("@bazel_features//:features.bzl", "bazel_features")
+load(":extend.bzl", "make_transitioned_rule")
 load(":frontend.bzl", "get_frontend")
 load(":select.bzl", "map_attr")
 load(":setting.bzl", "validate_and_get_attr_name")
@@ -85,6 +87,24 @@ def _build(*, rule_info, values, operations, mutable_has_been_built, mutable_ori
         operations = operations,
         original_settings_label = original_settings_label,
     )
+
+    # Resetting attributes is not yet supported with the extended rule approach.
+    if bazel_features.rules.rule_extension_apis_available and rule_info.supports_extension and not attrs_to_reset:
+        transitioned_rule = make_transitioned_rule(
+            rule_info = rule_info,
+            transition = transition,
+            values = values,
+        )
+        if original_settings_label:
+            transitioning_alias = make_transitioning_alias(
+                providers = rule_info.providers,
+                transition = transition,
+                values = values,
+                original_settings_label = original_settings_label,
+            )
+            return transitioned_rule, transitioning_alias
+        return transitioned_rule, None
+
     transitioning_alias = make_transitioning_alias(
         providers = rule_info.providers,
         transition = transition,

with_cfg/private/extend.bzl

@@ -0,0 +1,25 @@
+load(":setting.bzl", "get_attr_type", "validate_and_get_attr_name")
+
+visibility("private")
+
+def make_transitioned_rule(*, rule_info, transition, values):
+    settings_attrs = {
+        validate_and_get_attr_name(setting): getattr(attr, get_attr_type(value))()
+        for setting, value in values.items()
+    }
+    return rule(
+        implementation = _transitioned_rule_impl,
+        parent = rule_info.kind,
+        cfg = transition,
+        initializer = lambda **kwargs: _initializer_base(kwargs = kwargs, values = values),
+        attrs = settings_attrs,
+    )
+
+def _transitioned_rule_impl(ctx):
+    return ctx.super()
+
+def _initializer_base(*, kwargs, values):
+    return kwargs | {
+        validate_and_get_attr_name(setting): value
+        for setting, value in values.items()
+    }

with_cfg/private/providers.bzl

@@ -9,6 +9,7 @@ RuleInfo = provider(fields = [
     "kind",
     "native",
     "providers",
+    "supports_extension",
     "supports_inheritance",
     "test",
 ])

with_cfg/private/setting.bzl

@@ -42,11 +42,11 @@ def validate_and_get_attr_name(setting):
             fail("Custom build settings can only be referenced via Labels, did you mean Label({})?".format(repr(stripped_setting)))
         if setting.startswith("-"):
             fail("\"{}\" is not a valid setting, did you mean \"{}\"?".format(setting, stripped_setting))
-        if setting == "features":
-            # features is a special case as it is both a command-line option (--features) and a
-            # special attribute on all rules. Avoids "built-in attributes cannot be overridden".
-            return "features_attr"
-        return setting
+
+        # Avoid collisions with existing attributes in two cases:
+        # 1. The setting is `features`, which is also an attribute common to all rules.
+        # 2. Rule extensions are used and the extended rule has an attribute with the same name.
+        return "with_cfg_" + setting
     else:
         fail("Expected setting to be a Label or a string, got: {} ({})".format(repr(setting), type(setting)))
 

with_cfg/private/transition.bzl

@@ -26,7 +26,9 @@ def _make_transition_impl(*, operations, original_settings_label):
     )
 
 def _transition_base_impl(settings, attr, *, operations, original_settings_label):
-    if original_settings_label and attr.internal_only_reset:
+    # internal_only_reset may be missing if this transition is attached to an extended rule rather
+    # than a transitioning alias.
+    if original_settings_label and getattr(attr, "internal_only_reset", False):
         original_settings = settings[str(original_settings_label)]
         if original_settings:
             # The reset rule is used in the transitive closure of the transitioning rule. Reset the

with_cfg/private/transitioning_alias.bzl

@@ -12,10 +12,12 @@ def make_transitioning_alias(*, providers, transition, values, original_settings
     if original_settings_label:
         resettable_attrs = {
             "internal_only_reset": attr.bool(default = True),
+            # Fail if the original_settings_label doesn't have the correct provider.
             "_original_settings": attr.label(
                 default = original_settings_label,
                 providers = [OriginalSettingsInfo],
             ),
+            # Fail if --experimental_output_directory_naming_scheme=diff_against_dynamic_baseline isn't set.
             "_check_for_diff_against_dynamic_baseline": attr.label(
                 default = ":resettable_check_for_diff_against_dynamic_baseline",
             ),

with_cfg/private/with_cfg.bzl

@@ -128,6 +128,7 @@ def with_cfg(
         providers = DEFAULT_PROVIDERS + extra_providers,
         native = _is_native(kind),
         supports_inheritance = _supports_inheritance(kind),
+        supports_extension = _supports_extension(kind),
     )
     return make_builder(rule_info)
 
@@ -160,5 +161,16 @@ def _supports_inheritance(kind):
     # Legacy macros don't support inheritance.
     return not str(kind).startswith("<function ")
 
+# Rules that need https://github.com/bazelbuild/bazel/pull/24778 to be extendable.
+_NOT_EXTENDABLE = [
+    # keep sorted
+    "cc_binary",
+    "cc_test",
+]
+
+def _supports_extension(kind):
+    kind_str = str(kind)
+    return kind_str.startswith("<rule ") and get_rule_name(kind_str) not in _NOT_EXTENDABLE
+
 def get_implicit_targets(rule_name):
     return IMPLICIT_TARGETS.get(rule_name, [])

with_cfg/tests/setting_test.bzl

@@ -62,7 +62,7 @@ def _get_attr_type_test(env):
         env.expect.where(value = value).that_str(get_attr_type(value)).equals(expected_type)
 
 def _get_attr_name_test(env):
-    env.expect.that_str(validate_and_get_attr_name("platforms")).equals("platforms")
+    env.expect.that_str(validate_and_get_attr_name("platforms")).contains("platforms")
 
     some_setting_subject = env.expect.that_str(validate_and_get_attr_name(Label("@bazel_tools//:some_setting")))
     some_setting_subject.contains("some_setting")