From 33439dc24284678f1a127665d9e6554b2bbe7966 Mon Sep 17 00:00:00 2001 From: janakr Date: Thu, 22 Mar 2018 13:44:05 -0700 Subject: Use an immutable Attribute factory in objects that are persisted to Skyframe, rather than the potentially mutable builder, and @AutoCodec SkylarkAttr.Descriptor. PiperOrigin-RevId: 190118565 --- .../devtools/build/lib/packages/Attribute.java | 167 ++++++++++++++++----- 1 file changed, 126 insertions(+), 41 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/packages') 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 b7f2ca5f42..717b74bedc 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 @@ -361,6 +361,108 @@ public final class Attribute implements Comparable { return new Builder<>(name, type); } + /** A factory to generate {@link Attribute} instances. */ + @AutoCodec + public static class ImmutableAttributeFactory { + private final Type type; + private final ConfigurationTransition configTransition; + private final RuleClassNamePredicate allowedRuleClassesForLabels; + private final RuleClassNamePredicate allowedRuleClassesForLabelsWarning; + private final SplitTransitionProvider splitTransitionProvider; + private final FileTypeSet allowedFileTypesForLabels; + private final ValidityPredicate validityPredicate; + private final Object value; + private final AttributeValueSource valueSource; + private final boolean valueSet; + private final Predicate condition; + private final ImmutableSet propertyFlags; + private final PredicateWithMessage allowedValues; + private final RequiredProviders requiredProviders; + private final ImmutableList> aspects; + + @AutoCodec.VisibleForSerialization + ImmutableAttributeFactory( + Type type, + ImmutableSet propertyFlags, + Object value, + ConfigurationTransition configTransition, + RuleClassNamePredicate allowedRuleClassesForLabels, + RuleClassNamePredicate allowedRuleClassesForLabelsWarning, + SplitTransitionProvider splitTransitionProvider, + FileTypeSet allowedFileTypesForLabels, + ValidityPredicate validityPredicate, + AttributeValueSource valueSource, + boolean valueSet, + Predicate condition, + PredicateWithMessage allowedValues, + RequiredProviders requiredProviders, + ImmutableList> aspects) { + this.type = type; + this.configTransition = configTransition; + this.allowedRuleClassesForLabels = allowedRuleClassesForLabels; + this.allowedRuleClassesForLabelsWarning = allowedRuleClassesForLabelsWarning; + this.splitTransitionProvider = splitTransitionProvider; + this.allowedFileTypesForLabels = allowedFileTypesForLabels; + this.validityPredicate = validityPredicate; + this.value = value; + this.valueSource = valueSource; + this.valueSet = valueSet; + this.condition = condition; + this.propertyFlags = propertyFlags; + this.allowedValues = allowedValues; + this.requiredProviders = requiredProviders; + this.aspects = aspects; + } + + public AttributeValueSource getValueSource() { + return valueSource; + } + + public boolean isValueSet() { + return valueSet; + } + + public Attribute build(String name) { + Preconditions.checkState(!name.isEmpty(), "name has not been set"); + if (valueSource == AttributeValueSource.LATE_BOUND) { + Preconditions.checkState(isLateBound(name)); + } + // TODO(bazel-team): Set the default to be no file type, then remove this check, and also + // remove all allowedFileTypes() calls without parameters. + + // do not modify this.allowedFileTypesForLabels, instead create a copy. + FileTypeSet allowedFileTypesForLabels = this.allowedFileTypesForLabels; + if (type.getLabelClass() == LabelClass.DEPENDENCY) { + if (isPrivateAttribute(name) && allowedFileTypesForLabels == null) { + allowedFileTypesForLabels = FileTypeSet.ANY_FILE; + } + Preconditions.checkNotNull( + allowedFileTypesForLabels, "allowedFileTypesForLabels not set for %s", name); + } else if (type.getLabelClass() == LabelClass.OUTPUT) { + // TODO(bazel-team): Set the default to no file type and make explicit calls instead. + if (allowedFileTypesForLabels == null) { + allowedFileTypesForLabels = FileTypeSet.ANY_FILE; + } + } + + return new Attribute( + name, + type, + propertyFlags, + value, + configTransition, + splitTransitionProvider, + allowedRuleClassesForLabels, + allowedRuleClassesForLabelsWarning, + allowedFileTypesForLabels, + validityPredicate, + condition, + allowedValues, + requiredProviders, + aspects); + } + } + /** * A fluent builder for the {@code Attribute} instances. * @@ -1026,44 +1128,8 @@ public final class Attribute implements Comparable { return setPropertyFlag(PropertyFlag.NONCONFIGURABLE, "nonconfigurable"); } - /** - * Creates the attribute. Uses name, type, optionality, configuration type - * and the default value configured by the builder. - */ - public Attribute build() { - return build(this.name); - } - - /** - * Creates the attribute. Uses type, optionality, configuration type - * and the default value configured by the builder. Use the name - * passed as an argument. This function is used by Skylark where the - * name is provided only when we build. We don't want to modify the - * builder, as it is shared in a multithreaded environment. - */ - public Attribute build(String name) { - Preconditions.checkState(!name.isEmpty(), "name has not been set"); - if (valueSource == AttributeValueSource.LATE_BOUND) { - Preconditions.checkState(isLateBound(name)); - } - // TODO(bazel-team): Set the default to be no file type, then remove this check, and also - // remove all allowedFileTypes() calls without parameters. - - // do not modify this.allowedFileTypesForLabels, instead create a copy. - FileTypeSet allowedFileTypesForLabels = this.allowedFileTypesForLabels; - if (type.getLabelClass() == LabelClass.DEPENDENCY) { - if (isPrivateAttribute(name) && allowedFileTypesForLabels == null) { - allowedFileTypesForLabels = FileTypeSet.ANY_FILE; - } - Preconditions.checkNotNull( - allowedFileTypesForLabels, "allowedFileTypesForLabels not set for %s", name); - } else if (type.getLabelClass() == LabelClass.OUTPUT) { - // TODO(bazel-team): Set the default to no file type and make explicit calls instead. - if (allowedFileTypesForLabels == null) { - allowedFileTypesForLabels = FileTypeSet.ANY_FILE; - } - } - + /** Returns an {@link ImmutableAttributeFactory} that can be invoked to create attributes. */ + public ImmutableAttributeFactory buildPartial() { Preconditions.checkState( !allowedRuleClassesForLabels.consideredOverlapping(allowedRuleClassesForLabelsWarning), "allowedRuleClasses %s and allowedRuleClassesWithWarning %s " @@ -1071,22 +1137,41 @@ public final class Attribute implements Comparable { allowedRuleClassesForLabels, allowedRuleClassesForLabelsWarning); - return new Attribute( - name, + return new ImmutableAttributeFactory( type, Sets.immutableEnumSet(propertyFlags), valueSet ? value : type.getDefaultValue(), configTransition, - splitTransitionProvider, allowedRuleClassesForLabels, allowedRuleClassesForLabelsWarning, + splitTransitionProvider, allowedFileTypesForLabels, validityPredicate, + valueSource, + valueSet, condition, allowedValues, requiredProvidersBuilder.build(), ImmutableList.copyOf(aspects.values())); } + + /** + * Creates the attribute. Uses name, type, optionality, configuration type and the default value + * configured by the builder. + */ + public Attribute build() { + return build(this.name); + } + + /** + * Creates the attribute. Uses type, optionality, configuration type and the default value + * configured by the builder. Use the name passed as an argument. This function is used by + * Skylark where the name is provided only when we build. We don't want to modify the builder, + * as it is shared in a multithreaded environment. + */ + public Attribute build(String name) { + return buildPartial().build(name); + } } /** -- cgit v1.2.3