Skip to content

Commit

Permalink
Implement initializer (a.k.a macro_wrapper)
Browse files Browse the repository at this point in the history
Design doc: https://docs.google.com/document/d/1p6z-shWf9sdqo_ep7dcjZCGvqN5r2jsPkJCqHHgfRp4/edit

PiperOrigin-RevId: 563111797
Change-Id: I642fe788c5ee572c87a810769dddaf25e580367e
  • Loading branch information
comius authored and copybara-github committed Sep 6, 2023
1 parent 78f98e7 commit 45bac19
Show file tree
Hide file tree
Showing 6 changed files with 502 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ public class StarlarkRuleClassFunctions implements StarlarkRuleFunctionsApi {
.add(attr("output_licenses", LICENSE))
.build();

private static final ImmutableSet<AllowlistEntry> ALLOWLIST_INITIALIZER =
ImmutableSet.of(allowlistEntry("", "initializer_testing"));
private static final ImmutableSet<AllowlistEntry> ALLOWLIST_SUBRULES =
ImmutableSet.of(allowlistEntry("", "subrule_testing"));

Expand Down Expand Up @@ -288,6 +290,7 @@ public Object provider(Object doc, Object fields, Object init, StarlarkThread th
@Override
public StarlarkRuleFunction rule(
StarlarkFunction implementation,
Object initializer,
Boolean test,
Dict<?, ?> attrs,
Object implicitOutputs,
Expand Down Expand Up @@ -321,6 +324,10 @@ public StarlarkRuleFunction rule(
BuiltinRestriction.failIfCalledOutsideAllowlist(thread, ALLOWLIST_SUBRULES);
}

if (initializer != Starlark.NONE) {
BuiltinRestriction.failIfCalledOutsideAllowlist(thread, ALLOWLIST_INITIALIZER);
}

return createRule(
// Contextual parameters.
bazelContext,
Expand All @@ -332,6 +339,7 @@ public StarlarkRuleFunction rule(
thread.getSemantics(),
// rule() parameters
implementation,
initializer == Starlark.NONE ? null : (StarlarkFunction) initializer,
test,
attrs,
implicitOutputs,
Expand Down Expand Up @@ -371,6 +379,7 @@ public static StarlarkRuleFunction createRule(
StarlarkSemantics starlarkSemantics,
// Parameters that come from rule().
StarlarkFunction implementation,
@Nullable StarlarkFunction initializer,
boolean test,
Dict<?, ?> attrs,
Object implicitOutputs,
Expand Down Expand Up @@ -539,6 +548,7 @@ public static StarlarkRuleFunction createRule(
builder,
type,
attributes,
initializer,
loc,
Starlark.toJavaOptional(doc, String.class).map(Starlark::trimDocString));
}
Expand Down Expand Up @@ -750,6 +760,7 @@ public static final class StarlarkRuleFunction implements StarlarkExportable, Ru
private RuleClass ruleClass;
private final RuleClassType type;
private ImmutableList<Pair<String, StarlarkAttrModule.Descriptor>> attributes;
@Nullable private final StarlarkFunction initializer;
private final Location definitionLocation;
@Nullable private final String documentation;
private Label starlarkLabel;
Expand All @@ -762,6 +773,7 @@ public StarlarkRuleFunction(
RuleClass.Builder builder,
RuleClassType type,
ImmutableList<Pair<String, StarlarkAttrModule.Descriptor>> attributes,
@Nullable StarlarkFunction initializer,
Location definitionLocation,
Optional<String> documentation) {
this.builder = builder;
Expand All @@ -772,6 +784,7 @@ public StarlarkRuleFunction(
"Implicitly added attributes are expected to be built-in, not Starlark-defined");
this.type = type;
this.attributes = attributes;
this.initializer = initializer;
this.definitionLocation = definitionLocation;
this.documentation = documentation.orElse(null);
}
Expand Down Expand Up @@ -817,6 +830,39 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg

validateRulePropagatedAspects(ruleClass);

if (initializer != null) {
// TODO: b/298561048 - lift parameters to more accurate type - for example strings to Labels
// You might feel tempted to inspect the signature of the initializer function. The
// temptation might come from handling default values, making them work for better for the
// users.
// The less magic the better. Do not give in those temptations!
Dict.Builder<String, Object> initializerKwargs = Dict.builder();
for (var attr : ruleClass.getAttributes()) {
if (attr.isPublic() && attr.starlarkDefined()) {
if (kwargs.containsKey(attr.getName())) {
initializerKwargs.put(attr.getName(), kwargs.get(attr.getName()));
}
}
}
Object ret =
Starlark.call(
thread, initializer, Tuple.of(), initializerKwargs.build(thread.mutability()));
Dict<String, Object> newKwargs =
ret == Starlark.NONE
? Dict.empty()
: Dict.cast(ret, String.class, Object.class, "rule's initializer return value");

for (var arg : newKwargs.keySet()) {
Attribute attr = ruleClass.getAttributeByNameMaybe(arg);
if (attr != null && !(attr.isPublic() && attr.starlarkDefined())) {
throw Starlark.errorf(
"Initializer can only set public, Starlark defined attributes, not '%s'", arg);
}
}

kwargs.putEntries(newKwargs);
}

BuildLangTypedAttributeValuesMap attributeValues =
new BuildLangTypedAttributeValuesMap(kwargs);
try {
Expand Down Expand Up @@ -951,6 +997,7 @@ public void export(EventHandler handler, Label starlarkLabel, String ruleClassNa
errorf(handler, "cannot add attribute: %s", ex.getMessage());
}
}

// TODO(b/121385274): remove when we stop allowlisting starlark transitions
if (hasStarlarkDefinedTransition) {
if (!starlarkLabel.getRepository().getNameWithAt().equals("@_builtins")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.AspectsListBuilder.AspectDetails;
import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault.Resolver;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassNamePredicate;
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.packages.Type.LabelClass;
Expand Down Expand Up @@ -2183,6 +2182,10 @@ boolean hasComputedDefault() {
|| defaultValue instanceof StarlarkComputedDefaultTemplate;
}

public boolean isPublic() {
return !isPrivateAttribute(name);
}

/**
* Returns if this attribute is an implicit dependency according to the naming policy that
* designates implicit attributes.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ public void analysisTest(
thread.getSemantics(),
// rule() parameters.
implementation,
/* initializer= */ null,
/* test= */ true,
attrs,
/* implicitOutputs= */ Starlark.NONE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,27 @@ Object provider(Object doc, Object fields, Object init, StarlarkThread thread)
+ " analysis phase for each instance of the rule. It can access the attributes"
+ " provided by the user. It must create actions to generate all the declared "
+ "outputs."),
@Param(
name = "initializer",
named = true,
defaultValue = "None",
doc =
"the Stalark function initializing the attributes of the rule. The function is "
+ "called at load time for each instance of the rule. It's called with values "
+ "of public attributes defined by the rule (not with generic attributes, "
+ "for example <code>name</code> or <code>tags</code>). It has to return a "
+ "dictionary from the attribute names to the desired values. The attributes "
+ " that are not returned are unaffected. Returning <code>None</code> as value"
+ " results in using the default value specified in the attribute definition."
+ "<p>Initializers are evaluated before the default values specified in an"
+ "attribute definition. Consequently, if a parameter in the initializer's "
+ "signature contains a default values, it overwrites the default from the "
+ "attribute definition (except if returning <code>None</code>)."
+ "<p>Similarly, if a parameter in the initializer's signature doesn't have a "
+ "default, the parameter will become mandatory. It's a good practice to omit"
+ " default/mandatory settings on an attribute definition in such cases."
+ "<p>It's a good practice to use <code>**kwargs</code> for attributes "
+ " that are not handled."),
@Param(
name = "test",
named = true,
Expand Down Expand Up @@ -453,6 +474,7 @@ Object provider(Object doc, Object fields, Object init, StarlarkThread thread)
useStarlarkThread = true)
StarlarkCallable rule(
StarlarkFunction implementation,
Object initializer,
Boolean test,
Dict<?, ?> attrs,
Object implicitOutputs,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public ProviderInfoWrapper forProviderInfo(
@Override
public StarlarkCallable rule(
StarlarkFunction implementation,
Object initializer,
Boolean test,
Dict<?, ?> attrs,
Object implicitOutputs,
Expand Down
Loading

0 comments on commit 45bac19

Please sign in to comment.