C++: avoid implicit iteration over NestedSet

This makes the C++ rules compile with NestedSet not implementing
Iterable.

PiperOrigin-RevId: 289404247

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

@@ -562,6 +562,15 @@ private void setCoptsFilter(CoptsFilter coptsFilter) {
     this.coptsFilter = Preconditions.checkNotNull(coptsFilter);
   }
 
+  /**
+   * Adds the given defines to the compiler command line of this target as well as its dependent
+   * targets.
+   */
+  public CcCompilationHelper addDefines(NestedSet<String> defines) {
+    this.defines.addAll(defines.toList());
+    return this;
+  }
+
   /**
    * Adds the given defines to the compiler command line of this target as well as its dependent
    * targets.
@@ -595,6 +604,15 @@ private void setLooseIncludeDirs(Set<PathFragment> looseIncludeDirs) {
     this.looseIncludeDirs = looseIncludeDirs;
   }
 
+  /**
+   * Adds the given directories to the system include directories (they are passed with {@code
+   * "-isystem"} to the compiler); these are also passed to dependent rules.
+   */
+  public CcCompilationHelper addSystemIncludeDirs(NestedSet<PathFragment> systemIncludeDirs) {
+    this.systemIncludeDirs.addAll(systemIncludeDirs.toList());
+    return this;
+  }
+
   /**
    * Adds the given directories to the system include directories (they are passed with {@code
    * "-isystem"} to the compiler); these are also passed to dependent rules.
@@ -604,6 +622,15 @@ public CcCompilationHelper addSystemIncludeDirs(Iterable<PathFragment> systemInc
     return this;
   }
 
+  /**
+   * Adds the given directories to the quote include directories (they are passed with {@code
+   * "-iquote"} to the compiler); these are also passed to dependent rules.
+   */
+  public CcCompilationHelper addQuoteIncludeDirs(NestedSet<PathFragment> quoteIncludeDirs) {
+    this.quoteIncludeDirs.addAll(quoteIncludeDirs.toList());
+    return this;
+  }
+
   /**
    * Adds the given directories to the quote include directories (they are passed with {@code
    * "-iquote"} to the compiler); these are also passed to dependent rules.
@@ -613,6 +640,15 @@ public CcCompilationHelper addQuoteIncludeDirs(Iterable<PathFragment> quoteInclu
     return this;
   }
 
+  /**
+   * Adds the given directories to the include directories (they are passed with {@code "-I"} to the
+   * compiler); these are also passed to dependent rules.
+   */
+  public CcCompilationHelper addIncludeDirs(NestedSet<PathFragment> includeDirs) {
+    this.includeDirs.addAll(includeDirs.toList());
+    return this;
+  }
+
   /**
    * Adds the given directories to the include directories (they are passed with {@code "-I"} to the
    * compiler); these are also passed to dependent rules.

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

@@ -965,6 +965,66 @@ public int hashCode() {
     }
   }
 
+  /**
+   * A sequence of simple string values. Exists as a memory optimization - a typical build can
+   * contain millions of feature values, so getting rid of the overhead of {@code StringValue}
+   * objects significantly reduces memory overhead.
+   */
+  @Immutable
+  @AutoCodec
+  static final class StringSetSequence extends VariableValueAdapter {
+    private final NestedSet<String> values;
+    private int hash = 0;
+
+    public StringSetSequence(NestedSet<String> values) {
+      Preconditions.checkNotNull(values);
+      this.values = values;
+    }
+
+    @Override
+    public Iterable<? extends VariableValue> getSequenceValue(String variableName) {
+      final ImmutableList.Builder<VariableValue> sequences = ImmutableList.builder();
+      for (String value : values.toList()) {
+        sequences.add(new StringValue(value));
+      }
+      return sequences.build();
+    }
+
+    @Override
+    public String getVariableTypeName() {
+      return Sequence.SEQUENCE_VARIABLE_TYPE_NAME;
+    }
+
+    @Override
+    public boolean isTruthy() {
+      return !values.isEmpty();
+    }
+
+    @Override
+    public boolean equals(Object other) {
+      if (!(other instanceof StringSetSequence)) {
+        return false;
+      }
+      if (this == other) {
+        return true;
+      }
+      return values.equals(((StringSetSequence) other).values);
+    }
+
+    @Override
+    public int hashCode() {
+      int h = hash;
+      if (h == 0) {
+        h = 1;
+        for (String s : values.toList()) {
+          h = 31 * h + (s == null ? 0 : s.hashCode());
+        }
+        hash = h;
+      }
+      return h;
+    }
+  }
+
   /**
    * Single structure value. Be careful not to create sequences of single structures, as the memory
    * overhead is prohibitively big. Use optimized {@link StructureSequence} instead.
@@ -1177,7 +1237,7 @@ public Builder addStringSequenceVariable(String name, ImmutableSet<String> value
     public Builder addStringSequenceVariable(String name, NestedSet<String> values) {
       checkVariableNotPresentAlready(name);
       Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name);
-      variablesMap.put(name, new StringSequence(values));
+      variablesMap.put(name, new StringSetSequence(values));
       return this;
     }
 

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

@@ -360,10 +360,10 @@ public static void setupCommonVariables(
       List<VariablesExtension> variablesExtensions,
       Map<String, String> additionalBuildVariables,
       Iterable<Artifact> directModuleMaps,
-      Iterable<PathFragment> includeDirs,
-      Iterable<PathFragment> quoteIncludeDirs,
-      Iterable<PathFragment> systemIncludeDirs,
-      Iterable<PathFragment> frameworkIncludeDirs,
+      List<PathFragment> includeDirs,
+      List<PathFragment> quoteIncludeDirs,
+      List<PathFragment> systemIncludeDirs,
+      List<PathFragment> frameworkIncludeDirs,
       NestedSet<String> defines,
       Iterable<String> localDefines) {
     setupCommonVariablesInternal(
@@ -461,7 +461,12 @@ private static void setupCommonVariablesInternal(
   }
 
   /** Get the safe path strings for a list of paths to use in the build variables. */
