diff options
9 files changed, 538 insertions, 161 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java index 4fab639d96..abaccccdff 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java @@ -42,6 +42,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.CollectionUtils; +import com.google.devtools.build.lib.packages.Attribute.ComputationLimiter; import com.google.devtools.build.lib.packages.BuildType.Selector; import com.google.devtools.build.lib.packages.BuildType.SelectorList; import com.google.devtools.build.lib.syntax.Type; @@ -55,6 +56,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; /** @@ -67,18 +69,12 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { private static final ImmutableSet<Type<?>> scalarTypes = ImmutableSet.of(INTEGER, STRING, LABEL, NODEP_LABEL, OUTPUT, BOOLEAN, TRISTATE, LICENSE); - /** - * Store for all of this rule's attributes that are non-configurable. These are - * unconditionally available to computed defaults no matter what dependencies - * they've declared. - */ - private final List<String> nonConfigurableAttributes; + private final Rule rule; private AggregatingAttributeMapper(Rule rule) { super(rule.getPackage(), rule.getRuleClassObject(), rule.getLabel(), rule.getAttributeContainer()); - - nonConfigurableAttributes = rule.getRuleClassObject().getNonConfigurableAttributes(); + this.rule = rule; } public static AggregatingAttributeMapper of(Rule rule) { @@ -86,6 +82,14 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { } /** + * Returns all of this rule's attributes that are non-configurable. These are unconditionally + * available to computed defaults no matter what dependencies they've declared. + */ + private List<String> getNonConfigurableAttributes() { + return rule.getRuleClassObject().getNonConfigurableAttributes(); + } + + /** * Override that also visits the rule's configurable attribute keys (which are themselves labels). * * <p>Note that we directly parse the selectors rather than just calling {@link #visitAttribute} @@ -325,16 +329,7 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { // or y1, then compute default values for the (x1,y1), (x1,y2), (x2,y1), and (x2,y2) cases. Attribute.ComputedDefault computedDefault = getComputedDefault(attributeName, type); if (computedDefault != null) { - // The depMaps list holds every assignment of possible values to the computed default's - // declared possibly-configurable dependencies. - List<Map<String, Object>> depMaps = visitAttributes(computedDefault.dependencies()); - - List<T> possibleValues = new ArrayList<>(); // Not ImmutableList.Builder: values may be null. - // For each combination, call getDefault on a specialized AttributeMap providing those values. - for (Map<String, Object> depMap : depMaps) { - possibleValues.add(type.cast(computedDefault.getDefault(mapBackedAttributeMap(depMap)))); - } - return possibleValues; + return computedDefault.getPossibleValues(type, rule); } // For any other attribute, just return its direct value. @@ -461,18 +456,20 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { * ] * </pre> * - * <p>This uses time and space exponential on the number of inputs. To guard against misuse, - * {@code attributes.size()} must be two or less. + * <p>The work done by this method may be limited by providing a {@link ComputationLimiter} that + * throws if too much work is attempted. */ - private List<Map<String, Object>> visitAttributes(List<String> attributes) { - Preconditions.checkState(attributes.size() <= 2); + <TException extends Exception> List<Map<String, Object>> visitAttributes( + List<String> attributes, ComputationLimiter<TException> limiter) throws TException { List<Map<String, Object>> depMaps = new LinkedList<>(); - visitAttributesInner(attributes, depMaps, ImmutableMap.<String, Object>of()); + AtomicInteger combinationsSoFar = new AtomicInteger(0); + visitAttributesInner( + attributes, depMaps, ImmutableMap.<String, Object>of(), combinationsSoFar, limiter); return depMaps; } /** - * A recursive function used in the implementation of {@link #visitAttributes(List)}. + * A recursive function used in the implementation of {@link #visitAttributes}. * * @param attributes a list of attributes that are not yet assigned values in the {@code * currentMap} parameter. @@ -480,10 +477,21 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { * will add newly discovered maps to the list. * @param currentMap a (possibly non-empty) map holding {attrName --> attrValue} assignments for * attributes not in the {@code attributes} list. + * @param combinationsSoFar a counter for all previously processed combinations of possible + * values. + * @param limiter a strategy to limit the work done by invocations of this method. */ - private void visitAttributesInner( - List<String> attributes, List<Map<String, Object>> mappings, Map<String, Object> currentMap) { + private <TException extends Exception> void visitAttributesInner( + List<String> attributes, + List<Map<String, Object>> mappings, + Map<String, Object> currentMap, + AtomicInteger combinationsSoFar, + ComputationLimiter<TException> limiter) + throws TException { if (attributes.isEmpty()) { + // Because this method uses exponential time/space on the number of inputs, we may limit + // the total number of method calls. + limiter.onComputationCount(combinationsSoFar.incrementAndGet()); // Recursive base case: store whatever's already been populated in currentMap. mappings.add(currentMap); return; @@ -500,7 +508,8 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { Map<String, Object> newMap = new HashMap<>(); newMap.putAll(currentMap); newMap.put(firstAttribute, value); - visitAttributesInner(attributes.subList(1, attributes.size()), mappings, newMap); + visitAttributesInner( + attributes.subList(1, attributes.size()), mappings, newMap, combinationsSoFar, limiter); } } @@ -511,14 +520,14 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { * configurable attribute that's not in {@code directMap} causes an {@link * IllegalArgumentException} to be thrown. */ - private AttributeMap mapBackedAttributeMap(final Map<String, Object> directMap) { + AttributeMap createMapBackedAttributeMap(final Map<String, Object> directMap) { final AggregatingAttributeMapper owner = AggregatingAttributeMapper.this; return new AttributeMap() { @Override public <T> T get(String attributeName, Type<T> type) { owner.checkType(attributeName, type); - if (nonConfigurableAttributes.contains(attributeName)) { + if (getNonConfigurableAttributes().contains(attributeName)) { return owner.get(attributeName, type); } if (!directMap.containsKey(attributeName)) { @@ -549,7 +558,7 @@ public class AggregatingAttributeMapper extends AbstractAttributeMapper { public Iterable<String> getAttributeNames() { return ImmutableList.<String>builder() .addAll(directMap.keySet()) - .addAll(nonConfigurableAttributes) + .addAll(getNonConfigurableAttributes()) .build(); } 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 996371494d..68540c8b87 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 @@ -22,14 +22,18 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.collect.Ordering; import com.google.common.collect.Sets; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; import com.google.devtools.build.lib.syntax.ClassObject; import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.EvalUtils; +import com.google.devtools.build.lib.syntax.Runtime; import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.syntax.Type.ConversionException; @@ -37,6 +41,7 @@ import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.StringUtil; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.EnumSet; @@ -45,6 +50,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.atomic.AtomicReference; import javax.annotation.concurrent.Immutable; /** @@ -627,16 +633,14 @@ public final class Attribute implements Comparable<Attribute> { } /** - * Sets the attribute default value to a computed default value - use - * this when the default value is a function of other attributes of the - * Rule. The type of the computed default value for a mandatory attribute - * must match the type parameter: (e.g. list=[], integer=0, string="", + * Sets the attribute default value to a computed default value - use this when the default + * value is a function of other attributes of the Rule. The type of the computed default value + * for a mandatory attribute must match the type parameter: (e.g. list=[], integer=0, string="", * label=null). The {@code defaultValue} implementation must be immutable. * - * <p>If computedDefault returns a Label that is a target, that target will - * become an implicit dependency of this Rule; we will load the target - * (and its dependencies) if it encounters the Rule and build the target if - * needs to apply the Rule. + * <p>If the computed default returns a Label that is a target, that target will become an + * implicit dependency of this Rule; we will load the target (and its dependencies) if it + * encounters the Rule and build the target if needs to apply the Rule. */ public Builder<TYPE> value(ComputedDefault defaultValue) { Preconditions.checkState(!valueSet, "the default value is already set"); @@ -647,6 +651,28 @@ public final class Attribute implements Comparable<Attribute> { } /** + * Sets the attribute default value to a Skylark computed default template. Like a native + * Computed Default, this allows a Skylark-defined Rule Class to specify that the default value + * of an attribute is a function of other attributes of the Rule. + * + * <p>During the loading phase, the computed default template will be specialized for each rule + * it applies to. Those rules' attribute values will not be references to {@link + * SkylarkComputedDefaultTemplate}s, but instead will be references to {@link + * SkylarkComputedDefault}s. + * + * <p>If the computed default returns a Label that is a target, that target will become an + * implicit dependency of this Rule; we will load the target (and its dependencies) if it + * encounters the Rule and build the target if needs to apply the Rule. + */ + public Builder<TYPE> value(SkylarkComputedDefaultTemplate skylarkComputedDefaultTemplate) { + Preconditions.checkState(!valueSet, "the default value is already set"); + value = skylarkComputedDefaultTemplate; + valueSource = AttributeValueSource.COMPUTED_DEFAULT; + valueSet = true; + return this; + } + + /** * Sets the attribute default value to be late-bound, i.e., it is derived from the build * configuration. */ @@ -1021,28 +1047,154 @@ public final class Attribute implements Comparable<Attribute> { } /** - * A computed default is a default value for a Rule attribute that is a - * function of other attributes of the rule. + * A strategy for dealing with too many computations, used when creating lookup tables for {@link + * ComputedDefault}s. * - * <p>Attributes whose defaults are computed are first initialized to the default - * for their type, and then the computed defaults are evaluated after all - * non-computed defaults have been initialized. There is no defined order - * among computed defaults, so they must not depend on each other. + * @param <TException> The type of exception this strategy throws if too many computations are + * attempted. + */ + interface ComputationLimiter<TException extends Exception> { + void onComputationCount(int count) throws TException; + } + + /** + * An implementation of {@link ComputationLimiter} that never throws. For use with + * natively-defined {@link ComputedDefault}s, which are limited in the number of configurable + * attributes they depend on, not on the number of different combinations of possible inputs. + */ + private static final ComputationLimiter<RuntimeException> NULL_COMPUTATION_LIMITER = + new ComputationLimiter<RuntimeException>() { + @Override + public void onComputationCount(int count) throws RuntimeException {} + }; + + /** Exception for computed default attributes that depend on too many configurable attributes. */ + private static class TooManyConfigurableAttributesException extends Exception { + TooManyConfigurableAttributesException(int max) { + super( + String.format( + "Too many configurable attributes to compute all possible values: " + + "Found more than %d possible values.", + max)); + } + } + + private static class FixedComputationLimiter + implements ComputationLimiter<TooManyConfigurableAttributesException> { + + /** Upper bound of the number of combinations of values for a computed default attribute. */ + private static final int COMPUTED_DEFAULT_MAX_COMBINATIONS = 64; + + private static final FixedComputationLimiter INSTANCE = new FixedComputationLimiter(); + + @Override + public void onComputationCount(int count) throws TooManyConfigurableAttributesException { + if (count > COMPUTED_DEFAULT_MAX_COMBINATIONS) { + throw new TooManyConfigurableAttributesException(COMPUTED_DEFAULT_MAX_COMBINATIONS); + } + } + } + + /** + * Specifies how values of {@link ComputedDefault} attributes are computed based on the values of + * other attributes. * - * <p>If a computed default reads the value of another attribute, at least one of - * the following must be true: + * <p>The {@code TComputeException} type parameter allows the two specializations of this class to + * describe whether and how their computations throw. For natively defined computed defaults, + * computation does not throw, but for Skylark-defined computed defaults, computation may throw + * {@link InterruptedException}. + */ + private abstract static class ComputationStrategy<TComputeException extends Exception> { + abstract Object compute(AttributeMap map) throws TComputeException; + + /** + * Returns a lookup table mapping from: + * + * <ul> + * <li>tuples of values that may be assigned by {@code rule} to attributes with names in {@code + * dependencies} (note that there may be more than one such tuple for any given rule, if any + * of the dependencies are configurable) + * </ul> + * + * <p>to: + * + * <ul> + * <li>the value {@link #compute(AttributeMap)} evaluates to when the provided {@link + * AttributeMap} contains the values specified by that assignment, or {@code null} if the + * {@link ComputationStrategy} failed to evaluate. + * </ul> + * + * <p>The lookup table contains a tuple for each possible assignment to the {@code dependencies} + * attributes. The meaning of each tuple is well-defined because {@code dependencies} is + * ordered. + * + * <p>This is useful because configurable attributes may have many possible values. During the + * loading phase a configurable attribute can't be resolved to a single value. Configuration + * information, needed to resolve such an attribute, is only available during analysis. However, + * any labels that a ComputedDefault attribute may evaluate to must be loaded during the loading + * phase. + */ + <T, TLimitException extends Exception> Map<List<Object>, T> computeValuesForAllCombinations( + List<String> dependencies, + Type<T> type, + Rule rule, + ComputationLimiter<TLimitException> limiter) + throws TComputeException, TLimitException { + // This will hold every (value1, value2, ..) combination of the declared dependencies. + // Collect those combinations. + AggregatingAttributeMapper mapper = AggregatingAttributeMapper.of(rule); + List<Map<String, Object>> depMaps = mapper.visitAttributes(dependencies, limiter); + // For each combination, call compute() on a specialized AttributeMap providing those + // values. + Map<List<Object>, T> valueMap = new HashMap<>(); + for (Map<String, Object> depMap : depMaps) { + AttributeMap attrMap = mapper.createMapBackedAttributeMap(depMap); + Object value = compute(attrMap); + List<Object> key = createDependencyAssignmentTuple(dependencies, attrMap); + valueMap.put(key, type.cast(value)); + } + return valueMap; + } + + /** + * Given an {@link AttributeMap}, containing an assignment to each attribute in {@code + * dependencies}, this returns a list of the assigned values, ordered as {@code dependencies} is + * ordered. + */ + static List<Object> createDependencyAssignmentTuple( + List<String> dependencies, AttributeMap attrMap) { + ArrayList<Object> tuple = new ArrayList<>(dependencies.size()); + for (String attrName : dependencies) { + Type<?> attrType = attrMap.getAttributeType(attrName); + tuple.add(attrMap.get(attrName, attrType)); + } + return tuple; + } + } + + /** + * A computed default is a default value for a Rule attribute that is a function of other + * attributes of the rule. + * + * <p>Attributes whose defaults are computed are first initialized to the default for their type, + * and then the computed defaults are evaluated after all non-computed defaults have been + * initialized. There is no defined order among computed defaults, so they must not depend on each + * other. + * + * <p>If a computed default reads the value of another attribute, at least one of the following + * must be true: * * <ol> - * <li>The other attribute must be declared in the computed default's constructor</li> - * <li>The other attribute must be non-configurable ({@link Builder#nonconfigurable}</li> + * <li>The other attribute must be declared in the computed default's constructor + * <li>The other attribute must be non-configurable ({@link Builder#nonconfigurable} * </ol> * - * <p>The reason for enforced declarations is that, since attribute values might be - * configurable, a computed default that depends on them may itself take multiple - * values. Since we have no access to a target's configuration at the time these values - * are computed, we need the ability to probe the default's *complete* dependency space. - * Declared dependencies allow us to do so sanely. Non-configurable attributes don't have - * this problem because their value is fixed and known even without configuration information. + * <p>The reason for enforced declarations is that, since attribute values might be configurable, + * a computed default that depends on them may itself take multiple values. Since we have no + * access to a target's configuration at the time these values are computed, we need the ability + * to probe the default's *complete* dependency space. Declared dependencies allow us to do so + * sanely. Non-configurable attributes don't have this problem because their value is fixed and + * known even without configuration information. * * <p>Implementations of this interface must be immutable. */ @@ -1054,7 +1206,7 @@ public final class Attribute implements Comparable<Attribute> { * configurable attribute values. */ public ComputedDefault() { - dependencies = ImmutableList.of(); + this(ImmutableList.<String>of()); } /** @@ -1062,7 +1214,7 @@ public final class Attribute implements Comparable<Attribute> { * explicitly specified configurable attribute value */ public ComputedDefault(String depAttribute) { - dependencies = ImmutableList.of(depAttribute); + this(ImmutableList.of(depAttribute)); } /** @@ -1070,7 +1222,38 @@ public final class Attribute implements Comparable<Attribute> { * explicitly specified configurable attribute values. */ public ComputedDefault(String depAttribute1, String depAttribute2) { - dependencies = ImmutableList.of(depAttribute1, depAttribute2); + this(ImmutableList.of(depAttribute1, depAttribute2)); + } + + /** + * Creates a computed default that can read all non-configurable attributes and some explicitly + * specified configurable attribute values. + * + * <p>This constructor should not be used by native {@link ComputedDefault} functions. The limit + * of at-most-two depended-on configurable attributes is intended, to limit the exponential + * growth of possible values. {@link SkylarkComputedDefault} uses this, but is limited by {@link + * FixedComputationLimiter#COMPUTED_DEFAULT_MAX_COMBINATIONS}. + */ + protected ComputedDefault(ImmutableList<String> dependencies) { + // Order is important for #createDependencyAssignmentTuple. + this.dependencies = Ordering.natural().immutableSortedCopy(dependencies); + } + + <T> Iterable<T> getPossibleValues(Type<T> type, Rule rule) { + final ComputedDefault owner = ComputedDefault.this; + ComputationStrategy<RuntimeException> strategy = + new ComputationStrategy<RuntimeException>() { + @Override + public Object compute(AttributeMap map) { + return owner.getDefault(map); + } + }; + // Note that this uses ArrayList instead of something like ImmutableList because some + // values may be null. + return new ArrayList<>( + strategy + .computeValuesForAllCombinations(dependencies, type, rule, NULL_COMPUTATION_LIMITER) + .values()); } /** The list of configurable attributes this ComputedDefault declares it may read. */ @@ -1078,10 +1261,211 @@ public final class Attribute implements Comparable<Attribute> { return dependencies; } + /** + * Returns the value this {@link ComputedDefault} evaluates to, given the inputs contained in + * {@code rule}. + */ public abstract Object getDefault(AttributeMap rule); } /** + * A Skylark-defined computed default, which can be precomputed for a specific {@link Rule} by + * calling {@link #computePossibleValues}, which returns a {@link SkylarkComputedDefault} that + * contains a lookup table. + */ + public static final class SkylarkComputedDefaultTemplate { + private final Type<?> type; + private final SkylarkCallbackFunction callback; + private final Location location; + private final ImmutableList<String> dependencies; + + /** + * Creates a new SkylarkComputedDefaultTemplate that allows the computation of attribute values + * via a callback function during loading phase. + * + * @param type The type of the value of this attribute. + * @param dependencies A list of all names of other attributes that are accessed by this + * attribute. + * @param callback A function to compute the actual attribute value. + * @param location The location of the Skylark function. + */ + public SkylarkComputedDefaultTemplate( + Type<?> type, + ImmutableList<String> dependencies, + SkylarkCallbackFunction callback, + Location location) { + this.type = Preconditions.checkNotNull(type); + this.dependencies = Preconditions.checkNotNull(dependencies); + this.callback = Preconditions.checkNotNull(callback); + this.location = Preconditions.checkNotNull(location); + } + + /** + * Returns a {@link SkylarkComputedDefault} containing a lookup table specifying the output of + * this {@link SkylarkComputedDefaultTemplate}'s callback given each possible assignment {@code + * rule} might make to the attributes specified by {@link #dependencies}. + * + * <p>If the rule is missing an attribute specified by {@link #dependencies}, or if there are + * too many possible assignments, or if any evaluation fails, this throws {@link + * CannotPrecomputeDefaultsException}. + * + * <p>May only be called after all non-{@link ComputedDefault} attributes have been set on the + * {@code rule}. + */ + SkylarkComputedDefault computePossibleValues( + Attribute attr, final Rule rule, final EventHandler eventHandler) + throws InterruptedException, CannotPrecomputeDefaultsException { + + final SkylarkComputedDefaultTemplate owner = SkylarkComputedDefaultTemplate.this; + final String msg = + String.format( + "Cannot compute default value of attribute '%s' in rule '%s': ", + attr.getName().replace('$', '_'), rule.getLabel()); + final AtomicReference<EvalException> caughtEvalExceptionIfAny = new AtomicReference<>(); + ComputationStrategy<InterruptedException> strategy = + new ComputationStrategy<InterruptedException>() { + @Override + public Object compute(AttributeMap map) throws InterruptedException { + try { + return owner.computeValue(map); + } catch (EvalException ex) { + caughtEvalExceptionIfAny.compareAndSet(null, ex); + return null; + } + } + }; + + ImmutableList.Builder<Type<?>> dependencyTypesBuilder = ImmutableList.builder(); + Map<List<Object>, Object> lookupTable = new HashMap<>(); + try { + for (String dependency : dependencies) { + Attribute attribute = rule.getRuleClassObject().getAttributeByNameMaybe(dependency); + if (attribute == null) { + throw new AttributeNotFoundException( + String.format("No such attribute %s in rule %s", dependency, rule.getLabel())); + } + dependencyTypesBuilder.add(attribute.getType()); + } + lookupTable.putAll( + strategy.computeValuesForAllCombinations( + dependencies, attr.getType(), rule, FixedComputationLimiter.INSTANCE)); + if (caughtEvalExceptionIfAny.get() != null) { + throw caughtEvalExceptionIfAny.get(); + } + } catch (AttributeNotFoundException + | TooManyConfigurableAttributesException + | EvalException ex) { + String error = msg + ex.getMessage(); + rule.reportError(error, eventHandler); + throw new CannotPrecomputeDefaultsException(error); + } + return new SkylarkComputedDefault(dependencies, dependencyTypesBuilder.build(), lookupTable); + } + + private Object computeValue(AttributeMap rule) throws EvalException, InterruptedException { + Map<String, Object> attrValues = new HashMap<>(); + for (String attrName : rule.getAttributeNames()) { + Attribute attr = rule.getAttributeDefinition(attrName); + if (!attr.hasComputedDefault()) { + Object value = rule.get(attrName, attr.getType()); + if (!EvalUtils.isNullOrNone(value)) { + attrValues.put(attr.getName(), value); + } + } + } + return invokeCallback(attrValues); + } + + private Object invokeCallback(Map<String, Object> attrValues) + throws EvalException, InterruptedException { + ClassObject attrs = + SkylarkClassObjectConstructor.STRUCT.create( + attrValues, "No such regular (non computed) attribute '%s'."); + Object result = callback.call(attrs); + try { + return type.cast((result == Runtime.NONE) ? type.getDefaultValue() : result); + } catch (ClassCastException ex) { + throw new EvalException( + location, + String.format( + "Expected '%s', but got '%s'", type, EvalUtils.getDataTypeName(result, true))); + } + } + + private static class AttributeNotFoundException extends Exception { + private AttributeNotFoundException(String message) { + super(message); + } + } + + static class CannotPrecomputeDefaultsException extends Exception { + private CannotPrecomputeDefaultsException(String message) { + super(message); + } + } + } + + /** + * A class for computed attributes defined in Skylark. + * + * <p>Unlike {@link ComputedDefault}, instances of this class contain a pre-computed table of all + * possible assignments of depended-on attributes and what the Skylark function evaluates to, and + * {@link #getPossibleValues(Type, Rule)} and {@link #getDefault(AttributeMap)} do lookups in that + * table. + */ + static final class SkylarkComputedDefault extends ComputedDefault { + + private final List<Type<?>> dependencyTypes; + private final Map<List<Object>, Object> lookupTable; + + /** + * Creates a new SkylarkComputedDefault containing a lookup table. + * + * @param requiredAttributes A list of all names of other attributes that are accessed by this + * attribute. + * @param dependencyTypes A list of requiredAttributes' types. + * @param lookupTable An exhaustive mapping from requiredAttributes assignments to values this + * computed default evaluates to. + */ + SkylarkComputedDefault( + ImmutableList<String> requiredAttributes, + ImmutableList<Type<?>> dependencyTypes, + Map<List<Object>, Object> lookupTable) { + super(Preconditions.checkNotNull(requiredAttributes)); + this.dependencyTypes = Preconditions.checkNotNull(dependencyTypes); + this.lookupTable = Preconditions.checkNotNull(lookupTable); + } + + List<Type<?>> getDependencyTypes() { + return dependencyTypes; + } + + Map<List<Object>, Object> getLookupTable() { + return lookupTable; + } + + @Override + public Object getDefault(AttributeMap rule) { + List<Object> key = ComputationStrategy.createDependencyAssignmentTuple(dependencies(), rule); + Preconditions.checkState( + lookupTable.containsKey(key), + "Error in rule '%s': precomputed value missing for dependencies: %s", + rule.getLabel(), + Iterables.toString(key)); + return lookupTable.get(key); + } + + @Override + <T> Iterable<T> getPossibleValues(Type<T> type, Rule rule) { + List<T> result = new ArrayList<>(lookupTable.size()); + for (Object obj : lookupTable.values()) { + result.add(type.cast(obj)); + } + return result; + } + } + + /** * Marker interface for late-bound values. Unfortunately, we can't refer to BuildConfiguration * right now, since that is in a separate compilation unit. * @@ -1212,51 +1596,6 @@ public final class Attribute implements Comparable<Attribute> { public abstract List<Label> resolve(Rule rule, AttributeMap attributes, T configuration); } - /** - * A class for late bound attributes defined in Skylark. - */ - public static final class SkylarkLateBound implements LateBoundDefault<Object> { - - private final SkylarkCallbackFunction callback; - - public SkylarkLateBound(SkylarkCallbackFunction callback) { - this.callback = callback; - } - - @Override - public boolean useHostConfiguration() { - return false; - } - - @Override - public ImmutableSet<Class<?>> getRequiredConfigurationFragments() { - return ImmutableSet.of(); - } - - @Override - public Object getDefault() { - return null; - } - - @Override - public Object resolve(Rule rule, AttributeMap attributes, Object o) - throws EvalException, InterruptedException { - Map<String, Object> attrValues = new HashMap<>(); - for (Attribute attr : rule.getAttributes()) { - if (!attr.isLateBound()) { - Object value = attributes.get(attr.getName(), attr.getType()); - if (value != null) { - attrValues.put(attr.getName(), value); - } - } - } - ClassObject attrs = SkylarkClassObjectConstructor.STRUCT.create( - attrValues, - "No such regular (non late-bound) attribute '%s'."); - return callback.call(attrs, o); - } - } - private final String name; private final Type<?> type; @@ -1267,8 +1606,10 @@ public final class Attribute implements Comparable<Attribute> { // 1. defaultValue == null. // 2. defaultValue instanceof ComputedDefault && // type.isValid(defaultValue.getDefault()) - // 3. type.isValid(defaultValue). - // 4. defaultValue instanceof LateBoundDefault && + // 3. defaultValue instanceof SkylarkComputedDefaultTemplate && + // type.isValid(defaultValue.computePossibleValues().getDefault()) + // 4. type.isValid(defaultValue). + // 5. defaultValue instanceof LateBoundDefault && // type.isValid(defaultValue.getDefault(configuration)) // (We assume a hypothetical Type.isValid(Object) predicate.) private final Object defaultValue; @@ -1656,7 +1997,9 @@ public final class Attribute implements Comparable<Attribute> { * @see #getDefaultValue(Rule) */ boolean hasComputedDefault() { - return (defaultValue instanceof ComputedDefault) || (condition != null); + return (defaultValue instanceof ComputedDefault) + || (defaultValue instanceof SkylarkComputedDefaultTemplate) + || (condition != null); } /** diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeValueSource.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeValueSource.java index 30999c283f..7acaa58444 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AttributeValueSource.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeValueSource.java @@ -34,7 +34,8 @@ public enum AttributeValueSource { * Creates a new instance and defines the prefixes for both Skylark and native. * * @param nativePrefix The prefix when converted to a native attribute name. - * @param mustHaveSkylarkPrefix Whether the Skylark name must start with {@link SKYLARK_PREFIX}. + * @param mustHaveSkylarkPrefix Whether the Skylark name must start with {@link + * AttributeValueSource#SKYLARK_PREFIX}. */ AttributeValueSource(String nativePrefix, boolean mustHaveSkylarkPrefix) { this.nativePrefix = nativePrefix; diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java index 751e6cf7f6..cba10a705f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java @@ -14,8 +14,8 @@ package com.google.devtools.build.lib.packages; -import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST; +import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST; import static com.google.devtools.build.lib.syntax.Type.BOOLEAN; @@ -34,6 +34,8 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.events.NullEventHandler; +import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate; +import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate.CannotPrecomputeDefaultsException; import com.google.devtools.build.lib.packages.BuildType.SelectorList; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.packages.RuleFactory.AttributeValuesMap; @@ -1310,13 +1312,15 @@ public final class RuleClass { * attributeValues} or the default attribute values associated with this {@link RuleClass} and * {@code pkgBuilder}. * - * <p>The created {@link Rule} will also be populated with output files. These output files - * will have been collected from the explicitly provided values of type {@link BuildType#OUTPUT} - * and {@link BuildType#OUTPUT_LIST} as well as from the implicit outputs determined by this - * {@link RuleClass} and the values in {@code attributeValues}. + * <p>The created {@link Rule} will also be populated with output files. These output files will + * have been collected from the explicitly provided values of type {@link BuildType#OUTPUT} and + * {@link BuildType#OUTPUT_LIST} as well as from the implicit outputs determined by this {@link + * RuleClass} and the values in {@code attributeValues}. * * <p>This performs several validity checks. Invalid output file labels result in a thrown {@link - * LabelSyntaxException}. All other errors are reported on {@code eventHandler}. + * LabelSyntaxException}. Computed default attributes that fail during precomputation result in a + * {@link CannotPrecomputeDefaultsException}. All other errors are reported on {@code + * eventHandler}. */ Rule createRule( Package.Builder pkgBuilder, @@ -1326,7 +1330,7 @@ public final class RuleClass { @Nullable FuncallExpression ast, Location location, AttributeContainer attributeContainer) - throws LabelSyntaxException, InterruptedException { + throws LabelSyntaxException, InterruptedException, CannotPrecomputeDefaultsException { Rule rule = pkgBuilder.createRule(ruleLabel, this, location, attributeContainer); populateRuleAttributeValues(rule, pkgBuilder, attributeValues, eventHandler); checkAspectAllowedValues(rule, eventHandler); @@ -1354,7 +1358,7 @@ public final class RuleClass { Location location, AttributeContainer attributeContainer, ImplicitOutputsFunction implicitOutputsFunction) - throws LabelSyntaxException, InterruptedException { + throws LabelSyntaxException, InterruptedException, CannotPrecomputeDefaultsException { Rule rule = pkgBuilder.createRule( ruleLabel, this, @@ -1377,7 +1381,8 @@ public final class RuleClass { Rule rule, Package.Builder pkgBuilder, AttributeValuesMap attributeValues, - EventHandler eventHandler) { + EventHandler eventHandler) + throws InterruptedException, CannotPrecomputeDefaultsException { BitSet definedAttrIndices = populateDefinedRuleAttributeValues(rule, attributeValues, eventHandler); populateDefaultRuleAttributeValues(rule, pkgBuilder, definedAttrIndices, eventHandler); @@ -1457,14 +1462,15 @@ public final class RuleClass { /** * Populates the attributes table of the new {@link Rule} with default values provided by this - * {@link RuleClass} and the {@code pkgBuilder}. This will only provide values for attributes - * that haven't already been populated, using {@code definedAttrIndices} to determine whether an + * {@link RuleClass} and the {@code pkgBuilder}. This will only provide values for attributes that + * haven't already been populated, using {@code definedAttrIndices} to determine whether an * attribute was populated. * * <p>Errors are reported on {@code eventHandler}. */ private void populateDefaultRuleAttributeValues( - Rule rule, Package.Builder pkgBuilder, BitSet definedAttrIndices, EventHandler eventHandler) { + Rule rule, Package.Builder pkgBuilder, BitSet definedAttrIndices, EventHandler eventHandler) + throws InterruptedException, CannotPrecomputeDefaultsException { // Set defaults; ensure that every mandatory attribute has a value. Use the default if none // is specified. List<Attribute> attrsWithComputedDefaults = new ArrayList<>(); @@ -1501,13 +1507,29 @@ public final class RuleClass { // Set computed default attribute values now that all other (i.e. non-computed) default values // have been set. for (Attribute attr : attrsWithComputedDefaults) { - // If Attribute#hasComputedDefault is true, Attribute#getDefaultValue returns the computed - // default function object. Note that we don't evaluate the computed default function here - // because it may depend on other attribute values that are configurable (i.e. they came - // from select({..}) expressions in the build language, and they require configuration data - // from the analysis phase to be resolved). We're setting the attribute value to a - // reference to the computed default function. - rule.setAttributeValue(attr, attr.getDefaultValue(rule), /*explicit=*/ false); + // If Attribute#hasComputedDefault was true above, Attribute#getDefaultValue returns the + // computed default function object or a Skylark computed default template. Note that we + // cannot determine the exact value of the computed default function here because it may + // depend on other attribute values that are configurable (i.e. they came from select({..}) + // expressions in the build language, and they require configuration data from the analysis + // phase to be resolved). Instead, we're setting the attribute value to a reference to the + // computed default function, or if #getDefaultValue is a Skylark computed default + // template, setting the attribute value to a reference to the SkylarkComputedDefault + // returned from SkylarkComputedDefaultTemplate#computePossibleValues. + // + // SkylarkComputedDefaultTemplate#computePossibleValues pre-computes all possible values the + // function may evaluate to, and records them in a lookup table. By calling it here, with an + // EventHandler, any errors that might occur during the function's evaluation can + // be discovered and propagated here. + Object valueToSet; + Object defaultValue = attr.getDefaultValue(rule); + if (defaultValue instanceof SkylarkComputedDefaultTemplate) { + SkylarkComputedDefaultTemplate template = (SkylarkComputedDefaultTemplate) defaultValue; + valueToSet = template.computePossibleValues(attr, rule, eventHandler); + } else { + valueToSet = defaultValue; + } + rule.setAttributeValue(attr, valueToSet, /*explicit=*/ false); } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java index 3b6cf4c904..4795c7b1ac 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RuleFactory.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate.CannotPrecomputeDefaultsException; import com.google.devtools.build.lib.packages.Package.NameConflictException; import com.google.devtools.build.lib.packages.PackageFactory.PackageContext; import com.google.devtools.build.lib.syntax.BaseFunction; @@ -28,10 +29,8 @@ import com.google.devtools.build.lib.syntax.FuncallExpression; import com.google.devtools.build.lib.syntax.UserDefinedFunction; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.Preconditions; - import java.util.Map; import java.util.Set; - import javax.annotation.Nullable; /** @@ -135,7 +134,7 @@ public class RuleFactory { ast, generator.location, attributeContainer); - } catch (LabelSyntaxException e) { + } catch (LabelSyntaxException | CannotPrecomputeDefaultsException e) { throw new RuleFactory.InvalidRuleException(ruleClass + " " + e.getMessage()); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java index 3e2665dc97..49740a5fe4 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkAttr.java @@ -22,7 +22,7 @@ import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; -import com.google.devtools.build.lib.packages.Attribute.SkylarkLateBound; +import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate; import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.SkylarkAspect; import com.google.devtools.build.lib.skylarkinterface.Param; @@ -46,11 +46,9 @@ import com.google.devtools.build.lib.syntax.UserDefinedFunction; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.Preconditions; - import java.util.ArrayList; import java.util.List; import java.util.Map; - import javax.annotation.Nullable; /** @@ -163,10 +161,20 @@ public final class SkylarkAttr { Object defaultValue = arguments.get(DEFAULT_ARG); if (!EvalUtils.isNullOrNone(defaultValue)) { if (defaultValue instanceof UserDefinedFunction) { - // Late bound attribute. Non label type attributes already caused a type check error. + // Computed attribute. Non label type attributes already caused a type check error. + UserDefinedFunction callback = (UserDefinedFunction) defaultValue; + + // SkylarkComputedDefaultTemplate needs to know the names of all attributes that it depends + // on. However, this method does not know anything about other attributes. + // We solve this problem by asking the UserDefinedFunction for the parameter names used in + // the function definition, which must be the names of attributes used by the callback. + ImmutableList<String> dependencies = callback.getSignature().getSignature().getNames(); builder.value( - new SkylarkLateBound( - new SkylarkCallbackFunction((UserDefinedFunction) defaultValue, ast, env))); + new SkylarkComputedDefaultTemplate( + type, + dependencies, + new SkylarkCallbackFunction(callback, ast, env), + ast.getLocation())); } else { builder.defaultValue(defaultValue); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java index 04ff0ac2e7..60b293bb1a 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java @@ -26,7 +26,16 @@ import com.google.devtools.build.lib.syntax.compiler.LoopLabels; import com.google.devtools.build.lib.syntax.compiler.ReflectionUtils; import com.google.devtools.build.lib.syntax.compiler.VariableScope; import com.google.devtools.build.lib.vfs.PathFragment; - +import java.io.File; +import java.io.IOException; +import java.io.PrintWriter; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; import net.bytebuddy.ByteBuddy; import net.bytebuddy.asm.ClassVisitorWrapper; import net.bytebuddy.description.method.MethodDescription; @@ -42,7 +51,6 @@ import net.bytebuddy.implementation.MethodDelegation; import net.bytebuddy.implementation.bytecode.ByteCodeAppender; import net.bytebuddy.implementation.bytecode.member.MethodReturn; import net.bytebuddy.matcher.ElementMatchers; - import org.objectweb.asm.ClassVisitor; import org.objectweb.asm.ClassWriter; import org.objectweb.asm.Label; @@ -50,17 +58,6 @@ import org.objectweb.asm.Opcodes; import org.objectweb.asm.util.Textifier; import org.objectweb.asm.util.TraceClassVisitor; -import java.io.File; -import java.io.IOException; -import java.io.PrintWriter; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; -import java.nio.file.Files; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - /** * The actual function registered in the environment. This function is defined in the * parsed code using {@link FunctionDefStatement}. diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index 9f4514cd94..1426ea58fc 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -858,12 +858,11 @@ public abstract class BuildViewTestCase extends FoundationTestCase { getTarget(target); fail( String.format( - "checkLoadingPhaseError(): expected an error with '%s' when loading target '%s'.", - expectedErrorMessage, - target)); - } catch (Exception e) { - assertContainsEvent(expectedErrorMessage); + "checkLoadingPhaseError(): expected an exception with '%s' when loading target '%s'.", + expectedErrorMessage, target)); + } catch (Exception expected) { } + assertContainsEvent(expectedErrorMessage); } /** diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java index 24965818f0..03fbd31bda 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.events.EventCollector; import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.events.Location.LineAndColumn; +import com.google.devtools.build.lib.packages.Attribute.SkylarkComputedDefaultTemplate.CannotPrecomputeDefaultsException; import com.google.devtools.build.lib.packages.Attribute.ValidityPredicate; import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -59,12 +60,6 @@ import com.google.devtools.build.lib.syntax.BaseFunction; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.vfs.Path; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -75,8 +70,11 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; - import javax.annotation.Nullable; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; /** * Tests for {@link RuleClass}. @@ -772,8 +770,9 @@ public class RuleClassTest extends PackageLoadingTestCase { attributes.get("my-sorted-stringlist-attr", Type.STRING_LIST)); } - private Rule createRule(RuleClass ruleClass, String name, Map<String, Object> attributeValues, - Location location) throws LabelSyntaxException, InterruptedException { + private Rule createRule( + RuleClass ruleClass, String name, Map<String, Object> attributeValues, Location location) + throws LabelSyntaxException, InterruptedException, CannotPrecomputeDefaultsException { Package.Builder pkgBuilder = createDummyPackageBuilder(); Label ruleLabel; try { |