Expose cc_common.create_compile_build_variables

This cl enabled skylark rules to create build variables used for C++ compile
actions (in a limited form, e.g. build variables for modules are not exposed
etc).

Working towards #4571.

This is an encore of https://github.com/bazelbuild/bazel/commit/c4fc6201fdfa71993e2e9e295a946150e6990c75 after fixing memory regression
(CompileBuildVariables.getSafePathStrings fixed to return ImmutableList, which
has smaller memory footprint).

RELNOTES: None
PiperOrigin-RevId: 196797581

4 files changed, 92 insertions, 74 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index d03dda0348..b595600a57 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -1509,16 +1509,16 @@ public final class CcCompilationHelper {
         ruleContext,
         featureConfiguration,
         ccToolchain,
-        sourceFile,
-        builder.getOutputFile(),
-        gcnoFile,
-        dwoFile,
-        ltoIndexingFile,
+        toPathString(sourceFile),
+        toPathString(builder.getOutputFile()),
+        toPathString(gcnoFile),
+        toPathString(dwoFile),
+        toPathString(ltoIndexingFile),
         ImmutableList.of(),
         userCompileFlags.build(),
         cppModuleMap,
         usePic,
-        builder.getRealOutputFilePath(),
+        builder.getTempOutputFile(),
         CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()),
         dotdFileExecPath,
         ImmutableList.copyOf(variablesExtensions),
@@ -1530,6 +1530,10 @@ public final class CcCompilationHelper {
         ccCompilationContext.getDefines());
   }
 
+  private static String toPathString(Artifact a) {
+    return a == null ? null : a.getExecPathString();
+  }
+
   /**
    * Returns a {@code CppCompileActionBuilder} with the common fields for a C++ compile action being
    * initialized.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
index 2c246f7a8c..6b6247eab1 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
@@ -27,7 +27,6 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.VariablesExt
 import com.google.devtools.build.lib.syntax.EvalException;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import java.util.Collection;
 
 /** Enum covering all build variables we create for all various {@link CppCompileAction}. */
 public enum CompileBuildVariables {
@@ -108,24 +107,24 @@ public enum CompileBuildVariables {
       RuleContext ruleContext,
       FeatureConfiguration featureConfiguration,
       CcToolchainProvider ccToolchainProvider,
-      Artifact sourceFile,
-      Artifact outputFile,
-      Artifact gcnoFile,
-      Artifact dwoFile,
-      Artifact ltoIndexingFile,
+      String sourceFile,
+      String outputFile,
+      String gcnoFile,
+      String dwoFile,
+      String ltoIndexingFile,
       ImmutableList<String> includes,
-      ImmutableList<String> userCompileFlags,
+      Iterable<String> userCompileFlags,
       CppModuleMap cppModuleMap,
       boolean usePic,
-      PathFragment realOutputFilePath,
+      PathFragment fakeOutputFile,
       String fdoStamp,
       String dotdFileExecPath,
       ImmutableList<VariablesExtension> variablesExtensions,
       ImmutableMap<String, String> additionalBuildVariables,
       Iterable<Artifact> directModuleMaps,
-      Collection<PathFragment> includeDirs,
-      Collection<PathFragment> quoteIncludeDirs,
-      Collection<PathFragment> systemIncludeDirs,
+      Iterable<PathFragment> includeDirs,
+      Iterable<PathFragment> quoteIncludeDirs,
+      Iterable<PathFragment> systemIncludeDirs,
       Iterable<String> defines) {
     try {
       return setupVariablesOrThrowEvalException(
@@ -140,16 +139,20 @@ public enum CompileBuildVariables {
           userCompileFlags,
           cppModuleMap,
           usePic,
-          realOutputFilePath,
+          toPathString(fakeOutputFile),
           fdoStamp,
           dotdFileExecPath,
           variablesExtensions,
           additionalBuildVariables,
           directModuleMaps,
-          includeDirs,
-          quoteIncludeDirs,
-          systemIncludeDirs,
-          defines);
+          getSafePathStrings(includeDirs),
+          getSafePathStrings(quoteIncludeDirs),
+          getSafePathStrings(systemIncludeDirs),
+          defines,
+          /* addLegacyCxxOptions= */ CppFileTypes.CPP_SOURCE.matches(sourceFile)
+              || CppFileTypes.CPP_HEADER.matches(sourceFile)
+              || CppFileTypes.CPP_MODULE_MAP.matches(sourceFile)
+              || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFile));
     } catch (EvalException e) {
       ruleContext.ruleError(e.getMessage());
       return CcToolchainVariables.EMPTY;
@@ -159,25 +162,28 @@ public enum CompileBuildVariables {
   public static CcToolchainVariables setupVariablesOrThrowEvalException(
       FeatureConfiguration featureConfiguration,
       CcToolchainProvider ccToolchainProvider,
-      Artifact sourceFile,
-      Artifact outputFile,
-      Artifact gcnoFile,
-      Artifact dwoFile,
-      Artifact ltoIndexingFile,
+      String sourceFile,
+      // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are
+      // updated.
+      String outputFile,
+      String gcnoFile,
+      String dwoFile,
+      String ltoIndexingFile,
       ImmutableList<String> includes,
-      ImmutableList<String> userCompileFlags,
+      Iterable<String> userCompileFlags,
       CppModuleMap cppModuleMap,
       boolean usePic,
-      PathFragment realOutputFilePath,
+      String fakeOutputFile,
       String fdoStamp,
       String dotdFileExecPath,
       ImmutableList<VariablesExtension> variablesExtensions,
       ImmutableMap<String, String> additionalBuildVariables,
       Iterable<Artifact> directModuleMaps,
-      Iterable<PathFragment> includeDirs,
-      Iterable<PathFragment> quoteIncludeDirs,
-      Iterable<PathFragment> systemIncludeDirs,
-      Iterable<String> defines)
+      Iterable<String> includeDirs,
+      Iterable<String> quoteIncludeDirs,
+      Iterable<String> systemIncludeDirs,
+      Iterable<String> defines,
+      boolean addLegacyCxxOptions)
       throws EvalException {
     Preconditions.checkNotNull(directModuleMaps);
     Preconditions.checkNotNull(includeDirs);
@@ -190,34 +196,42 @@ public enum CompileBuildVariables {
     buildVariables.addStringSequenceVariable(
         USER_COMPILE_FLAGS.getVariableName(), userCompileFlags);
 
-    buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile.getExecPathString());
-
-    String sourceFilename = sourceFile.getExecPathString();
     buildVariables.addLazyStringSequenceVariable(
         LEGACY_COMPILE_FLAGS.getVariableName(),
-        getLegacyCompileFlagsSupplier(ccToolchainProvider, sourceFilename));
+        getLegacyCompileFlagsSupplier(ccToolchainProvider, addLegacyCxxOptions));
+
+    if (sourceFile != null) {
+      buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile);
+    }
 
-    if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename)
-        && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) {
+    if (sourceFile == null
+        || (!CppFileTypes.OBJC_SOURCE.matches(sourceFile)
+            && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFile))) {
       buildVariables.addLazyStringSequenceVariable(
           UNFILTERED_COMPILE_FLAGS.getVariableName(),
           getUnfilteredCompileFlagsSupplier(ccToolchainProvider));
     }
 
