Refactoring: Types report what class of labels they contain.
Currently label-type attributes are detected in many places across the
codebase by simply reference-comparing against each of the label types.
This CL aims to generalize most of these cases, moving the encoding of
this logic into a single place (Type/BuildType itself). Not all of these
cases can be made general without further refactoring, and some perhaps
shouldn't be - serialization and Skylark rule context, for example, need
to do exotic things based on the type. But most sites can avoid having to
enumerate all the types they work with explicitly.
This causes LABEL_DICT_UNARY to start being treated like the other label
types, which means that CcToolchainSuiteRule and JavaRuntimeSuiteRule
need to include a set of allowed file types (none, in their case).
Skylark will continue treating it as a dictionary from String to Label
in its rule context, however, to avoid visible behavior changes.
--
PiperOrigin-RevId: 147471542
MOS_MIGRATED_REVID=147471542
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index 937cf74..656ca25 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -81,6 +81,7 @@
import com.google.devtools.build.lib.shell.ShellUtils;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.lib.util.Preconditions;
@@ -1083,8 +1084,7 @@
throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging()
+ " attribute " + attributeName + " is not defined");
}
- if (!(attributeDefinition.getType() == BuildType.LABEL
- || attributeDefinition.getType() == BuildType.LABEL_LIST)) {
+ if (attributeDefinition.getType().getLabelClass() != LabelClass.DEPENDENCY) {
throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
+ " is not a label type attribute");
}
@@ -1125,8 +1125,7 @@
throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging()
+ " attribute " + attributeName + " is not defined");
}
- if (!(attributeDefinition.getType() == BuildType.LABEL
- || attributeDefinition.getType() == BuildType.LABEL_LIST)) {
+ if (attributeDefinition.getType().getLabelClass() != LabelClass.DEPENDENCY) {
throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName
+ " is not a label type attribute");
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
index 86d736d..33f7d34 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java
@@ -36,6 +36,8 @@
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
+import com.google.devtools.build.lib.syntax.Type.LabelVisitor;
import com.google.devtools.build.lib.util.Preconditions;
import java.util.Collection;
@@ -739,7 +741,7 @@
AttributeMap attributes = ruleContext.attributes();
for (String attr : attributes.getAttributeNames()) {
Attribute attrDef = attributes.getAttributeDefinition(attr);
- if ((attrDef.getType() != BuildType.LABEL && attrDef.getType() != BuildType.LABEL_LIST)
+ if (attrDef.getType().getLabelClass() != LabelClass.DEPENDENCY
|| attrDef.skipConstraintsOverride()) {
continue;
}
@@ -819,21 +821,22 @@
* Adds all label values from the given select to the given set. Automatically handles different
* value types (e.g. labels vs. label lists).
*/
- private static void addSelectValuesToSet(BuildType.Selector<?> select, Set<Label> set) {
+ private static void addSelectValuesToSet(BuildType.Selector<?> select, final Set<Label> set) {
Type<?> type = select.getOriginalType();
- if (type == BuildType.LABEL || type == BuildType.NODEP_LABEL) {
- set.addAll(((BuildType.Selector<Label>) select).getEntries().values());
- } else if (type == BuildType.LABEL_LIST || type == BuildType.NODEP_LABEL_LIST) {
- for (List<Label> labels : ((BuildType.Selector<List<Label>>) select).getEntries().values()) {
- set.addAll(labels);
+ LabelVisitor visitor = new LabelVisitor() {
+ @Override
+ public void visit(Label label) {
+ set.add(label);
}
- } else if (type == BuildType.LABEL_DICT_UNARY) {
- for (Map<String, Label> mapEntry :
- ((BuildType.Selector<Map<String, Label>>) select).getEntries().values()) {
- set.addAll(mapEntry.values());
+ };
+ for (Object value : select.getEntries().values()) {
+ try {
+ type.visitLabels(visitor, value);
+ } catch (InterruptedException ex) {
+ // Because the LabelVisitor does not throw InterruptedException, it should not be thrown
+ // by visitLabels here.
+ throw new AssertionError(ex);
}
- } else {
- throw new IllegalStateException("Expected a label-based type for this select");
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index 338d4bf..a305e51 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
@@ -25,11 +25,12 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.ConfigurationFragmentPolicy.MissingFragmentPolicy;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
+import com.google.devtools.build.lib.syntax.Type.LabelVisitor;
import com.google.devtools.build.lib.util.Preconditions;
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
-import java.util.List;
import java.util.Map;
import javax.annotation.Nullable;
@@ -190,32 +191,36 @@
* Collects all attribute labels from the specified aspectDefinition.
*/
public static void addAllAttributesOfAspect(
- Rule from,
- Multimap<Attribute, Label> labelBuilder,
+ final Rule from,
+ final Multimap<Attribute, Label> labelBuilder,
Aspect aspect,
DependencyFilter dependencyFilter) {
ImmutableMap<String, Attribute> attributes = aspect.getDefinition().getAttributes();
- for (Attribute aspectAttribute : attributes.values()) {
+ for (final Attribute aspectAttribute : attributes.values()) {
if (!dependencyFilter.apply(aspect, aspectAttribute)) {
continue;
}
- if (aspectAttribute.getType() == BuildType.LABEL) {
- Label label = maybeGetRepositoryRelativeLabel(
- from, BuildType.LABEL.cast(aspectAttribute.getDefaultValue(from)));
- if (label != null) {
- labelBuilder.put(aspectAttribute, label);
- }
- } else if (aspectAttribute.getType() == BuildType.LABEL_LIST) {
- List<Label> defaultLabels = BuildType.LABEL_LIST.cast(
+ Type type = aspectAttribute.getType();
+ if (type.getLabelClass() != LabelClass.DEPENDENCY) {
+ continue;
+ }
+ try {
+ type.visitLabels(
+ new LabelVisitor() {
+ @Override
+ public void visit(Label label) {
+ Label repositoryRelative = maybeGetRepositoryRelativeLabel(from, label);
+ if (repositoryRelative == null) {
+ return;
+ }
+ labelBuilder.put(aspectAttribute, repositoryRelative);
+ }
+ },
aspectAttribute.getDefaultValue(from));
- if (defaultLabels != null) {
- for (Label defaultLabel : defaultLabels) {
- Label label = maybeGetRepositoryRelativeLabel(from, defaultLabel);
- if (label != null) {
- labelBuilder.put(aspectAttribute, label);
- }
- }
- }
+ } catch (InterruptedException ex) {
+ // Because the LabelVisitor does not throw InterruptedException, it should not be thrown
+ // by visitLabels here.
+ throw new AssertionError(ex);
}
}
}
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 2288a76..f5e9b95 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
@@ -36,6 +36,7 @@
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;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Preconditions;
@@ -507,7 +508,7 @@
* Makes the built attribute producing a single artifact.
*/
public Builder<TYPE> singleArtifact() {
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"attribute '%s' must be a label-valued type", name);
return setPropertyFlag(PropertyFlag.SINGLE_ARTIFACT, "single_artifact");
}
@@ -517,7 +518,7 @@
* This flag is introduced to handle plugins, do not use it in other cases.
*/
public Builder<TYPE> silentRuleClassFilter() {
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
return setPropertyFlag(PropertyFlag.SILENT_RULECLASS_FILTER, "silent_ruleclass_filter");
}
@@ -526,7 +527,7 @@
* Skip analysis time filetype check. Don't use it if avoidable.
*/
public Builder<TYPE> skipAnalysisTimeFileTypeCheck() {
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
return setPropertyFlag(PropertyFlag.SKIP_ANALYSIS_TIME_FILETYPE_CHECK,
"skip_analysis_time_filetype_check");
@@ -801,7 +802,7 @@
* other words, it works for 'deps' attributes, but not 'srcs' attributes.
*/
public Builder<TYPE> allowedRuleClasses(Predicate<RuleClass> allowedRuleClasses) {
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING);
allowedRuleClassesForLabels = allowedRuleClasses;
@@ -831,7 +832,7 @@
* other words, it works for 'deps' attributes, but not 'srcs' attributes.
*/
public Builder<TYPE> allowedFileTypes(FileTypeSet allowedFileTypes) {
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING);
allowedFileTypesForLabels = Preconditions.checkNotNull(allowedFileTypes);
@@ -886,7 +887,7 @@
* other words, it works for 'deps' attributes, but not 'srcs' attributes.
*/
public Builder<TYPE> allowedRuleClassesWithWarning(Predicate<RuleClass> allowedRuleClasses) {
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
propertyFlags.add(PropertyFlag.STRICT_LABEL_CHECKING);
allowedRuleClassesForLabelsWarning = allowedRuleClasses;
@@ -914,7 +915,7 @@
*/
public final Builder<TYPE> mandatoryNativeProvidersList(
Iterable<? extends Iterable<Class<? extends TransitiveInfoProvider>>> providersList) {
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
ImmutableList.Builder<ImmutableList<Class<? extends TransitiveInfoProvider>>> listBuilder
= ImmutableList.builder();
@@ -941,7 +942,7 @@
*/
public Builder<TYPE> mandatoryProvidersList(
Iterable<? extends Iterable<SkylarkProviderIdentifier>> providersList){
- Preconditions.checkState((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST),
+ Preconditions.checkState(type.getLabelClass() == LabelClass.DEPENDENCY,
"must be a label-valued type");
ImmutableList.Builder<ImmutableSet<SkylarkProviderIdentifier>> listBuilder
= ImmutableList.builder();
@@ -1086,13 +1087,13 @@
// do not modify this.allowedFileTypesForLabels, instead create a copy.
FileTypeSet allowedFileTypesForLabels = this.allowedFileTypesForLabels;
- if ((type == BuildType.LABEL) || (type == BuildType.LABEL_LIST)) {
+ 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 == BuildType.OUTPUT) || (type == BuildType.OUTPUT_LIST)) {
+ } 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;
@@ -1769,8 +1770,8 @@
Preconditions.checkNotNull(configTransition);
Preconditions.checkArgument(
(configTransition == ConfigurationTransition.NONE && configurator == null)
- || type == BuildType.LABEL || type == BuildType.LABEL_LIST
- || type == BuildType.NODEP_LABEL || type == BuildType.NODEP_LABEL_LIST,
+ || type.getLabelClass() == LabelClass.DEPENDENCY
+ || type.getLabelClass() == LabelClass.NONDEP_REFERENCE,
"Configuration transitions can only be specified for label or label list attributes");
Preconditions.checkArgument(
isLateBound(name) == (defaultValue instanceof LateBoundDefault),
@@ -1975,8 +1976,8 @@
* Returns true if this attribute's value can be influenced by the build configuration.
*/
public boolean isConfigurable() {
- return !(type == BuildType.OUTPUT // Excluded because of Rule#populateExplicitOutputFiles.
- || type == BuildType.OUTPUT_LIST
+ // Output types are excluded because of Rule#populateExplicitOutputFiles.
+ return !(type.getLabelClass() == LabelClass.OUTPUT
|| getPropertyFlag(PropertyFlag.NONCONFIGURABLE));
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
index ae3b121..051b7e0 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/BuildType.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
import com.google.devtools.build.lib.syntax.Type.DictType;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
import com.google.devtools.build.lib.syntax.Type.ListType;
import java.util.Collections;
import java.util.LinkedHashMap;
@@ -47,7 +48,7 @@
* attributes that it's worth treating them specially (and providing support
* for resolution of relative-labels in the <code>convert()</code> method).
*/
- public static final Type<Label> LABEL = new LabelType();
+ public static final Type<Label> LABEL = new LabelType(LabelClass.DEPENDENCY);
/**
* The type of a dictionary of {@linkplain #LABEL labels}.
*/
@@ -62,7 +63,7 @@
* certain rules want to verify the type of a target referenced by one of their attributes, but
* if there was a dependency edge there, it would be a circular dependency.
*/
- public static final Type<Label> NODEP_LABEL = new LabelType();
+ public static final Type<Label> NODEP_LABEL = new LabelType(LabelClass.NONDEP_REFERENCE);
/**
* The type of a list of {@linkplain #NODEP_LABEL labels} that do not cause
* dependencies.
@@ -136,8 +137,7 @@
* Returns whether the specified type is a label type or not.
*/
public static boolean isLabelType(Type<?> type) {
- return type == LABEL || type == LABEL_LIST || type == LABEL_DICT_UNARY
- || type == NODEP_LABEL || type == NODEP_LABEL_LIST || type == FILESET_ENTRY_LIST;
+ return type.getLabelClass() != LabelClass.NONE;
}
/**
@@ -182,6 +182,11 @@
}
@Override
+ public LabelClass getLabelClass() {
+ return LabelClass.FILESET_ENTRY;
+ }
+
+ @Override
public FilesetEntry getDefaultValue() {
return null;
}
@@ -195,6 +200,12 @@
}
private static class LabelType extends Type<Label> {
+ private final LabelClass labelClass;
+
+ LabelType(LabelClass labelClass) {
+ this.labelClass = labelClass;
+ }
+
@Override
public Label cast(Object value) {
return (Label) value;
@@ -216,6 +227,11 @@
}
@Override
+ public LabelClass getLabelClass() {
+ return labelClass;
+ }
+
+ @Override
public Label convert(Object x, Object what, Object context)
throws ConversionException {
if (x instanceof Label) {
@@ -327,6 +343,11 @@
}
@Override
+ public LabelClass getLabelClass() {
+ return LabelClass.OUTPUT;
+ }
+
+ @Override
public String toString() {
return "output";
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java
index a5d41ee..88fe6d4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/DependencyFilter.java
@@ -15,6 +15,7 @@
import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
import com.google.devtools.build.lib.packages.DependencyFilter.AttributeInfoProvider;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
import com.google.devtools.build.lib.util.BinaryPredicate;
/**
@@ -38,8 +39,8 @@
new DependencyFilter() {
@Override
public boolean apply(AttributeInfoProvider infoProvider, Attribute attribute) {
- // isHostConfiguration() is only defined for labels and label lists.
- if (attribute.getType() != BuildType.LABEL && attribute.getType() != BuildType.LABEL_LIST) {
+ // getConfigurationTransition() is only defined for labels which introduce a dependency.
+ if (attribute.getType().getLabelClass() != LabelClass.DEPENDENCY) {
return true;
}
@@ -62,8 +63,7 @@
new DependencyFilter() {
@Override
public boolean apply(AttributeInfoProvider infoProvider, Attribute attribute) {
- return attribute.getType() != BuildType.NODEP_LABEL
- && attribute.getType() != BuildType.NODEP_LABEL_LIST;
+ return attribute.getType().getLabelClass() != LabelClass.NONDEP_REFERENCE;
}
};
/**
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 abec031..08113b6 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
@@ -46,6 +46,7 @@
import com.google.devtools.build.lib.syntax.SkylarkType;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
import com.google.devtools.build.lib.syntax.UserDefinedFunction;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.FileTypeSet;
@@ -235,7 +236,7 @@
Object fileTypesObj = arguments.get(ALLOW_SINGLE_FILE_ARG);
setAllowedFileTypes(ALLOW_SINGLE_FILE_ARG, fileTypesObj, ast, builder);
builder.setPropertyFlag("SINGLE_ARTIFACT");
- } else if (type.equals(BuildType.LABEL) || type.equals(BuildType.LABEL_LIST)) {
+ } else if (type.getLabelClass() == LabelClass.DEPENDENCY) {
builder.allowedFileTypes(FileTypeSet.NO_FILE);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
index 7e7b69c..8dfc157 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
@@ -64,6 +64,7 @@
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkType;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
@@ -206,7 +207,7 @@
for (Attribute a : attributes) {
String attrName = a.getName();
Type<?> type = a.getType();
- if (type != BuildType.OUTPUT && type != BuildType.OUTPUT_LIST) {
+ if (type.getLabelClass() != LabelClass.OUTPUT) {
continue;
}
ImmutableList.Builder<Artifact> artifactsBuilder = ImmutableList.builder();
@@ -290,7 +291,10 @@
for (Attribute a : attributes) {
Type<?> type = a.getType();
Object val = attributeValueExtractor.apply(a);
- if (type != BuildType.LABEL && type != BuildType.LABEL_LIST) {
+ // TODO(mstaib): Remove the LABEL_DICT_UNARY special case of this conditional
+ // LABEL_DICT_UNARY was previously not treated as a dependency-bearing type, and was put into
+ // Skylark as a Map<String, Label>; this special case preserves that behavior temporarily.
+ if (type.getLabelClass() != LabelClass.DEPENDENCY || type == BuildType.LABEL_DICT_UNARY) {
attrBuilder.put(a.getPublicName(), val == null ? Runtime.NONE
// Attribute values should be type safe
: SkylarkType.convertToSkylark(val, null));
@@ -336,10 +340,26 @@
prereq = Runtime.NONE;
}
attrBuilder.put(skyname, prereq);
- } else {
- // Type.LABEL_LIST
+ } else if (type == BuildType.LABEL_LIST
+ || (type == BuildType.LABEL && a.hasSplitConfigurationTransition())) {
List<?> allPrereq = ruleContext.getPrerequisites(a.getName(), Mode.DONT_CHECK);
attrBuilder.put(skyname, SkylarkList.createImmutable(allPrereq));
+ } else if (type == BuildType.LABEL_DICT_UNARY) {
+ Map<Label, TransitiveInfoCollection> prereqsByLabel = new LinkedHashMap<>();
+ for (TransitiveInfoCollection target
+ : ruleContext.getPrerequisites(a.getName(), Mode.DONT_CHECK)) {
+ prereqsByLabel.put(target.getLabel(), target);
+ }
+ ImmutableMap.Builder<String, TransitiveInfoCollection> attrValue =
+ new ImmutableMap.Builder<>();
+ for (Map.Entry<String, Label> entry : ((Map<String, Label>) val).entrySet()) {
+ attrValue.put(entry.getKey(), prereqsByLabel.get(entry.getValue()));
+ }
+ attrBuilder.put(skyname, attrValue.build());
+ } else {
+ throw new IllegalArgumentException(
+ "Can't transform attribute " + a.getName() + " of type " + type
+ + " to a Skylark object");
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java
index a08e6e0..55244b4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainSuiteRule.java
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.util.FileTypeSet;
/**
* Definition of the {@code cc_toolchain_suite} rule.
@@ -34,6 +35,7 @@
.setUndocumented()
.add(attr("toolchains", BuildType.LABEL_DICT_UNARY)
.mandatory()
+ .allowedFileTypes(FileTypeSet.NO_FILE)
.nonconfigurable("Used during configuration creation"))
.add(attr("proto", Type.STRING)
.nonconfigurable("Used during configuration creation"))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java
index 8b28e91..22de4df 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaRuntimeSuiteRule.java
@@ -35,7 +35,7 @@
/* <!-- #BLAZE_RULE(java_runtime_suite).ATTRIBUTE(runtimes) -->
A map from each supported architecture to the corresponding <code>java_runtime</code>.
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
- .add(attr("runtimes", BuildType.LABEL_DICT_UNARY))
+ .add(attr("runtimes", BuildType.LABEL_DICT_UNARY).allowedFileTypes(FileTypeSet.NO_FILE))
/* <!-- #BLAZE_RULE(java_runtime_suite).ATTRIBUTE(default) -->
The default <code>java_runtime</code>, used if
<a href="${link java_runtime_suite.runtimes}"><code>runtimes</code></a>
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Type.java b/src/main/java/com/google/devtools/build/lib/syntax/Type.java
index dbbfc79..387794b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Type.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Type.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.syntax;
import com.google.common.base.Joiner;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -139,6 +140,31 @@
*/
public abstract void visitLabels(LabelVisitor visitor, Object value) throws InterruptedException;
+ /** Classifications of labels by their usage. */
+ public enum LabelClass {
+ /** Used for types which are not labels. */
+ NONE,
+ /** Used for types which use labels to declare a dependency. */
+ DEPENDENCY,
+ /**
+ * Used for types which use labels to reference another target but do not declare a dependency,
+ * in cases where doing so would cause a dependency cycle.
+ */
+ NONDEP_REFERENCE,
+ /** Used for types which use labels to declare an output path. */
+ OUTPUT,
+ /**
+ * Used for types which contain Fileset entries, which contain labels but do not produce
+ * normal dependencies.
+ */
+ FILESET_ENTRY
+ }
+
+ /** Returns the class of labels contained by this type, if any. */
+ public LabelClass getLabelClass() {
+ return LabelClass.NONE;
+ }
+
/**
* Implementation of concatenation for this type (e.g. "val1 + val2"). Returns null to
* indicate concatenation isn't supported.
@@ -423,6 +449,8 @@
private final Map<KeyT, ValueT> empty = ImmutableMap.of();
+ private final LabelClass labelClass;
+
@Override
public void visitLabels(LabelVisitor visitor, Object value) throws InterruptedException {
for (Entry<KeyT, ValueT> entry : cast(value).entrySet()) {
@@ -433,12 +461,24 @@
public static <KEY, VALUE> DictType<KEY, VALUE> create(
Type<KEY> keyType, Type<VALUE> valueType) {
- return new DictType<>(keyType, valueType);
+ LabelClass keyLabelClass = keyType.getLabelClass();
+ LabelClass valueLabelClass = valueType.getLabelClass();
+ Preconditions.checkArgument(
+ keyLabelClass == LabelClass.NONE
+ || valueLabelClass == LabelClass.NONE
+ || keyLabelClass == valueLabelClass,
+ "A DictType's keys and values must be the same class of label if both contain labels, "
+ + "but the key type " + keyType + " contains " + keyLabelClass + " labels, while "
+ + "the value type " + valueType + " contains " + valueLabelClass + " labels.");
+ LabelClass labelClass = (keyLabelClass != LabelClass.NONE) ? keyLabelClass : valueLabelClass;
+
+ return new DictType<>(keyType, valueType, labelClass);
}
- private DictType(Type<KeyT> keyType, Type<ValueT> valueType) {
+ private DictType(Type<KeyT> keyType, Type<ValueT> valueType, LabelClass labelClass) {
this.keyType = keyType;
this.valueType = valueType;
+ this.labelClass = labelClass;
}
public Type<KeyT> getKeyType() {
@@ -449,6 +489,11 @@
return valueType;
}
+ @Override
+ public LabelClass getLabelClass() {
+ return labelClass;
+ }
+
@SuppressWarnings("unchecked")
@Override
public Map<KeyT, ValueT> cast(Object value) {
@@ -511,6 +556,11 @@
}
@Override
+ public LabelClass getLabelClass() {
+ return elemType.getLabelClass();
+ }
+
+ @Override
public List<ElemT> getDefaultValue() {
return empty;
}
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index 078d6af..578db1b 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -1383,4 +1383,25 @@
"ruleContext.coverage_instrumented(ruleContext.attr.deps[0])");
assertThat((Boolean) result).isTrue();
}
+
+ @Test
+ public void testStringKeyedLabelDictAttributeInSkylarkRuleContext() throws Exception {
+ scratch.file("jvm/BUILD",
+ "java_runtime(name='runtime', srcs=[], java_home='')",
+ "java_runtime_suite(",
+ " name = 'suite',",
+ " runtimes = {'x86': ':runtime'},",
+ " default = ':runtime',",
+ ")");
+
+ invalidatePackages();
+ SkylarkRuleContext ruleContext = createRuleContext("//jvm:suite");
+ assertNoEvents();
+ String keyString =
+ (String) evalRuleContextCode(ruleContext, "ruleContext.attr.runtimes.keys()[0]");
+ assertThat(keyString).isEqualTo("x86");
+ Label valueLabel =
+ (Label) evalRuleContextCode(ruleContext, "ruleContext.attr.runtimes.values()[0]");
+ assertThat(valueLabel).isEqualTo(Label.parseAbsolute("//jvm:runtime"));
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java b/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java
index 7db103e..e0e6b77 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/BuildRuleWithDefaultsBuilder.java
@@ -20,6 +20,8 @@
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.syntax.Type;
+import com.google.devtools.build.lib.syntax.Type.LabelClass;
+import com.google.devtools.build.lib.syntax.Type.ListType;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Preconditions;
@@ -57,8 +59,8 @@
*/
private String getDummyFileLabel(String rulePkg, String filePkg, String extension,
Type<?> attrType) {
- boolean isInput = (attrType == BuildType.LABEL || attrType == BuildType.LABEL_LIST);
- String fileName = (isInput ? "dummy_input" : "dummy_output") + extension;
+ boolean isOutput = attrType.getLabelClass() == LabelClass.OUTPUT;
+ String fileName = (isOutput ? "dummy_output" : "dummy_input") + extension;
generateFiles.add(filePkg + "/" + fileName);
if (rulePkg.equals(filePkg)) {
return ":" + fileName;
@@ -122,7 +124,7 @@
}
}
if (label != null) {
- if (attrType == BuildType.LABEL_LIST || attrType == BuildType.OUTPUT_LIST) {
+ if (attrType instanceof ListType<?>) {
addMultiValueAttributes(attribute.getName(), label);
} else {
setSingleValueAttribute(attribute.getName(), label);
@@ -175,17 +177,10 @@
public BuildRuleWithDefaultsBuilder populateAttributes(String rulePkg, boolean heuristics) {
for (Attribute attribute : ruleClass.getAttributes()) {
if (attribute.isMandatory()) {
- if (attribute.getType() == BuildType.LABEL_LIST
- || attribute.getType() == BuildType.OUTPUT_LIST) {
- if (attribute.isNonEmpty()) {
- populateLabelAttribute(rulePkg, attribute);
- } else {
- // TODO(bazel-team): actually here an empty list would be fine, but BuildRuleBuilder
- // doesn't support that, and it makes little sense anyway
- populateLabelAttribute(rulePkg, attribute);
- }
- } else if (attribute.getType() == BuildType.LABEL
- || attribute.getType() == BuildType.OUTPUT) {
+ if (BuildType.isLabelType(attribute.getType())) {
+ // TODO(bazel-team): actually an empty list would be fine in the case where
+ // attribute instanceof ListType && !attribute.isNonEmpty(), but BuildRuleBuilder
+ // doesn't support that, and it makes little sense anyway
populateLabelAttribute(rulePkg, attribute);
} else {
// Non label type attributes
