Remove LoadingResult

Instead, refactor the code to use TargetPatternPhaseValue exclusively. This
removes the need to convert from TargetPatternPhaseValue to LoadingResult, and
prepares for interleaving.

It also reduces the number of Skyframe calls which may speed up null builds a
bit, as a followup for https://github.com/bazelbuild/bazel/commit/1067310e18cb9ac203110726de0be53bdc403cea.

PiperOrigin-RevId: 205989338

5 files changed, 172 insertions, 182 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
index 1d635c02b3..91d7438e23 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -133,7 +133,8 @@ public class BuildViewTest extends BuildViewTestBase {
     targets =
         Lists.<ConfiguredTarget>newArrayList(
             BuildView.filterTestsByTargets(
-                targets, Sets.newHashSet(test1.getTarget(), suite.getTarget())));
+                targets,
+                Sets.newHashSet(test1.getTarget().getLabel(), suite.getTarget().getLabel())));
     assertThat(targets).containsExactlyElementsIn(Sets.newHashSet(test1CT, suiteCT));
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index b583a185a4..22d8fff15c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -51,7 +51,6 @@ import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.util.MockToolsConfig;
 import com.google.devtools.build.lib.pkgcache.LoadingOptions;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -64,6 +63,7 @@ import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
 import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import com.google.devtools.build.lib.testutil.TestConstants;
@@ -347,7 +347,7 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
     skyframeExecutor.invalidateFilesUnderPathForTesting(
         reporter, ModifiedFileSet.EVERYTHING_MODIFIED, Root.fromPath(rootDirectory));
 