-    // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are updated.
-    if (!FileType.contains(
-        outputFile,
-        CppFileTypes.ASSEMBLER,
-        CppFileTypes.PIC_ASSEMBLER,
-        CppFileTypes.PREPROCESSED_C,
-        CppFileTypes.PREPROCESSED_CPP,
-        CppFileTypes.PIC_PREPROCESSED_C,
-        CppFileTypes.PIC_PREPROCESSED_CPP)) {
+    String fakeOutputFileOrRealOutputFile = fakeOutputFile != null ? fakeOutputFile : outputFile;
+
+    if (outputFile != null) {
+      // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are
+      // updated.
+      if (!FileType.contains(
+          PathFragment.create(outputFile),
+          CppFileTypes.ASSEMBLER,
+          CppFileTypes.PIC_ASSEMBLER,
+          CppFileTypes.PREPROCESSED_C,
+          CppFileTypes.PREPROCESSED_CPP,
+          CppFileTypes.PIC_PREPROCESSED_C,
+          CppFileTypes.PIC_PREPROCESSED_CPP)) {
+        buildVariables.addStringVariable(
+            OUTPUT_OBJECT_FILE.getVariableName(), fakeOutputFileOrRealOutputFile);
+      }
+
       buildVariables.addStringVariable(
-          OUTPUT_OBJECT_FILE.getVariableName(), realOutputFilePath.getSafePathString());
+          OUTPUT_FILE.getVariableName(), fakeOutputFileOrRealOutputFile);
     }
