diff --git a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java index a139c8413af150..8ff682ab12deb0 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Attribute.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Attribute.java @@ -1774,7 +1774,7 @@ public static LabelListLateBoundDefault fromRuleAndAttributesOnly( // 5. defaultValue instanceof LateBoundDefault && // type.isValid(defaultValue.getDefault(configuration)) // (We assume a hypothetical Type.isValid(Object) predicate.) - private final Object defaultValue; + @Nullable private final Object defaultValue; private final TransitionFactory transitionFactory; @@ -2140,6 +2140,7 @@ public Object getDefaultValue(@Nullable Rule rule) { * Returns the default value of this attribute, even if it is a computed default, or a late-bound * default. */ + @Nullable public Object getDefaultValueUnchecked() { return defaultValue; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java b/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java index 75b2973cd4f0e9..11cc69ee969003 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/MacroClass.java @@ -196,7 +196,8 @@ private MacroInstance instantiateMacro(Package.Builder pkgBuilder, Map entry : ImmutableMap.copyOf(attrValues).entrySet()) { String attrName = entry.getKey(); - Attribute attribute = attributes.get(attrName); - Object normalizedValue = - // copyAndLiftStarlarkValue ensures immutability. - BuildType.copyAndLiftStarlarkValue( - name, attribute, entry.getValue(), pkgBuilder.getLabelConverter()); - // TODO(#19922): Validate that LABEL_LIST type attributes don't contain duplicates, to match - // the behavior of rules. This probably requires factoring out logic from - // AggregatingAttributeMapper. - if (attribute.isConfigurable() && !(normalizedValue instanceof SelectorList)) { - normalizedValue = SelectorList.wrapSingleValue(normalizedValue); + Object value = entry.getValue(); + // Skip auto-populated `None`s. They are not type-checked or lifted to select()s. + if (value != Starlark.NONE) { + Attribute attribute = attributes.get(attrName); + Object normalizedValue = + // copyAndLiftStarlarkValue ensures immutability. + BuildType.copyAndLiftStarlarkValue( + name, attribute, value, pkgBuilder.getLabelConverter()); + // TODO(#19922): Validate that LABEL_LIST type attributes don't contain duplicates, to match + // the behavior of rules. This probably requires factoring out logic from + // AggregatingAttributeMapper. + if (attribute.isConfigurable() && !(normalizedValue instanceof SelectorList)) { + normalizedValue = SelectorList.wrapSingleValue(normalizedValue); + } + attrValues.put(attrName, normalizedValue); } - attrValues.put(attrName, normalizedValue); } // Type and existence enforced by RuleClass.NAME_ATTRIBUTE. diff --git a/src/main/java/com/google/devtools/build/lib/packages/Type.java b/src/main/java/com/google/devtools/build/lib/packages/Type.java index 1c21e68c158819..b619ebee5451f8 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Type.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Type.java @@ -179,6 +179,7 @@ public final T convertOptional(Object x, String what) throws ConversionException * Returns the default value for this type; may return null iff no default is defined for this * type. */ + @Nullable public abstract T getDefaultValue(); /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD index 1ccd4b5fa99e7a..491bc1e3837b03 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD @@ -367,6 +367,7 @@ java_test( java_test( name = "SymbolicMacroTest", srcs = ["SymbolicMacroTest.java"], + shard_count = 5, deps = [ "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis:test/analysis_failure", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java b/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java index 06bfd7de698c0f..dafe7836ce225c 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java @@ -713,6 +713,35 @@ def query(): assertContainsEvent("existing_rules() keys: [\"outer_target\"]"); } + @Test + public void hardcodedDefaultAttrValue_isUsedWhenNotOverriddenAndAttrHasNoUserSpecifiedDefault() + throws Exception { + + scratch.file( + "pkg/foo.bzl", + """ + def _impl(name, dep): + print("dep is %s" % dep) + my_macro = macro( + implementation=_impl, + attrs = { + # Test label type, since LabelType#getDefaultValue returns null. + "dep": attr.label(configurable=False) + }, + ) + """); + scratch.file( + "pkg/BUILD", + """ + load(":foo.bzl", "my_macro") + my_macro(name="abc") + """); + + Package pkg = getPackage("pkg"); + assertPackageNotInError(pkg); + assertContainsEvent("dep is None"); + } + @Test public void defaultAttrValue_isUsedWhenNotOverridden() throws Exception { scratch.file(