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
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 d03dda0..b595600 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 @@ 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 @@ 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 2c246f7..6b6247e 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.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 @@ 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 @@ 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 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 @@ 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 (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename) - && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) { + if (sourceFile != null) { + buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile); + } + + 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 @@ 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 @@ } 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 @@ } /** 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 @@ * 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 @@ 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 0339e13..626f343 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 @@ " 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 7f92471..67d020f 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 @@ ruleContext, featureConfiguration, ccToolchainProvider, - sourceFile, - outputFile, + sourceFile.getExecPathString(), + outputFile.getExecPathString(), /* gcnoFile= */ null, /* dwoFile= */ null, /* ltoIndexingFile= */ null, @@ -158,7 +158,7 @@ CcCompilationHelper.getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()), /* cppModuleMap= */ null, needsPic, - outputFile.getExecPath(), + /* fakeOutputFile= */ null, fdoBuildStamp, /* dotdFileExecPath= */ null, /* variablesExtensions= */ ImmutableList.of(),