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/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));
}
