Migrate several more callers off of getOrderedValuesAndExceptions.

PiperOrigin-RevId: 481233853
Change-Id: Id5be191ab30759d839b9c892157af4a440bea994

src/main/java/com/google/devtools/build/lib/analysis/config/ConfigurationResolver.java

@@ -47,7 +47,7 @@
 import com.google.devtools.build.lib.util.OrderedSetMultimap;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
-import com.google.devtools.build.skyframe.SkyframeIterableResult;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -152,8 +152,8 @@ public OrderedSetMultimap<DependencyKind, Dependency> resolveConfigurations(
       if (depConfig == null) {
         // Instead of returning immediately, give the loop a chance to queue up every missing
         // dependency, then return all at once. That prevents re-executing this code an unnecessary
-        // number of times. i.e. this is equivalent to calling env.getOrderedValuesAndExceptions()
-        // once over all deps.
+        // number of times. i.e. this is equivalent to calling env.getValuesAndExceptions() once
+        // over all deps.
         needConfigsFromSkyframe = true;
       } else {
         resolvedDeps.putAll(dependencyKind, depConfig);
@@ -272,16 +272,15 @@ private ImmutableList<Dependency> resolveGenericTransition(
       throw new ConfiguredValueCreationException(ctgValue, e.getMessage());
     }
 
-    SkyframeIterableResult depConfigValues =
-        env.getOrderedValuesAndExceptions(configurationKeys.values());
+    SkyframeLookupResult depConfigValues = env.getValuesAndExceptions(configurationKeys.values());
     List<Dependency> dependencies = new ArrayList<>();
     try {
       for (Map.Entry<String, BuildConfigurationKey> entry : configurationKeys.entrySet()) {
         String transitionKey = entry.getKey();
         // TODO(blaze-configurability-team): Should be able to just use BuildConfigurationKey
         BuildConfigurationValue configuration =
             (BuildConfigurationValue)
-                depConfigValues.nextOrThrow(InvalidConfigurationException.class);
+                depConfigValues.getOrThrow(entry.getValue(), InvalidConfigurationException.class);
         if (configuration == null) {
           continue;
         }

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java

@@ -20,7 +20,7 @@
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyframeIterableResult;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import java.util.ArrayDeque;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -61,10 +61,10 @@ public static ImmutableMap<ModuleKey, Module> run(Environment env, RootModuleFil
           unexpandedSkyKeys.add(ModuleFileValue.key(depKey, overrides.get(depKey.getName())));
         }
       }
-      SkyframeIterableResult result = env.getOrderedValuesAndExceptions(unexpandedSkyKeys);
+      SkyframeLookupResult result = env.getValuesAndExceptions(unexpandedSkyKeys);
       for (SkyKey skyKey : unexpandedSkyKeys) {
         ModuleKey depKey = ((ModuleFileValue.Key) skyKey).getModuleKey();
-        ModuleFileValue moduleFileValue = (ModuleFileValue) result.next();
+        ModuleFileValue moduleFileValue = (ModuleFileValue) result.get(skyKey);
         if (moduleFileValue == null) {
           // Don't return yet. Try to expand any other unexpanded nodes before returning.
           depGraph.put(depKey, null);

src/main/java/com/google/devtools/build/lib/includescanning/IncludeParser.java

@@ -56,7 +56,7 @@
 import com.google.devtools.build.lib.vfs.UnixGlob;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyValue;
-import com.google.devtools.build.skyframe.SkyframeIterableResult;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -305,8 +305,7 @@ ImmutableSet<Artifact> getPathLevelHintedInclusions(
             ContainingPackageLookupValue.key(PackageIdentifier.createInMainRepo(relativePath)));
         findFilters.add(rule.findFilter);
       }
-      SkyframeIterableResult containingPackageLookupValues =
-          env.getOrderedValuesAndExceptions(rulePaths);
+      SkyframeLookupResult containingPackageLookupValues = env.getValuesAndExceptions(rulePaths);
       if (env.valuesMissing()
           && !env.inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors()) {
         return null;
@@ -319,7 +318,8 @@ ImmutableSet<Artifact> getPathLevelHintedInclusions(
         try {
           containingPackageLookupValue =
               (ContainingPackageLookupValue)
-                  containingPackageLookupValues.nextOrThrow(NoSuchPackageException.class);
+                  containingPackageLookupValues.getOrThrow(
+                      relativePathKey, NoSuchPackageException.class);
         } catch (NoSuchPackageException e) {
           if (env.inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors()) {
             throw e;
@@ -353,20 +353,19 @@ ImmutableSet<Artifact> getPathLevelHintedInclusions(
       if (env.valuesMissing()) {
         return null;
       }
-      SkyframeIterableResult globResults = env.getOrderedValuesAndExceptions(globKeys);
+      SkyframeLookupResult globResults = env.getValuesAndExceptions(globKeys);
       if (env.valuesMissing()
           && !env.inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors()) {
         return null;
       }
-      int i = 0;
-      while (globResults.hasNext()) {
-        GlobDescriptor globKey = globKeys.get(i++);
+      for (GlobDescriptor globKey : globKeys) {
         PathFragment packageFragment = globKey.getPackageId().getPackageFragment();
         GlobValue globValue;
         try {
           globValue =
               (GlobValue)
-                  globResults.nextOrThrow(IOException.class, BuildFileNotFoundException.class);
+                  globResults.getOrThrow(
+                      globKey, IOException.class, BuildFileNotFoundException.class);
         } catch (IOException | BuildFileNotFoundException e) {
           if (env.inErrorBubblingForSkyFunctionsThatCanFullyRecoverFromErrors()) {
             throw e;

src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java

@@ -46,7 +46,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyValue;
-import com.google.devtools.build.skyframe.SkyframeIterableResult;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import java.util.AbstractCollection;
 import java.util.ArrayList;
@@ -392,25 +392,26 @@ public static boolean handleTreeArtifacts(
         Map<PathFragment, Artifact> pathToLegalArtifact,
         ArrayList<Artifact> treeArtifacts)
         throws InterruptedException {
-      if (!treeArtifacts.isEmpty()) {
-        SkyframeIterableResult result = env.getOrderedValuesAndExceptions(treeArtifacts);
-        if (env.valuesMissing()) {
+      if (treeArtifacts.isEmpty()) {
+        return true;
+      }
+      SkyframeLookupResult result = env.getValuesAndExceptions(treeArtifacts);
+      if (env.valuesMissing()) {
+        return false;
+      }
+      for (Artifact treeArtifact : treeArtifacts) {
+        SkyValue value = result.get(treeArtifact);
+        if (value == null) {
+          BugReport.sendBugReport(
+              new IllegalStateException(
+                  "Some value from " + treeArtifacts + " was missing, this should never happen"));
           return false;
         }
-        while (result.hasNext()) {
-          SkyValue value = result.next();
-          if (value == null) {
-            BugReport.sendBugReport(
-                new IllegalStateException(
-                    "Some value from " + treeArtifacts + " was missing, this should never happen"));
-            return false;
-          }
-          checkState(
-              value instanceof TreeArtifactValue, "SkyValue %s is not TreeArtifactValue", value);
-          TreeArtifactValue treeArtifactValue = (TreeArtifactValue) value;
-          for (TreeFileArtifact treeFileArtifact : treeArtifactValue.getChildren()) {
-            pathToLegalArtifact.put(treeFileArtifact.getExecPath(), treeFileArtifact);
-          }
+        checkState(
+            value instanceof TreeArtifactValue, "SkyValue %s is not TreeArtifactValue", value);
+        TreeArtifactValue treeArtifactValue = (TreeArtifactValue) value;
+        for (TreeFileArtifact treeFileArtifact : treeArtifactValue.getChildren()) {
+          pathToLegalArtifact.put(treeFileArtifact.getExecPath(), treeFileArtifact);
         }
       }
       return true;
@@ -531,7 +532,7 @@ public Set<DerivedArtifact> computeUsedModules(
     return modules;
   }
 
-  private void removeArtifactsFromSet(Set<Artifact> set, ImmutableList<Artifact> artifacts) {
+  private static void removeArtifactsFromSet(Set<Artifact> set, ImmutableList<Artifact> artifacts) {
     // Not using iterators here as the resulting overhead is significant in profiles. Do not use
     // Iterables.removeAll() or Set.removeAll() here as with the given container sizes, that
     // needlessly deteriorates to a quadratic algorithm.

src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java

@@ -17,7 +17,6 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.common.flogger.GoogleLogger;
@@ -90,7 +89,7 @@
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyKey;
-import com.google.devtools.build.skyframe.SkyframeIterableResult;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
 import com.google.devtools.common.options.TriState;
@@ -217,12 +216,12 @@ public ConfiguredTarget create(RuleContext ruleContext)
    * DO NOT USE! We should get rid of this method: errors reported directly to this object don't set
    * the error flag in {@link ConfiguredTarget}.
    */
-  private ExtendedEventHandler getEventHandler(RuleContext ruleContext) {
+  private static ExtendedEventHandler getEventHandler(RuleContext ruleContext) {
     return ruleContext.getAnalysisEnvironment().getEventHandler();
   }
 
   @Nullable
-  private GenQueryResult executeQuery(
+  private static GenQueryResult executeQuery(
       RuleContext ruleContext,
       QueryOptions queryOptions,
       ImmutableList<Label> scope,
@@ -256,7 +255,7 @@ private GenQueryResult executeQuery(
   }
 
   @Nullable
-  private GenQueryResult doQuery(
+  private static GenQueryResult doQuery(
       QueryOptions queryOptions,
       GenQueryPackageProvider packageProvider,
       TargetPatternPreloader preloader,
@@ -454,17 +453,16 @@ public Map<String, Collection<Target>> preloadTargetPatterns(
       Map<String, ResolvedTargets<Label>> resolvedLabelsMap =
           Maps.newHashMapWithExpectedSize(patterns.size());
       synchronized (this) {
-        ImmutableSet<TargetPatternKey> targetPatternKeys = patternKeys.keySet();
-        SkyframeIterableResult patternKeysResult =
-            env.getOrderedValuesAndExceptions(targetPatternKeys);
-        for (TargetPatternKey targetPatternKey : targetPatternKeys) {
+        SkyframeLookupResult patternKeysResult = env.getValuesAndExceptions(patternKeys.keySet());
+        for (Map.Entry<TargetPatternKey, String> entry : patternKeys.entrySet()) {
           TargetPatternValue patternValue =
-              (TargetPatternValue) patternKeysResult.nextOrThrow(TargetParsingException.class);
+              (TargetPatternValue)
+                  patternKeysResult.getOrThrow(entry.getKey(), TargetParsingException.class);
           if (patternValue == null) {
             ok = false;
           } else {
             ResolvedTargets<Label> resolvedLabels = patternValue.getTargets();
-            resolvedLabelsMap.put(patternKeys.get(targetPatternKey), resolvedLabels);
+            resolvedLabelsMap.put(entry.getValue(), resolvedLabels);
             for (Label label :
                 Iterables.concat(
                     resolvedLabels.getTargets(), resolvedLabels.getFilteredTargets())) {
@@ -479,14 +477,14 @@ public Map<String, Collection<Target>> preloadTargetPatterns(
       Map<PackageIdentifier, Package> packages =
           Maps.newHashMapWithExpectedSize(packageKeys.size());
       synchronized (this) {
-        SkyframeIterableResult packageKeysResult = env.getOrderedValuesAndExceptions(packageKeys);
+        SkyframeLookupResult packageKeysResult = env.getValuesAndExceptions(packageKeys);
         // packageKeys is not mutated, the iteration order is the same.
         for (SkyKey depKey : packageKeys) {
           PackageIdentifier pkgName = (PackageIdentifier) depKey.argument();
           Package pkg;
           try {
             PackageValue packageValue =
-                (PackageValue) packageKeysResult.nextOrThrow(NoSuchPackageException.class);
+                (PackageValue) packageKeysResult.getOrThrow(depKey, NoSuchPackageException.class);
             if (packageValue == null) {
               ok = false;
               continue;
@@ -514,7 +512,7 @@ public Map<String, Collection<Target>> preloadTargetPatterns(
       return preloadedPatterns;
     }
 
-    private void checkValidPatternType(String pattern, TargetPattern.Parser parser)
+    private static void checkValidPatternType(String pattern, TargetPattern.Parser parser)
         throws TargetParsingException {
       TargetPattern.Type type = parser.parse(pattern).getType();
       if (type == TargetPattern.Type.PATH_AS_TARGET) {

src/main/java/com/google/devtools/build/lib/rules/genquery/GenQueryTtvPackageProviderFactory.java

@@ -32,7 +32,7 @@
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
-import com.google.devtools.build.skyframe.SkyframeIterableResult;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import java.util.Collection;
 import java.util.Set;
 import javax.annotation.Nullable;
@@ -59,13 +59,12 @@ public GenQueryPackageProvider constructPackageMap(Environment env, ImmutableLis
     Set<SkyKey> successfulPackageKeys = Sets.newHashSetWithExpectedSize(scope.size());
     Collection<SkyKey> transitiveTargetKeys =
         Collections2.transform(scope, TransitiveTargetKey::of);
-    SkyframeIterableResult transitiveTargetValues =
-        env.getOrderedValuesAndExceptions(transitiveTargetKeys);
+    SkyframeLookupResult transitiveTargetValues = env.getValuesAndExceptions(transitiveTargetKeys);
     if (env.valuesMissing()) {
       return null;
     }
     for (SkyKey skyKey : transitiveTargetKeys) {
-      SkyValue value = transitiveTargetValues.next();
+      SkyValue value = transitiveTargetValues.get(skyKey);
       if (value == null) {
         BugReport.logUnexpected("Value for: '%s' was missing, this should never happen", skyKey);
         return null;
@@ -84,16 +83,15 @@ public GenQueryPackageProvider constructPackageMap(Environment env, ImmutableLis
     }
 
     // Construct the package id to package map for all successful packages.
-    SkyframeIterableResult transitivePackages =
-        env.getOrderedValuesAndExceptions(successfulPackageKeys);
+    SkyframeLookupResult transitivePackages = env.getValuesAndExceptions(successfulPackageKeys);
     if (env.valuesMissing()) {
       // Packages from an untaken select branch could be missing: analysis avoids these, but query
       // does not.
       return null;
     }
     ImmutableMap.Builder<PackageIdentifier, Package> packageMapBuilder = ImmutableMap.builder();
     for (SkyKey skyKey : successfulPackageKeys) {
-      PackageValue pkg = (PackageValue) transitivePackages.next();
+      PackageValue pkg = (PackageValue) transitivePackages.get(skyKey);
       if (pkg == null) {
         BugReport.sendBugReport(
             new IllegalStateException(

src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryFunction.java

@@ -34,7 +34,7 @@
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
 import com.google.devtools.build.skyframe.SkyValue;
-import com.google.devtools.build.skyframe.SkyframeIterableResult;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import java.io.IOException;
 import java.util.Map;
 import javax.annotation.Nullable;
@@ -137,14 +137,14 @@ public RepositoryDirectoryValue.Builder fetch(
             e ->
                 FileValue.key(
                     RootedPath.toRootedPath(root, rootRelativePath.getRelative(e.getName()))));
-    SkyframeIterableResult fileValuesResult = env.getOrderedValuesAndExceptions(skyKeys);
+    SkyframeLookupResult fileValuesResult = env.getValuesAndExceptions(skyKeys);
     if (env.valuesMissing()) {
       return null;
     }
     ImmutableMap.Builder<SkyKey, SkyValue> fileValues =
         ImmutableMap.builderWithExpectedSize(directoryValue.getDirents().size());
     for (SkyKey skyKey : skyKeys) {
-      SkyValue value = fileValuesResult.next();
+      SkyValue value = fileValuesResult.get(skyKey);
       if (value == null) {
         BugReport.sendBugReport(
             new IllegalStateException(
@@ -187,9 +187,9 @@ private static ResolvedEvent resolve(Rule rule, BlazeDirectories directories) {
             .append(Starlark.repr(pathObj));
 
     Object buildFileObj = rule.getAttr("build_file");
-    if ((buildFileObj instanceof String) && ((String) buildFileObj).length() > 0) {
-      // Build fiels might refer to an embedded file (as they to for "local_jdk"),
-      // so we have to describe the argument in a portable way.
+    if ((buildFileObj instanceof String) && !((String) buildFileObj).isEmpty()) {
+      // Build files might refer to an embedded file (as they to for "local_jdk"), so we have to
+      // describe the argument in a portable way.
       origAttr.put("build_file", buildFileObj);
       String buildFileArg;
       PathFragment pathFragment = PathFragment.create((String) buildFileObj);
@@ -199,7 +199,7 @@ private static ResolvedEvent resolve(Rule rule, BlazeDirectories directories) {
             "__embedded_dir__ + \"/\" + "
                 + Starlark.repr(pathFragment.relativeTo(embeddedDir).toString());
       } else {
-        buildFileArg = Starlark.repr(buildFileObj.toString()).toString();
+        buildFileArg = Starlark.repr(buildFileObj.toString());
       }
       repr.append(", build_file = ").append(buildFileArg);
     } else {
@@ -211,7 +211,7 @@ private static ResolvedEvent resolve(Rule rule, BlazeDirectories directories) {
     }
 
     String nativeCommand = repr.append(")").toString();
-    ImmutableMap<String, Object> orig = origAttr.build();
+    ImmutableMap<String, Object> orig = origAttr.buildOrThrow();
 
     return new ResolvedEvent() {
       @Override
@@ -225,7 +225,7 @@ public Object getResolvedInformation(XattrProvider xattrProvider) {
             .put(ResolvedHashesFunction.ORIGINAL_RULE_CLASS, "new_local_repository")
             .put(ResolvedHashesFunction.ORIGINAL_ATTRIBUTES, orig)
             .put(ResolvedHashesFunction.NATIVE, nativeCommand)
-            .build();
+            .buildOrThrow();
       }
     };
   }