-  private static NestedSet<String> getSafePathStrings(Iterable<PathFragment> paths) {
+  private static NestedSet<String> getSafePathStrings(NestedSet<PathFragment> paths) {
+    return getSafePathStrings(paths.toList());
+  }
+
+  /** Get the safe path strings for a list of paths to use in the build variables. */
+  private static NestedSet<String> getSafePathStrings(List<PathFragment> paths) {
     // Using ImmutableSet first to remove duplicates, then NestedSet for smaller memory footprint.
     return NestedSetBuilder.wrap(
         Order.STABLE_ORDER,

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

@@ -859,7 +859,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
 
     NestedSet<Artifact> expandedLinkerArtifacts =
         getArtifactsPossiblyLtoMapped(
-            collectedLibrariesToLink.getExpandedLinkerInputs(), ltoMapping);
+            collectedLibrariesToLink.getExpandedLinkerInputs().toList(), ltoMapping);
 
     CcToolchainVariables variables;
     try {
@@ -1065,7 +1065,7 @@ public CppLinkAction build() throws InterruptedException, RuleErrorException {
     inputsBuilder.addTransitive(linkstampObjectArtifacts);
 
     ImmutableSet<Artifact> fakeLinkerInputArtifacts =
-        collectedLibrariesToLink.getExpandedLinkerInputs().stream()
+        collectedLibrariesToLink.getExpandedLinkerInputs().toList().stream()
             .filter(LinkerInput::isFake)
             .map(LinkerInput::getArtifact)
             .collect(ImmutableSet.toImmutableSet());

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

@@ -19,6 +19,8 @@
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.LibraryToLinkValue;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.SequenceBuilder;
@@ -115,15 +117,15 @@ public LibrariesToLinkCollector(
    */
   public static class CollectedLibrariesToLink {
     private final SequenceBuilder librariesToLink;
-    private final ImmutableSet<LinkerInput> expandedLinkerInputs;
-    private final ImmutableSet<String> librarySearchDirectories;
-    private final ImmutableSet<String> runtimeLibrarySearchDirectories;
+    private final NestedSet<LinkerInput> expandedLinkerInputs;
+    private final NestedSet<String> librarySearchDirectories;
+    private final NestedSet<String> runtimeLibrarySearchDirectories;
 
     private CollectedLibrariesToLink(
         SequenceBuilder librariesToLink,
-        ImmutableSet<LinkerInput> expandedLinkerInputs,
-        ImmutableSet<String> librarySearchDirectories,
-        ImmutableSet<String> runtimeLibrarySearchDirectories) {
+        NestedSet<LinkerInput> expandedLinkerInputs,
+        NestedSet<String> librarySearchDirectories,
+        NestedSet<String> runtimeLibrarySearchDirectories) {
       this.librariesToLink = librariesToLink;
       this.expandedLinkerInputs = expandedLinkerInputs;
       this.librarySearchDirectories = librarySearchDirectories;
@@ -135,15 +137,15 @@ public SequenceBuilder getLibrariesToLink() {
     }
 
     // TODO(b/78347840): Figure out how to make these Artifacts.
-    public ImmutableSet<LinkerInput> getExpandedLinkerInputs() {
+    public NestedSet<LinkerInput> getExpandedLinkerInputs() {
       return expandedLinkerInputs;
     }
 
-    public ImmutableSet<String> getLibrarySearchDirectories() {
+    public NestedSet<String> getLibrarySearchDirectories() {
       return librarySearchDirectories;
     }
 
-    public ImmutableSet<String> getRuntimeLibrarySearchDirectories() {
+    public NestedSet<String> getRuntimeLibrarySearchDirectories() {
       return runtimeLibrarySearchDirectories;
     }
   }
@@ -158,10 +160,10 @@ public ImmutableSet<String> getRuntimeLibrarySearchDirectories() {
    * <p>TODO: Factor out of the bazel binary into build variables for crosstool action_configs.
    */
   public CollectedLibrariesToLink collectLibrariesToLink() {
-    ImmutableSet.Builder<String> librarySearchDirectories = ImmutableSet.builder();
-    ImmutableSet.Builder<String> runtimeLibrarySearchDirectories = ImmutableSet.builder();
+    NestedSetBuilder<String> librarySearchDirectories = NestedSetBuilder.linkOrder();
+    NestedSetBuilder<String> runtimeLibrarySearchDirectories = NestedSetBuilder.linkOrder();
     ImmutableSet.Builder<String> rpathRootsForExplicitSoDeps = ImmutableSet.builder();
-    ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder = ImmutableSet.builder();
+    NestedSetBuilder<LinkerInput> expandedLinkerInputsBuilder = NestedSetBuilder.linkOrder();
     // List of command line parameters that need to be placed *outside* of
     // --whole-archive ... --no-whole-archive.
     SequenceBuilder librariesToLink = new SequenceBuilder();
@@ -210,14 +212,14 @@ public CollectedLibrariesToLink collectLibrariesToLink() {
     Preconditions.checkState(
         ltoMap == null || ltoMap.isEmpty(), "Still have LTO objects left: %s", ltoMap);
 
-    ImmutableSet.Builder<String> allRuntimeLibrarySearchDirectories = ImmutableSet.builder();
+    NestedSetBuilder<String> allRuntimeLibrarySearchDirectories = NestedSetBuilder.linkOrder();
     // rpath ordering matters for performance; first add the one where most libraries are found.
     if (includeSolibDir) {
       allRuntimeLibrarySearchDirectories.add(rpathRoot);
     }
     allRuntimeLibrarySearchDirectories.addAll(rpathRootsForExplicitSoDeps.build());
     if (includeToolchainLibrariesSolibDir) {
-      allRuntimeLibrarySearchDirectories.addAll(runtimeLibrarySearchDirectories.build());
+      allRuntimeLibrarySearchDirectories.addTransitive(runtimeLibrarySearchDirectories.build());
     }
 
     return new CollectedLibrariesToLink(
@@ -228,10 +230,10 @@ public CollectedLibrariesToLink collectLibrariesToLink() {
   }
 
   private Pair<Boolean, Boolean> addLinkerInputs(
-      ImmutableSet.Builder<String> librarySearchDirectories,
+      NestedSetBuilder<String> librarySearchDirectories,
       ImmutableSet.Builder<String> rpathEntries,
       SequenceBuilder librariesToLink,
-      ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder) {
+      NestedSetBuilder<LinkerInput> expandedLinkerInputsBuilder) {
     boolean includeSolibDir = false;
     boolean includeToolchainLibrariesSolibDir = false;
     for (LinkerInput input : linkerInputs) {
@@ -276,8 +278,8 @@ private Pair<Boolean, Boolean> addLinkerInputs(
   private void addDynamicInputLinkOptions(
       LinkerInput input,
       SequenceBuilder librariesToLink,
-      ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder,
-      ImmutableSet.Builder<String> librarySearchDirectories,
+      NestedSetBuilder<LinkerInput> expandedLinkerInputsBuilder,
+      NestedSetBuilder<String> librarySearchDirectories,
       ImmutableSet.Builder<String> rpathRootsForExplicitSoDeps) {
     Preconditions.checkState(
         input.getArtifactCategory() == ArtifactCategory.DYNAMIC_LIBRARY
@@ -359,7 +361,7 @@ private static String effectiveObjectFilePath(Artifact objectFile, boolean input
   private void addStaticInputLinkOptions(
       LinkerInput input,
       SequenceBuilder librariesToLink,
-      ImmutableSet.Builder<LinkerInput> expandedLinkerInputsBuilder) {
+      NestedSetBuilder<LinkerInput> expandedLinkerInputsBuilder) {
     ArtifactCategory artifactCategory = input.getArtifactCategory();
     Preconditions.checkArgument(
         artifactCategory.equals(ArtifactCategory.OBJECT_FILE)

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

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.SequenceBuilder;
 import com.google.devtools.build.lib.syntax.EvalException;
@@ -112,9 +113,9 @@ public static CcToolchainVariables setupVariables(
       PathFragment ltoOutputRootPrefix,
       String defFile,
       FdoContext fdoContext,
-      Iterable<String> runtimeLibrarySearchDirectories,
+      NestedSet<String> runtimeLibrarySearchDirectories,
       SequenceBuilder librariesToLink,
-      Iterable<String> librarySearchDirectories,
+      NestedSet<String> librarySearchDirectories,
       boolean addIfsoRelatedVariables)
       throws EvalException {
     CcToolchainVariables.Builder buildVariables =