Make TargetPatternKey implement SkyKey instead of wrapping in LegacySkyKey

Makes for much nicer client code.

PiperOrigin-RevId: 162454439

7 files changed, 48 insertions, 45 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
index fe54c4d033..6cc8fd1585 100644
--- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java
@@ -688,10 +688,8 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target>
   private Pair<TargetPattern, ImmutableSet<PathFragment>> getPatternAndExcludes(String pattern)
       throws TargetParsingException, InterruptedException {
     TargetPatternKey targetPatternKey =
-        ((TargetPatternKey)
-            TargetPatternValue.key(
-                    pattern, TargetPatternEvaluator.DEFAULT_FILTERING_POLICY, parserPrefix)
-                .argument());
+        TargetPatternValue.key(
+            pattern, TargetPatternEvaluator.DEFAULT_FILTERING_POLICY, parserPrefix);
     ImmutableSet<PathFragment> subdirectoriesToExclude =
         targetPatternKey.getAllSubdirectoriesToExclude(blacklistPatternsSupplier);
     return Pair.of(targetPatternKey.getParsedPattern(), subdirectoriesToExclude);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index d967df0956..1901c7862e 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -69,6 +69,7 @@ import com.google.devtools.build.lib.skyframe.PackageValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
 import com.google.devtools.build.lib.skyframe.SkyFunctions;
 import com.google.devtools.build.lib.skyframe.TargetPatternValue;
+import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
 import com.google.devtools.build.lib.skyframe.TransitiveTargetValue;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.Fingerprint;
@@ -409,7 +410,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
       boolean ok = true;
       Map<String, ResolvedTargets<Target>> preloadedPatterns =
           Maps.newHashMapWithExpectedSize(patterns.size());
-      Map<SkyKey, String> patternKeys = Maps.newHashMapWithExpectedSize(patterns.size());
+      Map<TargetPatternKey, String> patternKeys = Maps.newHashMapWithExpectedSize(patterns.size());
       for (String pattern : patterns) {
         checkValidPatternType(pattern);
         patternKeys.put(TargetPatternValue.key(pattern, FilteringPolicies.NO_FILTER, ""), pattern);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
index 79fd2fb9b2..f54604d536 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
@@ -89,18 +89,17 @@ public class PrepareDepsOfPatternValue implements SkyValue {
     ImmutableList.Builder<PrepareDepsOfPatternSkyKeyOrException> builder = ImmutableList.builder();
     for (int i = 0; i < keysMaybe.size(); i++) {
       TargetPatternSkyKeyOrException keyMaybe = keysMaybe.get(i);
-      SkyKey skyKey;
+      TargetPatternKey targetPatternKey;
       try {
-        skyKey = keyMaybe.getSkyKey();
+        targetPatternKey = keyMaybe.getSkyKey();
       } catch (TargetParsingException e) {
         // keyMaybe.getSkyKey() may throw TargetParsingException if its corresponding pattern
         // failed to parse. If so, wrap the exception and return it, so that our caller can
         // deal with it.
-        skyKey = null;
+        targetPatternKey = null;
         builder.add(new PrepareDepsOfPatternSkyKeyException(e, keyMaybe.getOriginalPattern()));
       }
-      if (skyKey != null) {
-        TargetPatternKey targetPatternKey = (TargetPatternKey) skyKey.argument();
+      if (targetPatternKey != null) {
         if (targetPatternKey.isNegative()) {
           if (!targetPatternKey.getParsedPattern().getType().equals(Type.TARGETS_BELOW_DIRECTORY)) {
             builder.add(
@@ -134,14 +133,13 @@ public class PrepareDepsOfPatternValue implements SkyValue {
     ImmutableSet.Builder<PathFragment> excludedDirectoriesBuilder = ImmutableSet.builder();
     for (int j = position + 1; j < keysMaybe.size(); j++) {
       TargetPatternSkyKeyOrException laterPatternMaybe = keysMaybe.get(j);
-      SkyKey laterSkyKey;
+      TargetPatternKey laterTargetPatternKey;
       try {
-        laterSkyKey = laterPatternMaybe.getSkyKey();
+        laterTargetPatternKey = laterPatternMaybe.getSkyKey();
       } catch (TargetParsingException ignored) {
-        laterSkyKey = null;
+        laterTargetPatternKey = null;
       }
-      if (laterSkyKey != null) {
-        TargetPatternKey laterTargetPatternKey = (TargetPatternKey) laterSkyKey.argument();
+      if (laterTargetPatternKey != null) {
         TargetPattern laterParsedPattern = laterTargetPatternKey.getParsedPattern();
         if (laterTargetPatternKey.isNegative()
             && laterParsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index e1239e1bc1..379d3de9d7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -118,6 +118,7 @@ import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossReposit
 import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionCompletedReceiver;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ProgressSupplier;
+import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
 import com.google.devtools.build.lib.syntax.SkylarkSemanticsOptions;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.ExitCode;
@@ -1249,7 +1250,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   EvaluationResult<TargetPatternValue> targetPatterns(
-      Iterable<SkyKey> patternSkyKeys,
+      Iterable<TargetPatternKey> patternSkyKeys,
       int numThreads,
       boolean keepGoing,
       ExtendedEventHandler eventHandler)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
index a0ccc8894c..c84e84946c 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeTargetPatternEvaluator.java
@@ -26,12 +26,12 @@ import com.google.devtools.build.lib.pkgcache.FilteringPolicies;
 import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
 import com.google.devtools.build.lib.pkgcache.ParsingFailedEvent;
 import com.google.devtools.build.lib.pkgcache.TargetPatternEvaluator;
+import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternKey;
 import com.google.devtools.build.lib.skyframe.TargetPatternValue.TargetPatternSkyKeyOrException;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.ErrorInfo;
 import com.google.devtools.build.skyframe.EvaluationResult;
-import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.WalkableGraph;
 import java.util.Collection;
 import java.util.List;
@@ -105,7 +105,7 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator {
     Iterable<TargetPatternSkyKeyOrException> keysMaybe =
         TargetPatternValue.keys(targetPatterns, policy, offset);
 
-    ImmutableList.Builder<SkyKey> builder = ImmutableList.builder();
+    ImmutableList.Builder<TargetPatternKey> builder = ImmutableList.builder();
     for (TargetPatternSkyKeyOrException skyKeyOrException : keysMaybe) {
       try {
         builder.add(skyKeyOrException.getSkyKey());
@@ -122,7 +122,7 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator {
         eventHandler.handle(Event.error("Skipping '" + pattern + "': " + e.getMessage()));
       }
     }
-    ImmutableList<SkyKey> skyKeys = builder.build();
+    ImmutableList<TargetPatternKey> skyKeys = builder.build();
     return parseTargetPatternKeys(
         targetPatterns,
         skyKeys,
@@ -140,9 +140,9 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator {
         : new BuildTargetPatternsResultBuilder();
   }
 
-  ResolvedTargets<Target> parseTargetPatternKeys(
+  private ResolvedTargets<Target> parseTargetPatternKeys(
       List<String> targetPattern,
-      Iterable<SkyKey> patternSkyKeys,
+      Iterable<TargetPatternKey> patternSkyKeys,
       int numThreads,
       boolean keepGoing,
       ExtendedEventHandler eventHandler,
@@ -152,19 +152,17 @@ final class SkyframeTargetPatternEvaluator implements TargetPatternEvaluator {
         skyframeExecutor.targetPatterns(patternSkyKeys, numThreads, keepGoing, eventHandler);
 
     String errorMessage = null;
-    for (SkyKey key : patternSkyKeys) {
+    for (TargetPatternKey key : patternSkyKeys) {
       TargetPatternValue resultValue = result.get(key);
       if (resultValue != null) {
         ResolvedTargets<Label> results = resultValue.getTargets();
-        if (((TargetPatternValue.TargetPatternKey) key.argument()).isNegative()) {
+        if (key.isNegative()) {
           finalTargetSetEvaluator.addLabelsOfNegativePattern(results);
         } else {
           finalTargetSetEvaluator.addLabelsOfPositivePattern(results);
         }
       } else {
-        TargetPatternValue.TargetPatternKey patternKey =
-            (TargetPatternValue.TargetPatternKey) key.argument();
-        String rawPattern = patternKey.getPattern();
+        String rawPattern = key.getPattern();
         ErrorInfo error = result.errorMap().get(key);
         if (error == null) {
           Preconditions.checkState(!keepGoing);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
index 967883c698..13b1556f22 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternPhaseFunction.java
@@ -188,7 +188,7 @@ final class TargetPatternPhaseFunction implements SkyFunction {
    */
   private static ResolvedTargets<Target> getTargetsToBuild(
       Environment env, TargetPatternList options) throws InterruptedException {
-    List<SkyKey> patternSkyKeys = new ArrayList<>();
+    List<TargetPatternKey> patternSkyKeys = new ArrayList<>();
     for (TargetPatternSkyKeyOrException keyOrException :
         TargetPatternValue.keys(
             options.getTargetPatterns(),
@@ -214,11 +214,10 @@ final class TargetPatternPhaseFunction implements SkyFunction {
     }
 
     ResolvedTargets.Builder<Target> builder = ResolvedTargets.builder();
-    for (SkyKey key : patternSkyKeys) {
-      TargetPatternKey pattern = (TargetPatternKey) key.argument();
+    for (TargetPatternKey pattern : patternSkyKeys) {
       TargetPatternValue value;
       try {
-        value = (TargetPatternValue) resolvedPatterns.get(key).get();
+        value = (TargetPatternValue) resolvedPatterns.get(pattern).get();
       } catch (TargetParsingException e) {
         String rawPattern = pattern.getPattern();
         String errorMessage = e.getMessage();
@@ -264,7 +263,7 @@ final class TargetPatternPhaseFunction implements SkyFunction {
   private static ResolvedTargets<Target> determineTests(
       Environment env, List<String> targetPatterns, String offset, TestFilter testFilter)
       throws InterruptedException {
-    List<SkyKey> patternSkyKeys = new ArrayList<>();
+    List<TargetPatternKey> patternSkyKeys = new ArrayList<>();
     for (TargetPatternSkyKeyOrException keyOrException :
         TargetPatternValue.keys(targetPatterns, FilteringPolicies.FILTER_TESTS, offset)) {
       try {
@@ -280,7 +279,7 @@ final class TargetPatternPhaseFunction implements SkyFunction {
     }
 
     List<SkyKey> expandedSuiteKeys = new ArrayList<>();
-    for (SkyKey key : patternSkyKeys) {
+    for (TargetPatternKey key : patternSkyKeys) {
       TargetPatternValue value;
       try {
         value = (TargetPatternValue) resolvedPatterns.get(key).get();
@@ -296,11 +295,10 @@ final class TargetPatternPhaseFunction implements SkyFunction {
     }
 
     ResolvedTargets.Builder<Target> testTargetsBuilder = ResolvedTargets.builder();
-    for (SkyKey key : patternSkyKeys) {
-      TargetPatternKey pattern = (TargetPatternKey) key.argument();
+    for (TargetPatternKey pattern : patternSkyKeys) {
       TargetPatternValue value;
       try {
-        value = (TargetPatternValue) resolvedPatterns.get(key).get();
+        value = (TargetPatternValue) resolvedPatterns.get(pattern).get();
       } catch (TargetParsingException e) {
         // This was already reported in getTargetsToBuild (maybe merge the two code paths?).
         continue;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
index ce65642e65..9d9ac22066 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
@@ -31,7 +31,7 @@ import com.google.devtools.build.lib.pkgcache.FilteringPolicy;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.InterruptibleSupplier;
-import com.google.devtools.build.skyframe.LegacySkyKey;
+import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
 import java.io.IOException;
@@ -109,7 +109,7 @@ public final class TargetPatternValue implements SkyValue {
    * @param offset The offset to apply to relative target patterns.
    */
   @ThreadSafe
-  public static SkyKey key(String pattern, FilteringPolicy policy, String offset)
+  public static TargetPatternKey key(String pattern, FilteringPolicy policy, String offset)
       throws TargetParsingException {
     return Iterables.getOnlyElement(keys(ImmutableList.of(pattern), policy, offset)).getSkyKey();
   }
@@ -147,8 +147,7 @@ public final class TargetPatternValue implements SkyValue {
               !positive,
               offset,
               ImmutableSet.<PathFragment>of());
-      SkyKey skyKey = LegacySkyKey.create(SkyFunctions.TARGET_PATTERN, targetPatternKey);
-      builder.add(new TargetPatternSkyKeyValue(skyKey));
+      builder.add(new TargetPatternSkyKeyValue(targetPatternKey));
     }
     return builder.build();
   }
@@ -162,7 +161,7 @@ public final class TargetPatternValue implements SkyValue {
    * offset, whether it is a positive or negative match, and a set of excluded subdirectories.
    */
   @ThreadSafe
-  public static class TargetPatternKey implements Serializable {
+  public static class TargetPatternKey implements SkyKey, Serializable {
 
     private final TargetPattern parsedPattern;
     private final FilteringPolicy policy;
@@ -184,6 +183,16 @@ public final class TargetPatternValue implements SkyValue {
       this.excludedSubdirectories = Preconditions.checkNotNull(excludedSubdirectories);
     }
 
+    @Override
+    public SkyFunctionName functionName() {
+      return SkyFunctions.TARGET_PATTERN;
+    }
+
+    @Override
+    public Object argument() {
+      return this;
+    }
+
     public String getPattern() {
       return parsedPattern.getOriginalPattern();
     }
@@ -267,7 +276,7 @@ public final class TargetPatternValue implements SkyValue {
      * Returns the stored {@link SkyKey} or throws {@link TargetParsingException} if one was thrown
      * when computing the key.
      */
-    SkyKey getSkyKey() throws TargetParsingException;
+    TargetPatternKey getSkyKey() throws TargetParsingException;
 
     /**
      * Returns the pattern that resulted in the stored {@link SkyKey} or {@link
@@ -278,14 +287,14 @@ public final class TargetPatternValue implements SkyValue {
 
   private static final class TargetPatternSkyKeyValue implements TargetPatternSkyKeyOrException {
 
-    private final SkyKey value;
+    private final TargetPatternKey value;
 
-    private TargetPatternSkyKeyValue(SkyKey value) {
+    private TargetPatternSkyKeyValue(TargetPatternKey value) {
       this.value = value;
     }
 
     @Override
-    public SkyKey getSkyKey() throws TargetParsingException {
+    public TargetPatternKey getSkyKey() throws TargetParsingException {
       return value;
     }
 
@@ -307,7 +316,7 @@ public final class TargetPatternValue implements SkyValue {
     }
 
     @Override
-    public SkyKey getSkyKey() throws TargetParsingException {
+    public TargetPatternKey getSkyKey() throws TargetParsingException {
       throw exception;
     }