Enforce that the SkyKey returned by ActionLookupKey#getSkyKey is an ActionLookupKey. It was only being violated for some singleton keys.

Also do some drive-by cleanups of unused variables. Step approximately -2.

--
PiperOrigin-RevId: 150668944
MOS_MIGRATED_REVID=150668944

4 files changed, 21 insertions, 30 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupValue.java
index 30ac3d2c3a..f5aab9a825 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionLookupValue.java
@@ -22,10 +22,10 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactOwner;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
 import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
-
 import java.util.Map;
 
 /**
@@ -68,14 +68,6 @@ public class ActionLookupValue implements SkyValue {
     return generatingActionMap;
   }
 
-  /**
-   * To be used only when setting the owners of deserialized artifacts whose owners were unknown at
-   * creation time -- not by other callers or values.
-   */
-  Iterable<ActionAnalysisMetadata> getActionsForFindingArtifactOwners() {
-    return generatingActionMap.values();
-  }
-
   @VisibleForTesting
   public static SkyKey key(ActionLookupKey ownerKey) {
     return ownerKey.getSkyKey();
@@ -101,13 +93,24 @@ public class ActionLookupValue implements SkyValue {
      */
     abstract SkyFunctionName getType();
 
+    SkyKey getSkyKeyInternal() {
+      return SkyKey.create(getType(), this);
+    }
+
     /**
      * Prefer {@link ActionLookupValue#key} to calling this method directly.
      *
-     * <p>Subclasses may override if the value key contents should not be the key itself.
+     * <p>Subclasses may override {@link #getSkyKeyInternal} if the {@link SkyKey} argument should
+     * not be this {@link ActionLookupKey} itself.
      */
-    SkyKey getSkyKey() {
-      return SkyKey.create(getType(), this);
+    final SkyKey getSkyKey() {
+      SkyKey result = getSkyKeyInternal();
+      Preconditions.checkState(
+          result.argument() instanceof ActionLookupKey,
+          "Not ActionLookupKey for %s: %s",
+          this,
+          result);
+      return result;
     }
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
index 70de5cc212..47ce1774c7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportFunction.java
@@ -46,9 +46,7 @@ public class CoverageReportFunction implements SkyFunction {
       outputs.addAll(action.getOutputs());
     }
 
-    return new CoverageReportValue(
-        outputs.build(),
-        actions);
+    return new CoverageReportValue(actions);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
index 27ace7605b..164b37e16e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/CoverageReportValue.java
@@ -15,9 +15,7 @@
 package com.google.devtools.build.lib.skyframe;
 
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
-import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactOwner;
 import com.google.devtools.build.skyframe.SkyFunctionName;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -26,21 +24,13 @@ import com.google.devtools.build.skyframe.SkyKey;
  * A SkyValue to store the coverage report Action and Artifacts.
  */
 public class CoverageReportValue extends ActionLookupValue {
-  private final ImmutableSet<Artifact> coverageReportArtifacts;
 
   // There should only ever be one CoverageReportValue value in the graph.
-  public static final SkyKey SKY_KEY =
-      SkyKey.create(SkyFunctions.COVERAGE_REPORT, "COVERAGE_REPORT");
   public static final ArtifactOwner ARTIFACT_OWNER = new CoverageReportKey();
+  static final SkyKey SKY_KEY = SkyKey.create(SkyFunctions.COVERAGE_REPORT, ARTIFACT_OWNER);
 
-  public CoverageReportValue(ImmutableSet<Artifact> coverageReportArtifacts,
-      ImmutableList <ActionAnalysisMetadata> coverageReportActions) {
+  CoverageReportValue(ImmutableList<ActionAnalysisMetadata> coverageReportActions) {
     super(coverageReportActions);
-    this.coverageReportArtifacts = coverageReportArtifacts;
-  }
-
-  public ImmutableSet<Artifact> getCoverageReportArtifacts() {
-    return coverageReportArtifacts;
   }
 
   private static class CoverageReportKey extends ActionLookupKey {
@@ -50,7 +40,7 @@ public class CoverageReportValue extends ActionLookupValue {
     }
 
     @Override
-    SkyKey getSkyKey() {
+    SkyKey getSkyKeyInternal() {
       return SKY_KEY;
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
index fca72dbc49..62f7858190 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceStatusValue.java
@@ -30,8 +30,8 @@ public class WorkspaceStatusValue extends ActionLookupValue {
   private final Artifact volatileArtifact;
 
   // There should only ever be one BuildInfo value in the graph.
-  public static final SkyKey SKY_KEY = SkyKey.create(SkyFunctions.BUILD_INFO, "BUILD_INFO");
   static final ArtifactOwner ARTIFACT_OWNER = new BuildInfoKey();
+  public static final SkyKey SKY_KEY = SkyKey.create(SkyFunctions.BUILD_INFO, ARTIFACT_OWNER);
 
   public WorkspaceStatusValue(Artifact stableArtifact, Artifact volatileArtifact,
       WorkspaceStatusAction action) {
@@ -55,7 +55,7 @@ public class WorkspaceStatusValue extends ActionLookupValue {
     }
 
     @Override
-    SkyKey getSkyKey() {
+    SkyKey getSkyKeyInternal() {
       return SKY_KEY;
     }
   }