-    buildVariables.addStringVariable(
-        OUTPUT_FILE.getVariableName(), realOutputFilePath.getSafePathString());
 
     // Set dependency_file to enable <object>.d file generation.
     if (dotdFileExecPath != null) {
@@ -241,12 +255,11 @@ public enum CompileBuildVariables {
       buildVariables.addStringSequenceVariable(MODULE_FILES.getVariableName(), ImmutableSet.of());
     }
     if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) {
+      buildVariables.addStringSequenceVariable(INCLUDE_PATHS.getVariableName(), includeDirs);
       buildVariables.addStringSequenceVariable(
-          INCLUDE_PATHS.getVariableName(), getSafePathStrings(includeDirs));
+          QUOTE_INCLUDE_PATHS.getVariableName(), quoteIncludeDirs);
       buildVariables.addStringSequenceVariable(
-          QUOTE_INCLUDE_PATHS.getVariableName(), getSafePathStrings(quoteIncludeDirs));
-      buildVariables.addStringSequenceVariable(
-          SYSTEM_INCLUDE_PATHS.getVariableName(), getSafePathStrings(systemIncludeDirs));
+          SYSTEM_INCLUDE_PATHS.getVariableName(), systemIncludeDirs);
     }
 
     if (!includes.isEmpty()) {
@@ -277,18 +290,16 @@ public enum CompileBuildVariables {
     }
 
     if (gcnoFile != null) {
-      buildVariables.addStringVariable(
-          GCOV_GCNO_FILE.getVariableName(), gcnoFile.getExecPathString());
+      buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile);
     }
 
     if (dwoFile != null) {
-      buildVariables.addStringVariable(
-          PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile.getExecPathString());
+      buildVariables.addStringVariable(PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile);
     }
 
     if (ltoIndexingFile != null) {
       buildVariables.addStringVariable(
-          LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile.getExecPathString());
+          LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile);
     }
 
     buildVariables.addAllStringVariables(additionalBuildVariables);
@@ -300,12 +311,13 @@ public enum CompileBuildVariables {
   }
 
   /** Get the safe path strings for a list of paths to use in the build variables. */
-  private static ImmutableSet<String> getSafePathStrings(Iterable<PathFragment> paths) {
-    ImmutableSet.Builder<String> result = ImmutableSet.builder();
-    for (PathFragment path : paths) {
-      result.add(path.getSafePathString());
-    }
-    return result.build();
+  private static ImmutableList<String> getSafePathStrings(Iterable<PathFragment> paths) {
+    // Using ImmutableSet first to remove duplicates, then ImmutableList for smaller memory
+    // footprint.
+    return ImmutableSet.copyOf(paths)
+        .stream()
+        .map(PathFragment::getSafePathString)
+        .collect(ImmutableList.toImmutableList());
   }
 
   /**
@@ -315,14 +327,11 @@ public enum CompileBuildVariables {
    * to arguments (to prevent accidental capture of enclosing instance which could regress memory).
    */
   private static Supplier<ImmutableList<String>> getLegacyCompileFlagsSupplier(
-      CcToolchainProvider toolchain, String sourceFilename) {
+      CcToolchainProvider toolchain, boolean addLegacyCxxOptions) {
     return () -> {
       ImmutableList.Builder<String> legacyCompileFlags = ImmutableList.builder();
       legacyCompileFlags.addAll(toolchain.getLegacyCompileOptions());
-      if (CppFileTypes.CPP_SOURCE.matches(sourceFilename)
-          || CppFileTypes.CPP_HEADER.matches(sourceFilename)
-          || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename)
-          || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) {
+      if (addLegacyCxxOptions) {
         legacyCompileFlags.addAll(toolchain.getLegacyCxxOptions());
       }
       return legacyCompileFlags.build();
@@ -340,6 +349,10 @@ public enum CompileBuildVariables {
     return () -> ccToolchain.getUnfilteredCompilerOptions();
   }
 
+  private static String toPathString(PathFragment a) {
+    return a == null ? null : a.getSafePathString();
+  }
+
   public String getVariableName() {
     return variableName;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
index 0339e13877..626f343015 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java
@@ -254,6 +254,7 @@ public class CppActionConfigs {
                     "    action: 'c++-module-codegen'",
                     "    action: 'c++-module-compile'",
                     "    flag_group {",
+                    "      expand_if_all_available: 'output_file'",
                     "      flag: '-frandom-seed=%{output_file}'",
                     "    }",
                     "  }",
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
index 7f92471e30..67d020f8ef 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
@@ -146,8 +146,8 @@ public class CppLinkstampCompileHelper {
         ruleContext,
         featureConfiguration,
         ccToolchainProvider,
-        sourceFile,
-        outputFile,
+        sourceFile.getExecPathString(),
+        outputFile.getExecPathString(),
         /* gcnoFile= */ null,
         /* dwoFile= */ null,
         /* ltoIndexingFile= */ null,
@@ -158,7 +158,7 @@ public class CppLinkstampCompileHelper {
         CcCompilationHelper.getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()),
         /* cppModuleMap= */ null,
         needsPic,
-        outputFile.getExecPath(),
+        /* fakeOutputFile= */ null,
         fdoBuildStamp,
         /* dotdFileExecPath= */ null,
         /* variablesExtensions= */ ImmutableList.of(),