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 54202e5e0f2b9e..2699b8d949354a 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 @@ -1271,14 +1271,6 @@ private CcToolchainVariables setupCompileBuildVariables( ImmutableMap additionalBuildVariables) throws RuleErrorException, EvalException, InterruptedException { Artifact sourceFile = builder.getSourceFile(); - String dotdFileExecPath = null; - if (builder.getDotdFile() != null) { - dotdFileExecPath = builder.getDotdFile().getExecPathString(); - } - String diagnosticsFileExecPath = null; - if (builder.getDiagnosticsFile() != null) { - diagnosticsFileExecPath = builder.getDiagnosticsFile().getExecPathString(); - } if (needsFdoBuildVariables && fdoContext.hasArtifacts()) { // This modifies the passed-in builder, which is a surprising side-effect, and makes it unsafe // to call this method multiple times for the same builder. @@ -1352,30 +1344,22 @@ private CcToolchainVariables setupCompileBuildVariables( CompileBuildVariables.setupSpecificVariables( buildVariables, - toPathString(sourceFile), - toPathString(builder.getOutputFile()), + sourceFile, + builder.getOutputFile(), enableCoverage, - toPathString(gcnoFile), - toPathString(dwoFile), + gcnoFile, + dwoFile, isUsingFission, - toPathString(ltoIndexingFile), - /* thinLtoIndex= */ null, - /* thinLtoInputBitcodeFile= */ null, - /* thinLtoOutputObjectFile= */ null, + ltoIndexingFile, getCopts(builder.getSourceFile(), sourceLabel), - dotdFileExecPath, - diagnosticsFileExecPath, + builder.getDotdFile(), + builder.getDiagnosticsFile(), usePic, ccCompilationContext.getExternalIncludeDirs(), additionalBuildVariables); return buildVariables.build(); } - @Nullable - 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/CcToolchainVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java index b4c45ce68030c1..1216b5cbd54db0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainVariables.java @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException; import com.google.devtools.build.lib.starlarkbuildapi.cpp.CcToolchainVariablesApi; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.LinkedHashMap; import java.util.List; @@ -1026,7 +1027,8 @@ private StringSequence(Iterable values) { @Override public ImmutableList getSequenceValue(String variableName) { - ImmutableList.Builder sequences = ImmutableList.builder(); + ImmutableList.Builder sequences = + ImmutableList.builderWithExpectedSize(values.size()); for (String value : values) { sequences.add(new StringValue(value)); } @@ -1085,8 +1087,10 @@ private StringSetSequence(NestedSet values) { @Override public ImmutableList getSequenceValue(String variableName) { - ImmutableList.Builder sequences = ImmutableList.builder(); - for (String value : values.toList()) { + List valuesList = values.toList(); + ImmutableList.Builder sequences = + ImmutableList.builderWithExpectedSize(valuesList.size()); + for (String value : valuesList) { sequences.add(new StringValue(value)); } return sequences.build(); @@ -1119,6 +1123,53 @@ public int hashCode() { } } + @Immutable + private static final class PathFragmentSetSequence extends VariableValueAdapter { + private final NestedSet values; + + private PathFragmentSetSequence(NestedSet values) { + Preconditions.checkNotNull(values); + this.values = values; + } + + @Override + public ImmutableList getSequenceValue(String variableName) { + List valuesList = values.toList(); + ImmutableList.Builder sequences = + ImmutableList.builderWithExpectedSize(valuesList.size()); + for (PathFragment value : valuesList) { + sequences.add(new StringValue(value.getSafePathString())); + } + 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 (this == other) { + return true; + } + if (!(other instanceof PathFragmentSetSequence otherPathFragments)) { + return false; + } + return values.shallowEquals(otherPathFragments.values); + } + + @Override + public int hashCode() { + return values.shallowHashCode(); + } + } + /** * Single structure value. Be careful not to create sequences of single structures, as the memory * overhead is prohibitively big. @@ -1171,7 +1222,7 @@ public int hashCode() { } /** - * The leaves in the variable sequence node tree are simple string values. Note that this should + * Most leaves in the variable sequence node tree are simple string values. Note that this should * never live outside of {@code expand}, as the object overhead is prohibitively expensive. */ @Immutable @@ -1247,6 +1298,53 @@ public boolean isTruthy() { } } + /** + * Represents leaves in the variable sequence node tree that are paths of artifacts. Note that + * this should never live outside of {@code expand}, as the object overhead is prohibitively + * expensive. + */ + @Immutable + private static final class ArtifactValue extends VariableValueAdapter { + private static final String ARTIFACT_VARIABLE_TYPE_NAME = "artifact"; + + private final Artifact value; + + ArtifactValue(Artifact value) { + this.value = value; + } + + @Override + public String getStringValue(String variableName) { + return value.getExecPathString(); + } + + @Override + public String getVariableTypeName() { + return ARTIFACT_VARIABLE_TYPE_NAME; + } + + @Override + public boolean isTruthy() { + return true; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof ArtifactValue otherValue)) { + return false; + } + return value.equals(otherValue.value); + } + + @Override + public int hashCode() { + return value.hashCode(); + } + } + public static Builder builder() { return new Builder(null); } @@ -1282,6 +1380,34 @@ public Builder addStringVariable(String name, String value) { return this; } + /** Add an artifact variable that expands {@code name} to {@code value}. */ + @CanIgnoreReturnValue + public Builder addArtifactVariable(String name, Artifact value) { + checkVariableNotPresentAlready(name); + Preconditions.checkNotNull(value, "Cannot set null as a value for variable '%s'", name); + variablesMap.put(name, value); + return this; + } + + /** + * Add an artifact or string variable that expands {@code name} to {@code value}. + * + *

Prefer {@link #addArtifactVariable} and {@link #addStringVariable}. This method is only + * meant to support string-based Starlark API. + */ + @CanIgnoreReturnValue + public Builder addArtifactOrStringVariable(String name, Object value) { + return switch (value) { + case String s -> addStringVariable(name, s); + case Artifact artifact -> addArtifactVariable(name, artifact); + case null -> + throw new IllegalArgumentException( + "Cannot set null as a value for variable '" + name + "'"); + default -> + throw new IllegalArgumentException("Unsupported value type: " + value.getClass()); + }; + } + /** Overrides a variable to expands {@code name} to {@code value} instead. */ @CanIgnoreReturnValue public Builder overrideStringVariable(String name, String value) { @@ -1335,6 +1461,19 @@ public Builder addStringSequenceVariable(String name, Iterable values) { return this; } + /** + * Add a sequence variable that expands {@code name} to {@link PathFragment} {@code values}. + * + *

Accepts values as NestedSet. Nested set is stored directly, not cloned, not flattened. + */ + @CanIgnoreReturnValue + public Builder addPathFragmentSequenceVariable(String name, NestedSet values) { + checkVariableNotPresentAlready(name); + Preconditions.checkNotNull(values, "Cannot set null as a value for variable '%s'", name); + variablesMap.put(name, new PathFragmentSetSequence(values)); + return this; + } + /** * Add a variable built using {@code VariableValueBuilder} api that expands {@code name} to the * value returned by the {@code builder}. @@ -1382,15 +1521,24 @@ public Builder addAllNonTransitive(CcToolchainVariables variables) { /** @return a new {@link CcToolchainVariables} object. */ public CcToolchainVariables build() { if (variablesMap.size() == 1) { - Object o = variablesMap.values().iterator().next(); - VariableValue variableValue = - o instanceof String ? new StringValue((String) o) : (VariableValue) o; - return new SingleVariables(parent, variablesMap.keySet().iterator().next(), variableValue); + return new SingleVariables( + parent, + variablesMap.keySet().iterator().next(), + asVariableValue(variablesMap.values().iterator().next())); } return new MapVariables(parent, variablesMap); } } + /** Wraps a raw variablesMap value into an appropriate VariableValue if necessary. */ + private static VariableValue asVariableValue(Object o) { + return switch (o) { + case String s -> new StringValue(s); + case Artifact artifact -> new ArtifactValue(artifact); + default -> (VariableValue) o; + }; + } + /** * A group of extra {@code Variable} instances, packaged as logic for adding to a {@code Builder} */ @@ -1450,11 +1598,7 @@ void addVariablesToMap(Map variablesMap) { @Override VariableValue getNonStructuredVariable(String name) { if (keyToIndex.containsKey(name)) { - Object o = values.get(keyToIndex.get(name)); - if (o instanceof String string) { - return new StringValue(string); - } - return (VariableValue) o; + return CcToolchainVariables.asVariableValue(values.get(keyToIndex.get(name))); } if (parent != null) { 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 ed1cdf2eb9d6fa..b53c81bd43c33b 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 @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.rules.cpp; +import static com.google.common.collect.ImmutableList.toImmutableList; + import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -140,20 +142,20 @@ public enum CompileBuildVariables { public static CcToolchainVariables setupVariablesOrReportRuleError( FeatureConfiguration featureConfiguration, CcToolchainProvider ccToolchainProvider, - String sourceFile, - String outputFile, + Artifact sourceFile, + Artifact outputFile, boolean isCodeCoverageEnabled, - String gcnoFile, + Artifact gcnoFile, boolean isUsingFission, - String dwoFile, - String ltoIndexingFile, + Artifact dwoFile, + Artifact ltoIndexingFile, ImmutableList includes, Iterable userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, String fdoStamp, - String dotdFileExecPath, - String diagnosticsFileExecPath, + Artifact dotdFile, + Artifact diagnosticsFile, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, @@ -188,15 +190,15 @@ public static CcToolchainVariables setupVariablesOrReportRuleError( usePic, fdoStamp, ccToolchainProvider.getFdoContext().getMemProfProfileArtifact() != null, - dotdFileExecPath, - diagnosticsFileExecPath, + dotdFile, + diagnosticsFile, variablesExtensions, additionalBuildVariables, directModuleMaps, - getSafePathStrings(includeDirs), - getSafePathStrings(quoteIncludeDirs), - getSafePathStrings(systemIncludeDirs), - getSafePathStrings(frameworkIncludeDirs), + includeDirs, + quoteIncludeDirs, + systemIncludeDirs, + frameworkIncludeDirs, defines, localDefines); } @@ -207,10 +209,10 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( String sourceFile, String outputFile, boolean isCodeCoverageEnabled, - String gcnoFile, + Artifact gcnoFile, boolean isUsingFission, - String dwoFile, - String ltoIndexingFile, + Artifact dwoFile, + Artifact ltoIndexingFile, String thinLtoIndex, String thinLtoInputBitcodeFile, String thinLtoOutputObjectFile, @@ -219,8 +221,8 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( CppModuleMap cppModuleMap, boolean usePic, String fdoStamp, - String dotdFileExecPath, - String diagnosticsFileExecPath, + Artifact dotdFile, + Artifact diagnosticsFile, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, @@ -255,15 +257,15 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( usePic, fdoStamp, ccToolchainProvider.getFdoContext().getMemProfProfileArtifact() != null, - dotdFileExecPath, - diagnosticsFileExecPath, + dotdFile, + diagnosticsFile, variablesExtensions, additionalBuildVariables, directModuleMaps, - includeDirs, - quoteIncludeDirs, - systemIncludeDirs, - frameworkIncludeDirs, + asPathFragments(includeDirs), + asPathFragments(quoteIncludeDirs), + asPathFragments(systemIncludeDirs), + asPathFragments(frameworkIncludeDirs), defines, localDefines); } @@ -271,31 +273,31 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException( private static CcToolchainVariables setupVariables( FeatureConfiguration featureConfiguration, CcToolchainVariables parent, - String sourceFile, - String outputFile, + Object sourceFile, + Object outputFile, boolean isCodeCoverageEnabled, - String gcnoFile, + Artifact gcnoFile, boolean isUsingFission, - String dwoFile, - String ltoIndexingFile, - String thinLtoIndex, - String thinLtoInputBitcodeFile, - String thinLtoOutputObjectFile, + Artifact dwoFile, + Artifact ltoIndexingFile, + Object thinLtoIndex, + Object thinLtoInputBitcodeFile, + Object thinLtoOutputObjectFile, ImmutableList includes, Iterable userCompileFlags, CppModuleMap cppModuleMap, boolean usePic, String fdoStamp, boolean isUsingMemProf, - String dotdFileExecPath, - String diagnosticsFileExecPath, + Artifact dotdFile, + Artifact diagnosticsFile, ImmutableList variablesExtensions, ImmutableMap additionalBuildVariables, Iterable directModuleMaps, - NestedSet includeDirs, - NestedSet quoteIncludeDirs, - NestedSet systemIncludeDirs, - NestedSet frameworkIncludeDirs, + NestedSet includeDirs, + NestedSet quoteIncludeDirs, + NestedSet systemIncludeDirs, + NestedSet frameworkIncludeDirs, Iterable defines, Iterable localDefines) { CcToolchainVariables.Builder buildVariables = CcToolchainVariables.builder(parent); @@ -328,8 +330,8 @@ private static CcToolchainVariables setupVariables( thinLtoInputBitcodeFile, thinLtoOutputObjectFile, userCompileFlags, - dotdFileExecPath, - diagnosticsFileExecPath, + dotdFile, + diagnosticsFile, usePic, ImmutableList.of(), ImmutableMap.of()); @@ -338,19 +340,54 @@ private static CcToolchainVariables setupVariables( public static void setupSpecificVariables( CcToolchainVariables.Builder buildVariables, - String sourceFile, - String outputFile, + Artifact sourceFile, + Artifact outputFile, boolean isCodeCoverageEnabled, - String gcnoFile, - String dwoFile, + Artifact gcnoFile, + Artifact dwoFile, boolean isUsingFission, - String ltoIndexingFile, - String thinLtoIndex, - String thinLtoInputBitcodeFile, - String thinLtoOutputObjectFile, + Artifact ltoIndexingFile, Iterable userCompileFlags, - String dotdFileExecPath, - String diagnosticsFileExecPath, + Artifact dotdFile, + Artifact diagnosticsFile, + boolean usePic, + ImmutableList externalIncludeDirs, + Map additionalBuildVariables) { + setupSpecificVariables( + buildVariables, + sourceFile, + outputFile, + isCodeCoverageEnabled, + gcnoFile, + dwoFile, + isUsingFission, + ltoIndexingFile, + /* thinLtoIndex= */ null, + /* thinLtoInputBitcodeFile= */ null, + /* thinLtoOutputObjectFile= */ null, + userCompileFlags, + dotdFile, + diagnosticsFile, + usePic, + externalIncludeDirs, + additionalBuildVariables); + } + + private static void setupSpecificVariables( + CcToolchainVariables.Builder buildVariables, + Object sourceFile, + Object outputFile, + boolean isCodeCoverageEnabled, + Artifact gcnoFile, + Artifact dwoFile, + boolean isUsingFission, + Artifact ltoIndexingFile, + Object thinLtoIndex, + Object thinLtoInputBitcodeFile, + Object thinLtoOutputObjectFile, + Iterable userCompileFlags, + Artifact dotdFile, + Artifact diagnosticsFile, boolean usePic, ImmutableList externalIncludeDirs, Map additionalBuildVariables) { @@ -358,26 +395,26 @@ public static void setupSpecificVariables( USER_COMPILE_FLAGS.getVariableName(), userCompileFlags); if (sourceFile != null) { - buildVariables.addStringVariable(SOURCE_FILE.getVariableName(), sourceFile); + buildVariables.addArtifactOrStringVariable(SOURCE_FILE.getVariableName(), sourceFile); } if (outputFile != null) { - buildVariables.addStringVariable(OUTPUT_FILE.getVariableName(), outputFile); + buildVariables.addArtifactOrStringVariable(OUTPUT_FILE.getVariableName(), outputFile); } // Set dependency_file to enable .d file generation. - if (dotdFileExecPath != null) { - buildVariables.addStringVariable(DEPENDENCY_FILE.getVariableName(), dotdFileExecPath); + if (dotdFile != null) { + buildVariables.addArtifactVariable(DEPENDENCY_FILE.getVariableName(), dotdFile); } // Set diagnostics_file to enable .dia file generation. - if (diagnosticsFileExecPath != null) { - buildVariables.addStringVariable( - SERIALIZED_DIAGNOSTICS_FILE.getVariableName(), diagnosticsFileExecPath); + if (diagnosticsFile != null) { + buildVariables.addArtifactVariable( + SERIALIZED_DIAGNOSTICS_FILE.getVariableName(), diagnosticsFile); } if (gcnoFile != null) { - buildVariables.addStringVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile); + buildVariables.addArtifactVariable(GCOV_GCNO_FILE.getVariableName(), gcnoFile); } else if (isCodeCoverageEnabled) { // TODO: Blaze currently uses `gcov_gcno_file` to detect if code coverage is enabled. It // should use a different signal. @@ -385,7 +422,7 @@ public static void setupSpecificVariables( } if (dwoFile != null) { - buildVariables.addStringVariable(PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile); + buildVariables.addArtifactVariable(PER_OBJECT_DEBUG_INFO_FILE.getVariableName(), dwoFile); } if (isUsingFission) { @@ -393,18 +430,18 @@ public static void setupSpecificVariables( } if (ltoIndexingFile != null) { - buildVariables.addStringVariable( + buildVariables.addArtifactVariable( LTO_INDEXING_BITCODE_FILE.getVariableName(), ltoIndexingFile); } if (thinLtoIndex != null) { - buildVariables.addStringVariable(THINLTO_INDEX.getVariableName(), thinLtoIndex); + buildVariables.addArtifactOrStringVariable(THINLTO_INDEX.getVariableName(), thinLtoIndex); } if (thinLtoInputBitcodeFile != null) { - buildVariables.addStringVariable( + buildVariables.addArtifactOrStringVariable( THINLTO_INPUT_BITCODE_FILE.getVariableName(), thinLtoInputBitcodeFile); } if (thinLtoOutputObjectFile != null) { - buildVariables.addStringVariable( + buildVariables.addArtifactOrStringVariable( THINLTO_OUTPUT_OBJECT_FILE.getVariableName(), thinLtoOutputObjectFile); } @@ -431,10 +468,10 @@ public static void setupCommonVariables( List variablesExtensions, Map additionalBuildVariables, Iterable directModuleMaps, - List includeDirs, - List quoteIncludeDirs, - List systemIncludeDirs, - List frameworkIncludeDirs, + ImmutableList includeDirs, + ImmutableList quoteIncludeDirs, + ImmutableList systemIncludeDirs, + ImmutableList frameworkIncludeDirs, Iterable defines, Iterable localDefines) { setupCommonVariablesInternal( @@ -447,10 +484,12 @@ public static void setupCommonVariables( variablesExtensions, additionalBuildVariables, directModuleMaps, - getSafePathStrings(includeDirs), - getSafePathStrings(quoteIncludeDirs), - getSafePathStrings(systemIncludeDirs), - getSafePathStrings(frameworkIncludeDirs), + // Stable order NestedSets wrapping ImmutableLists are interned, otherwise this would be + // a clear waste of memory as the single caller ensure that there are no duplicates. + NestedSetBuilder.wrap(Order.STABLE_ORDER, includeDirs), + NestedSetBuilder.wrap(Order.STABLE_ORDER, quoteIncludeDirs), + NestedSetBuilder.wrap(Order.STABLE_ORDER, systemIncludeDirs), + NestedSetBuilder.wrap(Order.STABLE_ORDER, frameworkIncludeDirs), defines, localDefines); } @@ -465,10 +504,10 @@ private static void setupCommonVariablesInternal( List variablesExtensions, Map additionalBuildVariables, Iterable directModuleMaps, - NestedSet includeDirs, - NestedSet quoteIncludeDirs, - NestedSet systemIncludeDirs, - NestedSet frameworkIncludeDirs, + NestedSet includeDirs, + NestedSet quoteIncludeDirs, + NestedSet systemIncludeDirs, + NestedSet frameworkIncludeDirs, Iterable defines, Iterable localDefines) { Preconditions.checkNotNull(directModuleMaps); @@ -481,8 +520,8 @@ private static void setupCommonVariablesInternal( if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) { buildVariables.addStringVariable(MODULE_NAME.getVariableName(), cppModuleMap.getName()); - buildVariables.addStringVariable( - MODULE_MAP_FILE.getVariableName(), cppModuleMap.getArtifact().getExecPathString()); + buildVariables.addArtifactVariable( + MODULE_MAP_FILE.getVariableName(), cppModuleMap.getArtifact()); buildVariables.addStringSequenceVariable( DEPENDENT_MODULE_MAP_FILES.getVariableName(), Iterables.transform(directModuleMaps, Artifact::getExecPathString)); @@ -491,17 +530,17 @@ private static void setupCommonVariablesInternal( // Module inputs will be set later when the action is executed. buildVariables.addStringSequenceVariable(MODULE_FILES.getVariableName(), ImmutableSet.of()); } - buildVariables.addStringSequenceVariable(INCLUDE_PATHS.getVariableName(), includeDirs); - buildVariables.addStringSequenceVariable( + buildVariables.addPathFragmentSequenceVariable(INCLUDE_PATHS.getVariableName(), includeDirs); + buildVariables.addPathFragmentSequenceVariable( QUOTE_INCLUDE_PATHS.getVariableName(), quoteIncludeDirs); - buildVariables.addStringSequenceVariable( + buildVariables.addPathFragmentSequenceVariable( SYSTEM_INCLUDE_PATHS.getVariableName(), systemIncludeDirs); if (!includes.isEmpty()) { buildVariables.addStringSequenceVariable(INCLUDES.getVariableName(), includes); } - buildVariables.addStringSequenceVariable( + buildVariables.addPathFragmentSequenceVariable( FRAMEWORK_PATHS.getVariableName(), frameworkIncludeDirs); Iterable allDefines; @@ -528,17 +567,11 @@ private static void setupCommonVariablesInternal( } } - /** Get the safe path strings for a list of paths to use in the build variables. */ - private static NestedSet getSafePathStrings(NestedSet paths) { - return getSafePathStrings(paths.toList()); - } - - /** Get the safe path strings for a list of paths to use in the build variables. */ - private static NestedSet getSafePathStrings(List paths) { - // Using ImmutableSet first to remove duplicates, then NestedSet for smaller memory footprint. + private static NestedSet asPathFragments(NestedSet paths) { + // Using ImmutableList as the final type to benefit from NestedSet interning. return NestedSetBuilder.wrap( Order.STABLE_ORDER, - Iterables.transform(ImmutableSet.copyOf(paths), PathFragment::getSafePathString)); + paths.toList().stream().map(PathFragment::create).collect(toImmutableList())); } public String getVariableName() { 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 349b38073bca51..3a49d3efca8dde 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 @@ -149,8 +149,8 @@ private static CcToolchainVariables getVariables( return CompileBuildVariables.setupVariablesOrReportRuleError( featureConfiguration, ccToolchainProvider, - sourceFile.getExecPathString(), - outputFile.getExecPathString(), + sourceFile, + outputFile, /* isCodeCoverageEnabled= */ false, /* gcnoFile= */ null, /* isUsingFission= */ false, @@ -164,8 +164,8 @@ private static CcToolchainVariables getVariables( /* cppModuleMap= */ null, needsPic, fdoBuildStamp, - /* dotdFileExecPath= */ null, - /* diagnosticsFileExecPath= */ null, + /* dotdFile= */ null, + /* diagnosticsFile= */ null, /* variablesExtensions= */ ImmutableList.of(), /* additionalBuildVariables= */ ImmutableMap.of(), /* directModuleMaps= */ ImmutableList.of(),