-    LoadingResult loadingResult =
+    TargetPatternPhaseValue loadingResult =
         skyframeExecutor.loadTargetPatterns(
             reporter,
             ImmutableList.copyOf(labels),
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index b3d9a06ca8..a0fb40f01c 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -74,12 +74,12 @@ import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupC
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
 import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
 import com.google.devtools.build.lib.skyframe.ToolchainException;
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
@@ -96,7 +96,6 @@ import java.util.Set;
  * tests access to Skyframe internals. The code largely predates the introduction of Skyframe, and
  * mostly exists to avoid having to rewrite our tests to work with Skyframe natively.
  */
-@VisibleForTesting
 public class BuildViewForTesting {
   private final BuildView buildView;
   private final SkyframeExecutor skyframeExecutor;
@@ -140,7 +139,7 @@ public class BuildViewForTesting {
 
   @ThreadCompatible
   public AnalysisResult update(
-      LoadingResult loadingResult,
+      TargetPatternPhaseValue loadingResult,
       BuildConfigurationCollection configurations,
       List<String> aspects,
       AnalysisOptions viewOptions,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index a5d8368792..739437d231 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -123,7 +123,6 @@ import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.util.MockToolsConfig;
 import com.google.devtools.build.lib.pkgcache.LoadingOptions;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -139,6 +138,7 @@ import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
 import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
 import com.google.devtools.build.lib.syntax.SkylarkSemantics;
 import com.google.devtools.build.lib.testutil.BlazeTestUtils;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
@@ -1730,7 +1730,7 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
 
     AnalysisOptions viewOptions = Options.getDefaults(AnalysisOptions.class);
 
-    LoadingResult loadingResult =
+    TargetPatternPhaseValue loadingResult =
         skyframeExecutor.loadTargetPatterns(
             reporter,
             targets,
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 23d5ca181e..cd9246eee9 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -50,6 +50,7 @@ import com.google.devtools.build.lib.skyframe.PatternExpandingError;
 import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
 import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
 import com.google.devtools.build.lib.testutil.ManualClock;
 import com.google.devtools.build.lib.testutil.MoreAsserts;
 import com.google.devtools.build.lib.testutil.TestConstants;
@@ -66,12 +67,10 @@ import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.UUID;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -96,8 +95,12 @@ public class LoadingPhaseRunnerTest {
     tester = new LoadingPhaseTester();
   }
 
-  protected List<Target> getTargets(String... targetNames) throws Exception {
-    return tester.getTargets(targetNames);
+  private List<Label> getLabels(String... labels) throws Exception {
+    List<Label> result = new ArrayList<>();
+    for (String label : labels) {
+      result.add(Label.parseAbsoluteUnchecked(label));
+    }
+    return result;
   }
 
   private void assertCircularSymlinksDuringTargetParsing(String targetPattern) throws Exception {
@@ -110,9 +113,9 @@ public class LoadingPhaseRunnerTest {
     }
   }
 
-  private LoadingResult assertNoErrors(LoadingResult loadingResult) {
-    assertThat(loadingResult.hasTargetPatternError()).isFalse();
-    assertThat(loadingResult.hasLoadingError()).isFalse();
+  private TargetPatternPhaseValue assertNoErrors(TargetPatternPhaseValue loadingResult) {
+    assertThat(loadingResult.hasError()).isFalse();
+    assertThat(loadingResult.hasPostExpansionError()).isFalse();
     tester.assertNoEvents();
     return loadingResult;
   }
@@ -121,18 +124,19 @@ public class LoadingPhaseRunnerTest {
   public void testSmoke() throws Exception {
     tester.addFile("base/BUILD",
         "filegroup(name = 'hello', srcs = ['foo.txt'])");
-    LoadingResult loadingResult = assertNoErrors(tester.load("//base:hello"));
-    assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//base:hello"));
-    assertThat(loadingResult.getTestsToRun()).isNull();
+    TargetPatternPhaseValue loadingResult = assertNoErrors(tester.load("//base:hello"));
+    assertThat(loadingResult.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//base:hello"));
+    assertThat(loadingResult.getTestsToRunLabels()).isNull();
   }
 
   @Test
   public void testNonExistentPackage() throws Exception {
-    LoadingResult loadingResult = tester.loadKeepGoing("//base:missing");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
-    assertThat(loadingResult.hasLoadingError()).isFalse();
-    assertThat(loadingResult.getTargets()).isEmpty();
-    assertThat(loadingResult.getTestsToRun()).isNull();
+    TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//base:missing");
+    assertThat(loadingResult.hasError()).isTrue();
+    assertThat(loadingResult.hasPostExpansionError()).isFalse();
+    assertThat(loadingResult.getTargetLabels()).isEmpty();
+    assertThat(loadingResult.getTestsToRunLabels()).isNull();
     tester.assertContainsError("Skipping '//base:missing': no such package 'base'");
     tester.assertContainsWarning("Target pattern parsing failed.");
     PatternExpandingError err = tester.findPostOnce(PatternExpandingError.class);
@@ -165,11 +169,11 @@ public class LoadingPhaseRunnerTest {
   @Test
   public void testNonExistentTarget() throws Exception {
     tester.addFile("base/BUILD");
-    LoadingResult loadingResult = tester.loadKeepGoing("//base:missing");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
-    assertThat(loadingResult.hasLoadingError()).isFalse();
-    assertThat(loadingResult.getTargets()).isEmpty();
-    assertThat(loadingResult.getTestsToRun()).isNull();
+    TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//base:missing");
+    assertThat(loadingResult.hasError()).isTrue();
+    assertThat(loadingResult.hasPostExpansionError()).isFalse();
+    assertThat(loadingResult.getTargetLabels()).isEmpty();
+    assertThat(loadingResult.getTestsToRunLabels()).isNull();
     tester.assertContainsError("Skipping '//base:missing': no such target '//base:missing'");
     tester.assertContainsWarning("Target pattern parsing failed.");
     PatternExpandingError err = tester.findPostOnce(PatternExpandingError.class);
@@ -203,8 +207,8 @@ public class LoadingPhaseRunnerTest {
 
   @Test
   public void testMistypedTargetKeepGoing() throws Exception {
-    LoadingResult result = tester.loadKeepGoing("foo//bar:missing");
-    assertThat(result.hasTargetPatternError()).isTrue();
+    TargetPatternPhaseValue result = tester.loadKeepGoing("foo//bar:missing");
+    assertThat(result.hasError()).isTrue();
     tester.assertContainsError(
           "invalid target format 'foo//bar:missing': "
           + "invalid package name 'foo//bar': "
@@ -216,11 +220,11 @@ public class LoadingPhaseRunnerTest {
   @Test
   public void testBadTargetPatternWithTest() throws Exception {
     tester.addFile("base/BUILD");
-    LoadingResult loadingResult = tester.loadTestsKeepGoing("//base:missing");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
-    assertThat(loadingResult.hasLoadingError()).isFalse();
-    assertThat(loadingResult.getTargets()).isEmpty();
-    assertThat(loadingResult.getTestsToRun()).isEmpty();
+    TargetPatternPhaseValue loadingResult = tester.loadTestsKeepGoing("//base:missing");
+    assertThat(loadingResult.hasError()).isTrue();
+    assertThat(loadingResult.hasPostExpansionError()).isFalse();
+    assertThat(loadingResult.getTargetLabels()).isEmpty();
+    assertThat(loadingResult.getTestsToRunLabels()).isEmpty();
     tester.assertContainsError("Skipping '//base:missing': no such target '//base:missing'");
     tester.assertContainsWarning("Target pattern parsing failed.");
   }
@@ -229,12 +233,12 @@ public class LoadingPhaseRunnerTest {
   public void testManualTarget() throws Exception {
     AnalysisMock.get().ccSupport().setup(tester.mockToolsConfig);
     tester.addFile("cc/BUILD", "cc_library(name = 'my_lib', srcs = ['lib.cc'], tags = ['manual'])");
-    LoadingResult loadingResult = assertNoErrors(tester.load("//cc:all"));
-    assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets());
+    TargetPatternPhaseValue loadingResult = assertNoErrors(tester.load("//cc:all"));
+    assertThat(loadingResult.getTargetLabels()).containsExactlyElementsIn(getLabels());
 
     // Explicitly specified on the command line.
     loadingResult = assertNoErrors(tester.load("//cc:my_lib"));
-    assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//cc:my_lib"));
+    assertThat(loadingResult.getTargetLabels()).containsExactlyElementsIn(getLabels("//cc:my_lib"));
   }
 
   @Test
@@ -244,14 +248,12 @@ public class LoadingPhaseRunnerTest {
         "cc_library(name = 'somelib', srcs = [ 'somelib.cc' ], hdrs = [ 'somelib.h' ])",
         "config_setting(name = 'configa', values = { 'define': 'foo=a' })",
         "config_setting(name = 'configb', values = { 'define': 'foo=b' })");
-    LoadingResult loadingResult = assertNoErrors(tester.load("//config:all"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//config:somelib"));
+    TargetPatternPhaseValue result = assertNoErrors(tester.load("//config:all"));
+    assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//config:somelib"));
 
     // Explicitly specified on the command line.
-    loadingResult = assertNoErrors(tester.load("//config:configa"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//config:configa"));
+    result = assertNoErrors(tester.load("//config:configa"));
+    assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//config:configa"));
   }
 
   @Test
@@ -259,19 +261,8 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("my_test/BUILD",
         "sh_test(name = 'my_test', srcs = ['test.cc'])");
     assertNoErrors(tester.loadTests("-//my_test"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
-  }
-
-  private static void assertThinTargetsEqualToTargets(
-      Collection<TargetParsingCompleteEvent.ThinTarget> thinTargets, Collection<Target> targets) {
-    assertThat(
-            thinTargets
-                .stream()
-                .map(TargetParsingCompleteEvent.ThinTarget::getLabel)
-                .collect(Collectors.toList()))
-        .containsExactlyElementsIn(
-            targets.stream().map(Target::getLabel).collect(Collectors.toList()));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).isEmpty();
   }
 
   @Test
@@ -279,8 +270,8 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("my_library/BUILD",
         "cc_library(name = 'my_library', srcs = ['test.cc'])");
     assertNoErrors(tester.loadTests("-//my_library"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).isEmpty();
   }
 
   private void writeBuildFilesForTestFiltering() throws Exception {
@@ -293,75 +284,75 @@ public class LoadingPhaseRunnerTest {
   @Test
   public void testTestFiltering() throws Exception {
     writeBuildFilesForTestFiltering();
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
-    assertThat(loadingResult.getTestsToRun())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+    TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
+    assertThat(loadingResult.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+    assertThat(loadingResult.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).isEmpty();
   }
 
   @Test
   public void testTestFilteringIncludingManual() throws Exception {
     writeBuildFilesForTestFiltering();
     tester.useLoadingOptions("--build_manual_tests");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2", "//tests:t3"));
-    assertThat(loadingResult.getTestsToRun())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+    TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
+    assertThat(loadingResult.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2", "//tests:t3"));
+    assertThat(loadingResult.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).isEmpty();
   }
 
   @Test
   public void testTestFilteringBuildTestsOnly() throws Exception {
     writeBuildFilesForTestFiltering();
     tester.useLoadingOptions("--build_tests_only");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
-    assertThat(loadingResult.getTestsToRun())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+    TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all"));
+    assertThat(result.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+    assertThat(result.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).isEmpty();
   }
 
   @Test
   public void testTestFilteringSize() throws Exception {
     writeBuildFilesForTestFiltering();
     tester.useLoadingOptions("--test_size_filters=small");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t2"));
-    assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//tests:t1"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets());
+    TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all"));
+    assertThat(result.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t2"));
+    assertThat(result.getTestsToRunLabels()).containsExactlyElementsIn(getLabels("//tests:t1"));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).isEmpty();
   }
 
   @Test
   public void testTestFilteringSizeAndBuildTestsOnly() throws Exception {
     writeBuildFilesForTestFiltering();
     tester.useLoadingOptions("--test_size_filters=small", "--build_tests_only");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all"));
-    assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//tests:t1"));
-    assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//tests:t1"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets("//tests:t2"));
+    TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all"));
+    assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//tests:t1"));
+    assertThat(result.getTestsToRunLabels()).containsExactlyElementsIn(getLabels("//tests:t1"));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getLabels("//tests:t2"));
   }
 
   @Test
   public void testTestFilteringLocalAndBuildTestsOnly() throws Exception {
     writeBuildFilesForTestFiltering();
     tester.useLoadingOptions("--test_tag_filters=local", "--build_tests_only");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//tests:all", "//tests:t3"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t3"));
-    assertThat(loadingResult.getTestsToRun())
-        .containsExactlyElementsIn(getTargets("//tests:t1", "//tests:t3"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets("//tests:t2"));
+    TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//tests:all", "//tests:t3"));
+    assertThat(result.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t3"));
+    assertThat(result.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//tests:t1", "//tests:t3"));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets()).containsExactlyElementsIn(getLabels("//tests:t2"));
   }
 
   @Test
@@ -370,11 +361,13 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("cc/BUILD",
         "cc_test(name = 'my_test', srcs = ['test.cc'])",
         "test_suite(name = 'tests', tests = [':my_test'])");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:tests"));
-    assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//cc:my_test"));
-    assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets("//cc:my_test"));
-    assertThinTargetsEqualToTargets(
-        tester.getOriginalTargets(), getTargets("//cc:tests", "//cc:my_test"));
+    TargetPatternPhaseValue loadingResult = assertNoErrors(tester.loadTests("//cc:tests"));
+    assertThat(loadingResult.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//cc:my_test"));
+    assertThat(loadingResult.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//cc:my_test"));
+    assertThat(tester.getOriginalTargets())
+        .containsExactlyElementsIn(getLabels("//cc:tests", "//cc:my_test"));
     assertThat(tester.getTestSuiteTargets())
         .containsExactly(Label.parseAbsoluteUnchecked("//cc:tests"));
   }
@@ -384,9 +377,9 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("ts/BUILD",
         "test_suite(name = 'tests', tests = ['//nonexistent:my_test'])");
     tester.useLoadingOptions("--build_tests_only");
-    LoadingResult loadingResult = tester.loadTestsKeepGoing("//ts:tests");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
-    assertThat(loadingResult.hasLoadingError()).isFalse();
+    TargetPatternPhaseValue loadingResult = tester.loadTestsKeepGoing("//ts:tests");
+    assertThat(loadingResult.hasError()).isTrue();
+    assertThat(loadingResult.hasPostExpansionError()).isFalse();
     tester.assertContainsError("no such package 'nonexistent'");
   }
 
@@ -394,9 +387,9 @@ public class LoadingPhaseRunnerTest {
   public void testTestSuiteExpansionFailsForBuild() throws Exception {
     tester.addFile("ts/BUILD",
         "test_suite(name = 'tests', tests = [':nonexistent_test'])");
-    LoadingResult loadingResult = tester.loadKeepGoing("//ts:tests");
-    assertThat(loadingResult.hasTargetPatternError()).isFalse();
-    assertThat(loadingResult.hasLoadingError()).isTrue();
+    TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//ts:tests");
+    assertThat(loadingResult.hasError()).isFalse();
+    assertThat(loadingResult.hasPostExpansionError()).isTrue();
     tester.assertContainsError(
         "expecting a test or a test_suite rule but '//ts:nonexistent_test' is not one");
   }
@@ -406,9 +399,9 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("other/BUILD", "");
     tester.addFile("ts/BUILD",
         "test_suite(name = 'tests', tests = ['//other:no_such_test'])");
-    LoadingResult loadingResult = tester.loadTestsKeepGoing("//ts:tests");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
-    assertThat(loadingResult.hasLoadingError()).isTrue();
+    TargetPatternPhaseValue result = tester.loadTestsKeepGoing("//ts:tests");
+    assertThat(result.hasError()).isTrue();
+    assertThat(result.hasPostExpansionError()).isTrue();
     tester.assertContainsError("no such target '//other:no_such_test'");
   }
 
@@ -418,9 +411,9 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("ts/BUILD",
         "test_suite(name = 'a', tests = ['//other:no_such_test'])",
         "test_suite(name = 'b', tests = [])");
-    LoadingResult loadingResult = tester.loadTestsKeepGoing("//ts:all");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
-    assertThat(loadingResult.hasLoadingError()).isTrue();
+    TargetPatternPhaseValue result = tester.loadTestsKeepGoing("//ts:all");
+    assertThat(result.hasError()).isTrue();
+    assertThat(result.hasPostExpansionError()).isTrue();
     tester.assertContainsError("no such target '//other:no_such_test'");
   }
 
@@ -432,13 +425,14 @@ public class LoadingPhaseRunnerTest {
         "sh_test(name = 'baz', srcs = ['baz.sh'])",
         "test_suite(name = 'foo_suite', tests = [':foo', ':baz'])");
     tester.useLoadingOptions("--build_tests_only");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//foo:all"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//foo:foo", "//foo:baz"));
-    assertThat(loadingResult.getTestsToRun())
-        .containsExactlyElementsIn(getTargets("//foo:foo", "//foo:baz"));
-    assertThinTargetsEqualToTargets(tester.getFilteredTargets(), getTargets());
-    assertThinTargetsEqualToTargets(tester.getTestFilteredTargets(), getTargets("//foo:foo_suite"));
+    TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//foo:all"));
+    assertThat(result.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//foo:foo", "//foo:baz"));
+    assertThat(result.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//foo:foo", "//foo:baz"));
+    assertThat(tester.getFilteredTargets()).isEmpty();
+    assertThat(tester.getTestFilteredTargets())
+        .containsExactlyElementsIn(getLabels("//foo:foo_suite"));
   }
 
   /** Regression test for bug: "subtracting tests from test doesn't work" */
@@ -449,11 +443,12 @@ public class LoadingPhaseRunnerTest {
         "cc_test(name = 'my_test', srcs = ['test.cc'])",
         "cc_test(name = 'my_other_test', srcs = ['other_test.cc'])",
         "test_suite(name = 'tests', tests = [':my_test', ':my_other_test'])");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:tests", "-//cc:my_test"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//cc:my_other_test", "//cc:my_test"));
-    assertThat(loadingResult.getTestsToRun())
-        .containsExactlyElementsIn(getTargets("//cc:my_other_test"));
+    TargetPatternPhaseValue result =
+        assertNoErrors(tester.loadTests("//cc:tests", "-//cc:my_test"));
+    assertThat(result.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//cc:my_other_test", "//cc:my_test"));
+    assertThat(result.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//cc:my_other_test"));
   }
 
   /** Regression test for bug: "blaze doesn't seem to respect target subtractions" */
@@ -465,11 +460,12 @@ public class LoadingPhaseRunnerTest {
         "cc_test(name = 'my_other_test', srcs = ['other_test.cc'])",
         "test_suite(name = 'tests', tests = [':my_test'])",
         "test_suite(name = 'all_tests', tests = ['my_other_test'])");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:all_tests", "-//cc:tests"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//cc:my_other_test"));
-    assertThat(loadingResult.getTestsToRun())
-        .containsExactlyElementsIn(getTargets("//cc:my_other_test"));
+    TargetPatternPhaseValue result =
+        assertNoErrors(tester.loadTests("//cc:all_tests", "-//cc:tests"));
+    assertThat(result.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//cc:my_other_test"));
+    assertThat(result.getTestsToRunLabels())
+        .containsExactlyElementsIn(getLabels("//cc:my_other_test"));
   }
 
   @Test
@@ -482,9 +478,9 @@ public class LoadingPhaseRunnerTest {
         "test_suite(name = 'suite1', tests = ['empty', 'test1'])",
         "test_suite(name = 'suite2', tests = ['test2'])",
         "test_suite(name = 'all_tests', tests = ['suite1', 'suite2'])");
-    LoadingResult loadingResult = assertNoErrors(tester.loadTests("//cc:all_tests"));
-    assertThat(loadingResult.getTargets())
-        .containsExactlyElementsIn(getTargets("//cc:test1", "//cc:test2"));
+    TargetPatternPhaseValue result = assertNoErrors(tester.loadTests("//cc:all_tests"));
+    assertThat(result.getTargetLabels())
+        .containsExactlyElementsIn(getLabels("//cc:test1", "//cc:test2"));
   }
 
   /**
@@ -496,19 +492,19 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("cc/BUILD",
         "cc_test(name = 'my_test', srcs = ['test.cc'])",
         "test_suite(name = 'tests', tests = [':my_test'])");
-    LoadingResult loadingResult = tester.loadTests("//cc:tests", "-//cc:my_test");
+    TargetPatternPhaseValue result = tester.loadTests("//cc:tests", "-//cc:my_test");
     tester.assertContainsWarning("All specified test targets were excluded by filters");
-    assertThat(loadingResult.getTestsToRun()).containsExactlyElementsIn(getTargets());
+    assertThat(result.getTestsToRunLabels()).containsExactlyElementsIn(getLabels());
   }
 
   @Test
   public void testRepeatedSameLoad() throws Exception {
     tester.addFile("base/BUILD",
         "filegroup(name = 'hello', srcs = ['foo.txt'])");
-    LoadingResult firstResult = assertNoErrors(tester.load("//base:hello"));
-    LoadingResult secondResult = assertNoErrors(tester.load("//base:hello"));
-    assertThat(secondResult.getTargets()).isEqualTo(firstResult.getTargets());
-    assertThat(secondResult.getTestsToRun()).isEqualTo(firstResult.getTestsToRun());
+    TargetPatternPhaseValue firstResult = assertNoErrors(tester.load("//base:hello"));
+    TargetPatternPhaseValue secondResult = assertNoErrors(tester.load("//base:hello"));
+    assertThat(secondResult.getTargetLabels()).isEqualTo(firstResult.getTargetLabels());
+    assertThat(secondResult.getTestsToRunLabels()).isEqualTo(firstResult.getTestsToRunLabels());
   }
 
   /**
@@ -521,14 +517,17 @@ public class LoadingPhaseRunnerTest {
   public void testGlobPicksUpNewFile() throws Exception {
     tester.addFile("foo/BUILD", "filegroup(name='x', srcs=glob(['*.y']))");
     tester.addFile("foo/a.y");
-    Target result = Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargets());
+    Label label =
+        Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargetLabels());
+    Target result = tester.getTarget(label.toString());
     assertThat(
         Iterables.transform(result.getAssociatedRule().getLabels(), Functions.toStringFunction()))
         .containsExactly("//foo:a.y");
 
     tester.addFile("foo/b.y");
     tester.sync();
-    result = Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargets());
+    label = Iterables.getOnlyElement(assertNoErrors(tester.load("//foo:x")).getTargetLabels());
+    result = tester.getTarget(label.toString());
     assertThat(
         Iterables.transform(result.getAssociatedRule().getLabels(), Functions.toStringFunction()))
         .containsExactly("//foo:a.y", "//foo:b.y");
@@ -579,8 +578,8 @@ public class LoadingPhaseRunnerTest {
         "test_suite(name = 'a', tests = [':b'])",
         "test_suite(name = 'b', tests = [':c'])",
         "sh_test(name = 'c', srcs = ['test.cc'])");
-    LoadingResult loadingResult = assertNoErrors(tester.load("//suite:a"));
-    assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//suite:c"));
+    TargetPatternPhaseValue result = assertNoErrors(tester.load("//suite:a"));
+    assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//suite:c"));
   }
 
   @Test
@@ -603,8 +602,8 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("bad/BUILD",
         "sh_binary(name = 'bad', srcs = ['bad.sh'])",
         "undefined_symbol");
-    LoadingResult loadingResult = tester.loadKeepGoing("//bad");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
+    TargetPatternPhaseValue result = tester.loadKeepGoing("//bad");
+    assertThat(result.hasError()).isTrue();
     tester.assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
   }
 
@@ -613,8 +612,8 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("base/BUILD",
         "cc_library(name = 'hello', srcs = ['hello.cc'])");
     tester.useLoadingOptions("--compile_one_dependency");
-    LoadingResult loadingResult = assertNoErrors(tester.load("base/hello.cc"));
-    assertThat(loadingResult.getTargets()).containsExactlyElementsIn(getTargets("//base:hello"));
+    TargetPatternPhaseValue result = assertNoErrors(tester.load("base/hello.cc"));
+    assertThat(result.getTargetLabels()).containsExactlyElementsIn(getLabels("//base:hello"));
   }
 
   @Test
@@ -623,8 +622,8 @@ public class LoadingPhaseRunnerTest {
         "cc_library(name = 'hello', srcs = ['hello.cc', '//bad:bad.cc'])");
     tester.useLoadingOptions("--compile_one_dependency");
     try {
-      LoadingResult loadingResult = tester.load("base/hello.cc");
-      assertThat(loadingResult.hasLoadingError()).isFalse();
+      TargetPatternPhaseValue loadingResult = tester.load("base/hello.cc");
+      assertThat(loadingResult.hasPostExpansionError()).isFalse();
     } catch (LoadingFailedException expected) {
       tester.assertContainsError("no such package 'bad'");
     }
@@ -635,8 +634,8 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("base/BUILD",
         "cc_library(name = 'hello', srcs = ['hello.cc', '//bad:bad.cc'])");
     tester.useLoadingOptions("--compile_one_dependency");
-    LoadingResult loadingResult = tester.loadKeepGoing("base/hello.cc");
-    assertThat(loadingResult.hasLoadingError()).isFalse();
+    TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("base/hello.cc");
+    assertThat(loadingResult.hasPostExpansionError()).isFalse();
   }
 
   @Test
@@ -655,8 +654,8 @@ public class LoadingPhaseRunnerTest {
 
   @Test
   public void testParsingFailureReported() throws Exception {
-    LoadingResult loadingResult = tester.loadKeepGoing("//does_not_exist");
-    assertThat(loadingResult.hasTargetPatternError()).isTrue();
+    TargetPatternPhaseValue loadingResult = tester.loadKeepGoing("//does_not_exist");
+    assertThat(loadingResult.hasError()).isTrue();
     ParsingFailedEvent event = tester.findPostOnce(ParsingFailedEvent.class);
     assertThat(event.getPattern()).isEqualTo("//does_not_exist");
     assertThat(event.getMessage()).contains("BUILD file not found on package path");
@@ -668,7 +667,7 @@ public class LoadingPhaseRunnerTest {
     tester.addFile("test/cycle1.bzl", "load(':cycle2.bzl', 'make_cycle')");
     tester.addFile("test/cycle2.bzl", "load(':cycle1.bzl', 'make_cycle')");
     // The skyframe target pattern evaluator isn't able to provide partial results in the presence
-    // of cycles, so it simply raises an exception rather than returning an empty LoadingResult.
+    // of cycles, so it simply raises an exception rather than returning an empty result.
     try {
       tester.load("//test:cycle1");
       fail();
@@ -790,28 +789,27 @@ public class LoadingPhaseRunnerTest {
       this.options = parser.getOptions(LoadingOptions.class);
     }
 
-    public LoadingResult load(String... patterns) throws Exception {
-      return load(/*keepGoing=*/false, /*determineTests=*/false, patterns);
+    public TargetPatternPhaseValue load(String... patterns) throws Exception {
+      return loadWithFlags(/*keepGoing=*/false, /*determineTests=*/false, patterns);
     }
 
-    public LoadingResult loadKeepGoing(String... patterns) throws Exception {
-      return load(/*keepGoing=*/true, /*determineTests=*/false, patterns);
+    public TargetPatternPhaseValue loadKeepGoing(String... patterns) throws Exception {
+      return loadWithFlags(/*keepGoing=*/true, /*determineTests=*/false, patterns);
     }
 
-    public LoadingResult loadTests(String... patterns) throws Exception {
-      return load(/*keepGoing=*/false, /*determineTests=*/true, patterns);
+    public TargetPatternPhaseValue loadTests(String... patterns) throws Exception {
+      return loadWithFlags(/*keepGoing=*/false, /*determineTests=*/true, patterns);
     }
 
-    public LoadingResult loadTestsKeepGoing(String... patterns) throws Exception {
-      return load(/*keepGoing=*/true, /*determineTests=*/true, patterns);
+    public TargetPatternPhaseValue loadTestsKeepGoing(String... patterns) throws Exception {
+      return loadWithFlags(/*keepGoing=*/true, /*determineTests=*/true, patterns);
     }
 
-    public LoadingResult load(boolean keepGoing, boolean determineTests, String... patterns)
-        throws Exception {
+    public TargetPatternPhaseValue loadWithFlags(
+        boolean keepGoing, boolean determineTests, String... patterns) throws Exception {
       sync();
       storedErrors.clear();
-      LoadingResult result;
-      result =
+      TargetPatternPhaseValue result =
           skyframeExecutor.loadTargetPatterns(
               storedErrors,
               ImmutableList.copyOf(patterns),
@@ -867,14 +865,6 @@ public class LoadingPhaseRunnerTest {
       changes.clear();
     }
 
-    public List<Target> getTargets(String... targetNames) throws Exception {
-      List<Target> result = new ArrayList<>();
-      for (String targetName : targetNames) {
-        result.add(getTarget(targetName));
-      }
-      return result;
-    }
-
     public Target getTarget(String targetName) throws Exception {
       StoredEventHandler eventHandler = new StoredEventHandler();
       Target target = getPkgManager().getTarget(
@@ -887,16 +877,16 @@ public class LoadingPhaseRunnerTest {
       return skyframeExecutor.getPackageManager();
     }
 
-    public ImmutableSet<TargetParsingCompleteEvent.ThinTarget> getFilteredTargets() {
-      return targetParsingCompleteEvent.getFilteredTargets();
+    public ImmutableSet<Label> getFilteredTargets() {
+      return ImmutableSet.copyOf(targetParsingCompleteEvent.getFilteredLabels());
     }
 
-    public ImmutableSet<TargetParsingCompleteEvent.ThinTarget> getTestFilteredTargets() {
-      return targetParsingCompleteEvent.getTestFilteredTargets();
+    public ImmutableSet<Label> getTestFilteredTargets() {
+      return ImmutableSet.copyOf(targetParsingCompleteEvent.getTestFilteredLabels());
     }
 
-    public ImmutableSet<TargetParsingCompleteEvent.ThinTarget> getOriginalTargets() {
-      return targetParsingCompleteEvent.getTargets();
+    public ImmutableSet<Label> getOriginalTargets() {
+      return ImmutableSet.copyOf(targetParsingCompleteEvent.getLabels());
     }
 
     public ImmutableSet<Label> getTestSuiteTargets() {