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 d03dda03481c4f..b595600a57e866 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 @@ private CcToolchainVariables setupCompileBuildVariables( 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 @@ private CcToolchainVariables setupCompileBuildVariables( 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 2c246f7a8c424d..6b6247eab16d6f 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 @@ public static CcToolchainVariables setupVariablesOrReportRuleError( 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 includes, - ImmutableList userCompileFlags, + Iterable userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, - PathFragment realOutputFilePath, + PathFragment fakeOutputFile, String fdoStamp, String dotdFileExecPath, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, - Collection includeDirs, - Collection quoteIncludeDirs, - Collection systemIncludeDirs, + Iterable includeDirs, + Iterable quoteIncludeDirs, + Iterable systemIncludeDirs, Iterable defines) { try { return setupVariablesOrThrowEvalException( @@ -140,16 +139,20 @@ public static CcToolchainVariables setupVariablesOrReportRuleError( 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 setupVariablesOrReportRuleError( 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 includes, - ImmutableList userCompileFlags, + Iterable userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, - PathFragment realOutputFilePath, + String fakeOutputFile, String fdoStamp, String dotdFileExecPath, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, - Iterable includeDirs, - Iterable quoteIncludeDirs, - Iterable systemIncludeDirs, - Iterable defines) + Iterable includeDirs, + Iterable quoteIncludeDirs, + Iterable systemIncludeDirs, + Iterable defines, + boolean addLegacyCxxOptions) throws EvalException { Preconditions.checkNotNull(directModuleMaps); Preconditions.checkNotNull(includeDirs); @@ -190,34 +196,42 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( 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 .d file generation. if (dotdFileExecPath != null) { @@ -241,12 +255,11 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( 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 static CcToolchainVariables setupVariablesOrThrowEvalException( } 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 static CcToolchainVariables setupVariablesOrThrowEvalException( } /** Get the safe path strings for a list of paths to use in the build variables. */ - private static ImmutableSet getSafePathStrings(Iterable paths) { - ImmutableSet.Builder result = ImmutableSet.builder(); - for (PathFragment path : paths) { - result.add(path.getSafePathString()); - } - return result.build(); + private static ImmutableList getSafePathStrings(Iterable 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 @@ private static ImmutableSet getSafePathStrings(Iterable pa * to arguments (to prevent accidental capture of enclosing instance which could regress memory). */ private static Supplier> getLegacyCompileFlagsSupplier( - CcToolchainProvider toolchain, String sourceFilename) { + CcToolchainProvider toolchain, boolean addLegacyCxxOptions) { return () -> { ImmutableList.Builder 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 @@ private static Supplier> getUnfilteredCompileFlagsSupplier 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 0339e138770395..626f3430154bc6 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 static String getCppActionConfigs( " 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 7f92471e3024ef..67d020f8ef5e93 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 @@ private static CcToolchainVariables getVariables( ruleContext, featureConfiguration, ccToolchainProvider, - sourceFile, - outputFile, + sourceFile.getExecPathString(), + outputFile.getExecPathString(), /* gcnoFile= */ null, /* dwoFile= */ null, /* ltoIndexingFile= */ null, @@ -158,7 +158,7 @@ private static CcToolchainVariables getVariables( CcCompilationHelper.getCoptsFromOptions(cppConfiguration, sourceFile.getExecPathString()), /* cppModuleMap= */ null, needsPic, - outputFile.getExecPath(), + /* fakeOutputFile= */ null, fdoBuildStamp, /* dotdFileExecPath= */ null, /* variablesExtensions= */ ImmutableList.of(),