Skip to content

Commit

Permalink
Put macro attribute inheritance behind an off-by-default --experiment…
Browse files Browse the repository at this point in the history
…al_enable_macro_inherit_attrs flag

We still have open questions about how macro attribute inheritance ought to
interact with the tracking of whether rule attributes were or were not
explicitly provided.

In effect, this re-opens #24066
Part of addressing #24319

RELNOTES: symbolic macro attribute inheritance is now marked experimental; set --experimental_enable_macro_inherit_attrs flag to enable it.
PiperOrigin-RevId: 696582223
Change-Id: I3d7cb434bf8fe2da9cd10019e6075990205e7153
  • Loading branch information
tetromino authored and copybara-github committed Nov 14, 2024
1 parent 12dc0b9 commit 08beb21
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,16 @@ public final class BuildLanguageOptions extends OptionsBase {
help = "If set to true, enables the `macro()` construct for defining symbolic macros.")
public boolean experimentalEnableFirstClassMacros;

@Option(
name = "experimental_enable_macro_inherit_attrs",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS,
help =
"If set to true, enables attribute inheritance in symbolic macros and the inherit_attrs"
+ " parameter in the macro() built-in")
public boolean experimentalEnableMacroInheritAttrs;

@Option(
name = "experimental_enable_scl_dialect",
defaultValue = "true",
Expand Down Expand Up @@ -857,6 +867,7 @@ public StarlarkSemantics toStarlarkSemantics() {
EXPERIMENTAL_SINGLE_PACKAGE_TOOLCHAIN_BINDING,
experimentalSinglePackageToolchainBinding)
.setBool(EXPERIMENTAL_ENABLE_FIRST_CLASS_MACROS, experimentalEnableFirstClassMacros)
.setBool(EXPERIMENTAL_ENABLE_MACRO_INHERIT_ATTRS, experimentalEnableMacroInheritAttrs)
.setBool(EXPERIMENTAL_ENABLE_SCL_DIALECT, experimentalEnableSclDialect)
.setBool(ENABLE_BZLMOD, enableBzlmod)
.setBool(ENABLE_WORKSPACE, enableWorkspace)
Expand Down Expand Up @@ -972,6 +983,8 @@ public StarlarkSemantics toStarlarkSemantics() {
"-experimental_single_package_toolchain_binding";
public static final String EXPERIMENTAL_ENABLE_FIRST_CLASS_MACROS =
"+experimental_enable_first_class_macros";
public static final String EXPERIMENTAL_ENABLE_MACRO_INHERIT_ATTRS =
"-experimental_enable_macro_inherit_attrs";
public static final String EXPERIMENTAL_ENABLE_SCL_DIALECT = "+experimental_enable_scl_dialect";
public static final String ENABLE_BZLMOD = "+enable_bzlmod";
public static final String ENABLE_WORKSPACE = "-enable_workspace";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,8 @@ site of the rule. Such attributes can be assigned a default value (as in
positional = false,
named = true,
defaultValue = "None",
enableOnlyWithFlag = BuildLanguageOptions.EXPERIMENTAL_ENABLE_MACRO_INHERIT_ATTRS,
valueWhenDisabled = "None",
doc =
"""
A rule symbol, macro symbol, or the name of a built-in common attribute list (see below) from which
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1582,8 +1582,33 @@ def _impl(name, visibility, **kwargs):
.doesNotContainKey("disabled_attr");
}

@Test
public void inheritAttrs_disabledByDefault() throws Exception {
scratch.file(
"pkg/foo.bzl",
"""
def _my_macro_impl(name, visibility, **kwargs):
pass
my_macro = macro(
implementation = _my_macro_impl,
inherit_attrs = native.cc_library,
)
""");
scratch.file(
"pkg/BUILD",
"""
load(":foo.bzl", "my_macro")
""");
reporter.removeHandler(failFastHandler);
assertThat(getPackage("pkg")).isNull();
assertContainsEvent(
"parameter 'inherit_attrs' is experimental and thus unavailable with the current flags");
}

@Test
public void inheritAttrs_fromInvalidSource_fails() throws Exception {
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
scratch.file(
"pkg/foo.bzl",
"""
Expand All @@ -1608,6 +1633,7 @@ def _my_macro_impl(name, visibility, **kwargs):

@Test
public void inheritAttrs_withoutKwargsInImplementation_fails() throws Exception {
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
scratch.file(
"pkg/foo.bzl",
"""
Expand All @@ -1632,6 +1658,7 @@ def _my_macro_impl(name, visibility, tags):

@Test
public void inheritAttrs_fromCommon_withOverrides() throws Exception {
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
scratch.file(
"pkg/my_macro.bzl",
"""
Expand Down Expand Up @@ -1694,6 +1721,7 @@ public void inheritAttrs_fromAnyNativeRule() throws Exception {
// * a new AttributeValueSource or a new attribute type is introduced, and symbolic macros
// cannot inherit an attribute with a default with this source or of such a type (to fix, add
// a check for it in MacroClass#forceDefaultToNone).
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
for (RuleClass ruleClass : getBuiltinRuleClasses(false)) {
if (ruleClass.getAttributes().isEmpty()) {
continue;
Expand Down Expand Up @@ -1771,6 +1799,7 @@ def _impl(name, visibility, **kwargs):

@Test
public void inheritAttrs_fromExportedStarlarkRule() throws Exception {
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
scratch.file(
"pkg/my_rule.bzl",
"""
Expand Down Expand Up @@ -1813,6 +1842,7 @@ def _my_macro_impl(name, visibility, **kwargs):

@Test
public void inheritAttrs_fromUnexportedStarlarkRule() throws Exception {
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
scratch.file(
"pkg/my_macro.bzl",
"""
Expand Down Expand Up @@ -1850,6 +1880,7 @@ def _my_macro_impl(name, visibility, **kwargs):

@Test
public void inheritAttrs_fromExportedMacro() throws Exception {
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
scratch.file(
"pkg/other_macro.bzl",
"""
Expand Down Expand Up @@ -1890,6 +1921,7 @@ def _my_macro_impl(name, visibility, **kwargs):

@Test
public void inheritAttrs_fromUnexportedMacro() throws Exception {
setBuildLanguageOptions("--experimental_enable_macro_inherit_attrs");
scratch.file(
"pkg/my_macro.bzl",
"""
Expand Down

1 comment on commit 08beb21

@tetromino
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bazel-io fork 8.0.0

Please sign in to comment.