From 3fb43f1db1d1ae49f63c2a10dd7e8a15956ac6eb Mon Sep 17 00:00:00 2001 From: shahan Date: Thu, 27 Jan 2022 18:53:25 -0800 Subject: [PATCH] Deletes @AutoCodec annotation on AspectsListBuilder.StarlarkAspectDetails. This makes it necessary to serialize the parametersExtractor present in the base AspectDetails class from StarlarkDefinedAspect.getDefaultParametersExtractor. PiperOrigin-RevId: 424759672 --- .../lib/packages/AspectsListBuilder.java | 11 +-- .../lib/packages/StarlarkDefinedAspect.java | 75 ++++++++++--------- 2 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java b/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java index 04f7f31de42a8d..8b1f7fa3c30cd9 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AspectsListBuilder.java @@ -21,7 +21,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; import java.util.HashMap; @@ -111,7 +110,6 @@ public boolean validateTopLevelAspectsParameters(ImmutableMap as } /** Wraps the information necessary to construct an Aspect. */ - @VisibleForSerialization public abstract static class AspectDetails { final C aspectClass; final Function parametersExtractor; @@ -181,13 +179,10 @@ protected Aspect getTopLevelAspect(ImmutableMap aspectParameters } } - @VisibleForSerialization - @AutoCodec - static class StarlarkAspectDetails extends AspectDetails { + private static class StarlarkAspectDetails extends AspectDetails { private final StarlarkDefinedAspect aspect; - @VisibleForSerialization - StarlarkAspectDetails(StarlarkDefinedAspect aspect, String baseAspectName) { + private StarlarkAspectDetails(StarlarkDefinedAspect aspect, String baseAspectName) { super(aspect.getAspectClass(), aspect.getDefaultParametersExtractor(), baseAspectName); this.aspect = aspect; } @@ -237,7 +232,7 @@ public Aspect getTopLevelAspect(ImmutableMap aspectParameters) } } - @SerializationConstant @AutoCodec.VisibleForSerialization + @SerializationConstant @VisibleForSerialization static final Function EMPTY_FUNCTION = input -> AspectParameters.EMPTY; /** diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java index 14e0f8e12213fe..bb5454e49a8e00 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java @@ -223,46 +223,51 @@ public boolean isExported() { @Override public Function getDefaultParametersExtractor() { - return rule -> { - AttributeMap ruleAttrs = RawAttributeMapper.of(rule); - AspectParameters.Builder builder = new AspectParameters.Builder(); - for (Attribute aspectAttr : attributes) { - String param = aspectAttr.getName(); - if (Attribute.isImplicit(param) || Attribute.isLateBound(param)) { - // These attributes are the private matters of the aspect - continue; - } + // This is serialized along with AspectsListBuilder.StarlarkAspectDetail and should not be + // turned into a lambda. + return new Function() { + @Override + public AspectParameters apply(Rule rule) { + AttributeMap ruleAttrs = RawAttributeMapper.of(rule); + AspectParameters.Builder builder = new AspectParameters.Builder(); + for (Attribute aspectAttr : attributes) { + String param = aspectAttr.getName(); + if (Attribute.isImplicit(param) || Attribute.isLateBound(param)) { + // These attributes are the private matters of the aspect + continue; + } - Attribute ruleAttr = ruleAttrs.getAttributeDefinition(param); - if (paramAttributes.contains(aspectAttr.getName())) { - // These are preconditions because if they are false, RuleFunction.call() should - // already have generated an error. - Preconditions.checkArgument( - ruleAttr != null, - "Cannot apply aspect %s to %s that does not define attribute '%s'.", - getName(), - rule.getTargetKind(), - param); - Preconditions.checkArgument( - ruleAttr.getType() == Type.STRING - || ruleAttr.getType() == Type.INTEGER - || ruleAttr.getType() == Type.BOOLEAN, - "Cannot apply aspect %s to %s since attribute '%s' is not boolean, integer, nor" - + " string.", - getName(), - rule.getTargetKind(), - param); - } + Attribute ruleAttr = ruleAttrs.getAttributeDefinition(param); + if (paramAttributes.contains(aspectAttr.getName())) { + // These are preconditions because if they are false, RuleFunction.call() should + // already have generated an error. + Preconditions.checkArgument( + ruleAttr != null, + "Cannot apply aspect %s to %s that does not define attribute '%s'.", + getName(), + rule.getTargetKind(), + param); + Preconditions.checkArgument( + ruleAttr.getType() == Type.STRING + || ruleAttr.getType() == Type.INTEGER + || ruleAttr.getType() == Type.BOOLEAN, + "Cannot apply aspect %s to %s since attribute '%s' is not boolean, integer, nor" + + " string.", + getName(), + rule.getTargetKind(), + param); + } - if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) { - // If the attribute has a select() (which aspect attributes don't yet support), the - // error gets reported in RuleClass.checkAspectAllowedValues. - if (!ruleAttrs.isConfigurable(param)) { - builder.addAttribute(param, ruleAttrs.get(param, ruleAttr.getType()).toString()); + if (ruleAttr != null && ruleAttr.getType() == aspectAttr.getType()) { + // If the attribute has a select() (which aspect attributes don't yet support), the + // error gets reported in RuleClass.checkAspectAllowedValues. + if (!ruleAttrs.isConfigurable(param)) { + builder.addAttribute(param, ruleAttrs.get(param, ruleAttr.getType()).toString()); + } } } + return builder.build(); } - return builder.build(); }; }