From 46963d2d3c85957df8841a02cd4094ea547dddc8 Mon Sep 17 00:00:00 2001 From: John Cater Date: Tue, 24 Nov 2020 13:54:53 -0800 Subject: [PATCH] Move getConfigConditions into ConfiguredTarget. 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 --- .../com/google/devtools/build/lib/analysis/BUILD | 1 + .../build/lib/analysis/ConfiguredTarget.java | 10 ++++++++++ .../devtools/build/lib/analysis/RuleContext.java | 2 +- .../build/lib/analysis/TargetCompleteEvent.java | 2 +- .../configuredtargets/RuleConfiguredTarget.java | 5 ++--- .../cquery/BuildOutputFormatterCallback.java | 10 +--------- .../query2/cquery/ConfiguredTargetAccessor.java | 2 +- .../cquery/ProtoOutputFormatterCallback.java | 16 ++++------------ .../TransitionsOutputFormatterCallback.java | 2 +- .../build/lib/rules/AliasConfiguredTarget.java | 1 + .../lib/runtime/commands/PrintActionCommand.java | 4 +--- .../lib/analysis/util/BuildViewTestCase.java | 3 +-- 12 files changed, 25 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index cb33c634e1f878..1b1f10a7b3ad4d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -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", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java index dda55c53b7abf8..4404f567482f91 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java @@ -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; @@ -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 getConfigConditions() { + return ImmutableMap.of(); + } } 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 b1fce278e1f610..8c8251b501b499 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 @@ -1712,7 +1712,7 @@ public static final class Builder implements RuleErrorConsumer { private final PrerequisiteValidator prerequisiteValidator; private final RuleErrorConsumer reporter; private OrderedSetMultimap prerequisiteMap; - private ImmutableMap configConditions; + private ImmutableMap configConditions = ImmutableMap.of(); private NestedSet visibility; private ImmutableMap aspectAttributes; private ImmutableList aspects; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java index 8ff17f62120735..99078d1c74db08 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java @@ -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 diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java index cab8b551cad765..04df6f23cb2877 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java @@ -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 getConfigConditions() { return configConditions; } diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java index 8ac9eecb747ead..dc695036dfe5ce 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/BuildOutputFormatterCallback.java @@ -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; @@ -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; @@ -77,9 +74,6 @@ 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, @@ -87,9 +81,7 @@ private ConfiguredAttributeMapper getAttributeMap(ConfiguredTarget ct) .getGeneratingConfiguredTarget((OutputFileConfiguredTarget) ct) .getConfigConditions()); } else { - Verify.verify(ct instanceof RuleConfiguredTarget); - return ConfiguredAttributeMapper.of( - associatedRule, ((RuleConfiguredTarget) ct).getConfigConditions()); + return ConfiguredAttributeMapper.of(associatedRule, ct.getConfigConditions()); } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java index 3320a5e4c6664b..877d459972271c 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetAccessor.java @@ -123,7 +123,7 @@ public List getPrerequisites( Rule rule = (Rule) getTargetFromConfiguredTarget(actualConfiguredTarget); ImmutableMap configConditions = - ((RuleConfiguredTarget) actualConfiguredTarget).getConfigConditions(); + actualConfiguredTarget.getConfigConditions(); ConfiguredAttributeMapper attributeMapper = ConfiguredAttributeMapper.of(rule, configConditions); if (!attributeMapper.has(attrName)) { diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java index 79e5325d77cb4a..c5e7c20df60d0d 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/ProtoOutputFormatterCallback.java @@ -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; @@ -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; @@ -163,18 +161,12 @@ private class ConfiguredProtoOutputFormatter extends ProtoOutputFormatter { @Override protected void addAttributes( Build.Rule.Builder rulePb, Rule rule, Object extraDataForAttrHash) { - // We know currentTarget will be one of these two types of configured targets + // We know currentTarget 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 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 configConditions = + currentTarget.getConfigConditions(); ConfiguredAttributeMapper attributeMapper = ConfiguredAttributeMapper.of(rule, configConditions); for (Attribute attr : sortAttributes(rule.getAttributes())) { diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java index 8a8f335b5a72f5..2a954b8aa63769 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java @@ -115,7 +115,7 @@ public void processOutput(Iterable partialResult) throws Inter } OrderedSetMultimap deps; ImmutableMap configConditions = - ((RuleConfiguredTarget) configuredTarget).getConfigConditions(); + configuredTarget.getConfigConditions(); // Get a ToolchainContext to use for dependency resolution. ToolchainCollection toolchainContexts = diff --git a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java index db1d7aa13cb589..4c353e2e3dcfae 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java @@ -84,6 +84,7 @@ public boolean isImmutable() { return true; // immutable and Starlark-hashable } + @Override public ImmutableMap getConfigConditions() { return configConditions; } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java index 1bcd05b3436b6b..66a239ada48cf5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/PrintActionCommand.java @@ -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; @@ -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 = @@ -404,7 +402,7 @@ public boolean shouldOutput( } List