Skip to content

Commit

Permalink
Move getConfigConditions into ConfiguredTarget.
Browse files Browse the repository at this point in the history
Uses a default value for non-alias and non-rule CTs.

This alloows removing many instanceof/cast checks when getting config
conditions.

Part of the work on #11993.

Closes #12550.

PiperOrigin-RevId: 344126290
  • Loading branch information
katre authored and philwo committed Mar 15, 2021
1 parent c652976 commit 46963d2
Show file tree
Hide file tree
Showing 12 changed files with 25 additions and 33 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ java_library(
name = "configured_target",
srcs = ["ConfiguredTarget.java"],
deps = [
":config/config_matching_provider",
":transitive_info_collection",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/cmdline",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
package com.google.devtools.build.lib.analysis;

import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.Artifact.SourceArtifact;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -91,4 +93,12 @@ default ConfiguredTarget getActual() {
default Label getOriginalLabel() {
return getLabel();
}

/**
* The configuration conditions that trigger this configured target's configurable attributes. For
* targets that do not support configurable attributes, this will be an empty map.
*/
default ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions() {
return ImmutableMap.of();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1712,7 +1712,7 @@ public static final class Builder implements RuleErrorConsumer {
private final PrerequisiteValidator prerequisiteValidator;
private final RuleErrorConsumer reporter;
private OrderedSetMultimap<Attribute, ConfiguredTargetAndData> prerequisiteMap;
private ImmutableMap<Label, ConfigMatchingProvider> configConditions;
private ImmutableMap<Label, ConfigMatchingProvider> configConditions = ImmutableMap.of();
private NestedSet<PackageGroupContents> visibility;
private ImmutableMap<String, Attribute> aspectAttributes;
private ImmutableList<Aspect> aspects;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private TargetCompleteEvent(
AttributeMap attributes =
ConfiguredAttributeMapper.of(
(Rule) targetAndData.getTarget(),
((RuleConfiguredTarget) targetAndData.getConfiguredTarget()).getConfigConditions());
targetAndData.getConfiguredTarget().getConfigConditions());
// Every build rule (implicitly) has a "tags" attribute. However other rule configured targets
// are repository rules (which don't have a tags attribute); morevoer, thanks to the virtual
// "external" package, they are user visible as targets and can create a completed event as
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,8 @@ public RuleConfiguredTarget(
}
}

/**
* The configuration conditions that trigger this rule's configurable attributes.
*/
/** The configuration conditions that trigger this rule's configurable attributes. */
@Override
public ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions() {
return configConditions;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,9 @@

package com.google.devtools.build.lib.query2.cquery;

import com.google.common.base.Verify;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.OutputFileConfiguredTarget;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper;
Expand All @@ -30,7 +28,6 @@
import com.google.devtools.build.lib.query2.query.output.BuildOutputFormatter.AttributeReader;
import com.google.devtools.build.lib.query2.query.output.BuildOutputFormatter.TargetOutputter;
import com.google.devtools.build.lib.query2.query.output.PossibleAttributeValues;
import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import java.io.IOException;
import java.io.OutputStream;
Expand Down Expand Up @@ -77,19 +74,14 @@ private ConfiguredAttributeMapper getAttributeMap(ConfiguredTarget ct)
Rule associatedRule = accessor.getTargetFromConfiguredTarget(ct).getAssociatedRule();
if (associatedRule == null) {
return null;
} else if (ct instanceof AliasConfiguredTarget) {
return ConfiguredAttributeMapper.of(
associatedRule, ((AliasConfiguredTarget) ct).getConfigConditions());
} else if (ct instanceof OutputFileConfiguredTarget) {
return ConfiguredAttributeMapper.of(
associatedRule,
accessor
.getGeneratingConfiguredTarget((OutputFileConfiguredTarget) ct)
.getConfigConditions());
} else {
Verify.verify(ct instanceof RuleConfiguredTarget);
return ConfiguredAttributeMapper.of(
associatedRule, ((RuleConfiguredTarget) ct).getConfigConditions());
return ConfiguredAttributeMapper.of(associatedRule, ct.getConfigConditions());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ public List<ConfiguredTarget> getPrerequisites(

Rule rule = (Rule) getTargetFromConfiguredTarget(actualConfiguredTarget);
ImmutableMap<Label, ConfigMatchingProvider> configConditions =
((RuleConfiguredTarget) actualConfiguredTarget).getConfigConditions();
actualConfiguredTarget.getConfigConditions();
ConfiguredAttributeMapper attributeMapper =
ConfiguredAttributeMapper.of(rule, configConditions);
if (!attributeMapper.has(attrName)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import com.google.devtools.build.lib.analysis.AnalysisProtos;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.Attribute;
Expand All @@ -31,7 +30,6 @@
import com.google.devtools.build.lib.query2.proto.proto2api.Build.QueryResult;
import com.google.devtools.build.lib.query2.query.aspectresolvers.AspectResolver;
import com.google.devtools.build.lib.query2.query.output.ProtoOutputFormatter;
import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.protobuf.Message;
import com.google.protobuf.TextFormat;
Expand Down Expand Up @@ -163,18 +161,12 @@ private class ConfiguredProtoOutputFormatter extends ProtoOutputFormatter {
@Override
protected void addAttributes(
Build.Rule.Builder rulePb, Rule rule, Object extraDataForAttrHash) {
// We know <code>currentTarget</code> will be one of these two types of configured targets
// We know <code>currentTarget</code> will be either an AliasConfiguredTarget or
// RuleConfiguredTarget,
// because this method is only triggered in ProtoOutputFormatter.toTargetProtoBuffer when
// the target in currentTarget is an instanceof Rule.
ImmutableMap<Label, ConfigMatchingProvider> configConditions;
if (currentTarget instanceof AliasConfiguredTarget) {
configConditions = ((AliasConfiguredTarget) currentTarget).getConfigConditions();
} else if (currentTarget instanceof RuleConfiguredTarget) {
configConditions = ((RuleConfiguredTarget) currentTarget).getConfigConditions();
} else {
// Other subclasses of ConfiguredTarget don't have attribute information.
return;
}
ImmutableMap<Label, ConfigMatchingProvider> configConditions =
currentTarget.getConfigConditions();
ConfiguredAttributeMapper attributeMapper =
ConfiguredAttributeMapper.of(rule, configConditions);
for (Attribute attr : sortAttributes(rule.getAttributes())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void processOutput(Iterable<ConfiguredTarget> partialResult) throws Inter
}
OrderedSetMultimap<DependencyKind, DependencyKey> deps;
ImmutableMap<Label, ConfigMatchingProvider> configConditions =
((RuleConfiguredTarget) configuredTarget).getConfigConditions();
configuredTarget.getConfigConditions();

// Get a ToolchainContext to use for dependency resolution.
ToolchainCollection<ToolchainContext> toolchainContexts =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ public boolean isImmutable() {
return true; // immutable and Starlark-hashable
}

@Override
public ImmutableMap<Label, ConfigMatchingProvider> getConfigConditions() {
return configConditions;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.OutputGroupInfo;
import com.google.devtools.build.lib.analysis.PrintActionVisitor;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.buildtool.BuildResult;
import com.google.devtools.build.lib.buildtool.BuildTool;
Expand Down Expand Up @@ -389,7 +388,6 @@ public boolean shouldOutput(

// C++ header files show up in the dependency on the Target, but not the ConfiguredTarget, so
// we also check the target's header files there.
RuleConfiguredTarget ruleConfiguredTarget = (RuleConfiguredTarget) configuredTarget;
Rule rule;
try {
rule =
Expand All @@ -404,7 +402,7 @@ public boolean shouldOutput(
}

List<Label> hdrs =
ConfiguredAttributeMapper.of(rule, ruleConfiguredTarget.getConfigConditions())
ConfiguredAttributeMapper.of(rule, configuredTarget.getConfigConditions())
.get("hdrs", BuildType.LABEL_LIST);
if (hdrs != null) {
for (Label hdrLabel : hdrs) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1672,8 +1672,7 @@ protected void assertSrcsValidity(String ruleType, String targetName, boolean ex
protected static ConfiguredAttributeMapper getMapperFromConfiguredTargetAndTarget(
ConfiguredTargetAndData ctad) {
return ConfiguredAttributeMapper.of(
(Rule) ctad.getTarget(),
((RuleConfiguredTarget) ctad.getConfiguredTarget()).getConfigConditions());
(Rule) ctad.getTarget(), ctad.getConfiguredTarget().getConfigConditions());
}

public static Label makeLabel(String label) {
Expand Down

0 comments on commit 46963d2

Please sign in to comment.