Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose virtual:original header mappings via CcToolchain variables #21157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Expose virtual:original header mappings via CcToolchain variables #21157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,16 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif
// This is needed only when code coverage collection is enabled, to report the actual source file
// name in the coverage output file.
private final NestedSet<Tuple> virtualToOriginalHeaders;

/**
* Preserves mappings of _virtual_include directories to original paths relative to
* the workspace directory. Intended for use by compiler options like GCC's
* {@code -fdebug-prefix-map}. See {@link virtualToOriginalHeaders} for additional
* context. Unlike {@link virtualToOriginalHeaders}, this is populated even if
* coverage isn't enabled.
*/
private final NestedSet<Tuple> virtualToOriginalDirs;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new field will potentially result in memory regression. There's already virtualToOriginalHeaders, which is probably even larger, containing all header files, however it's only populated with coverage enabled.

I wonder if there's a more efficient way to reconstruct the information from _virtual_includes constant. It would be quite hacky though, but a wrapper script should be able to do it.


/**
* Caches the actual number of transitive headers reachable through transitiveHeaderInfos. We need
* to create maps to store these and so caching this count can substantially help with memory
Expand All @@ -116,6 +126,7 @@ private CcCompilationContext(
CppModuleMap cppModuleMap,
boolean propagateModuleMapAsActionInput,
NestedSet<Tuple> virtualToOriginalHeaders,
NestedSet<Tuple> virtualToOriginalDirs,
NestedSet<Artifact> headerTokens) {
Preconditions.checkNotNull(commandLineCcCompilationContext);
this.commandLineCcCompilationContext = commandLineCcCompilationContext;
Expand All @@ -130,6 +141,7 @@ private CcCompilationContext(
this.compilationPrerequisites = compilationPrerequisites;
this.propagateModuleMapAsActionInput = propagateModuleMapAsActionInput;
this.virtualToOriginalHeaders = virtualToOriginalHeaders;
this.virtualToOriginalDirs = virtualToOriginalDirs;
this.transitiveHeaderCount = -1;
this.headerTokens = headerTokens;
}
Expand Down Expand Up @@ -253,6 +265,12 @@ public Depset getStarlarkVirtualToOriginalHeaders(StarlarkThread thread) throws
return Depset.of(Tuple.class, getVirtualToOriginalHeaders());
}

@Override
public Depset getStarlarkVirtualToOriginalDirs(StarlarkThread thread) throws EvalException {
CcModule.checkPrivateStarlarkificationAllowlist(thread);
return Depset.of(Tuple.class, getVirtualToOriginalDirs());
}

@Override
@Nullable
public CppModuleMap getStarlarkModuleMap(StarlarkThread thread) throws EvalException {
Expand Down Expand Up @@ -648,6 +666,7 @@ public static CcCompilationContext createWithExtraHeaderTokens(
ccCompilationContext.cppModuleMap,
ccCompilationContext.propagateModuleMapAsActionInput,
ccCompilationContext.virtualToOriginalHeaders,
ccCompilationContext.virtualToOriginalDirs,
headerTokens.build());
}

Expand All @@ -665,6 +684,10 @@ public NestedSet<Tuple> getVirtualToOriginalHeaders() {
return virtualToOriginalHeaders;
}

public NestedSet<Tuple> getVirtualToOriginalDirs() {
return virtualToOriginalDirs;
}

/**
* The parts of the {@code CcCompilationContext} that influence the command line of compilation
* actions.
Expand Down Expand Up @@ -725,6 +748,7 @@ public static class Builder {
private CppModuleMap cppModuleMap;
private boolean propagateModuleMapAsActionInput = true;
private final NestedSetBuilder<Tuple> virtualToOriginalHeaders = NestedSetBuilder.stableOrder();
private final NestedSetBuilder<Tuple> virtualToOriginalDirs = NestedSetBuilder.stableOrder();
private final NestedSetBuilder<Artifact> headerTokens = NestedSetBuilder.stableOrder();

/** Creates a new builder for a {@link CcCompilationContext} instance. */
Expand Down Expand Up @@ -799,6 +823,8 @@ private void mergeDependentCcCompilationContext(
allDefines.addTransitive(otherCcCompilationContext.getDefines());
virtualToOriginalHeaders.addTransitive(
otherCcCompilationContext.getVirtualToOriginalHeaders());
virtualToOriginalDirs.addTransitive(
otherCcCompilationContext.getVirtualToOriginalDirs());

headerTokens.addTransitive(otherCcCompilationContext.getHeaderTokens());
}
Expand Down Expand Up @@ -1073,6 +1099,12 @@ public Builder addVirtualToOriginalHeaders(NestedSet<Tuple> virtualToOriginalHea
return this;
}

@CanIgnoreReturnValue
public Builder addVirtualToOriginalDirs(NestedSet<Tuple> virtualToOriginalDirs) {
this.virtualToOriginalDirs.addTransitive(virtualToOriginalDirs);
return this;
}

/** Builds the {@link CcCompilationContext}. */
public CcCompilationContext build() {
TransitiveSetHelper<String> allDefines = new TransitiveSetHelper<>();
Expand Down Expand Up @@ -1114,6 +1146,7 @@ public CcCompilationContext build() {
cppModuleMap,
propagateModuleMapAsActionInput,
virtualToOriginalHeaders.build(),
virtualToOriginalDirs.build(),
headerTokens.build());
}

Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1332,7 +1332,8 @@ private CcToolchainVariables setupCompileBuildVariables(
ccCompilationContext.getSystemIncludeDirs(),
ccCompilationContext.getFrameworkIncludeDirs(),
ccCompilationContext.getDefines(),
ccCompilationContext.getNonTransitiveDefines());
ccCompilationContext.getNonTransitiveDefines(),
ccCompilationContext.getVirtualToOriginalDirs());

if (usePrebuiltParent) {
parent = buildVariables.build();
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ public CcToolchainVariables getCompileBuildVariables(
Depset.noneableCast(
frameworkIncludeDirs, String.class, "framework_include_directories"),
Depset.noneableCast(defines, String.class, "preprocessor_defines").toList(),
ImmutableList.of()))
ImmutableList.of(),
/* virtualToOriginalDirs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER)))
.addStringSequenceVariable("stripopts", asClassImmutableList(stripOpts));
String inputFileString = convertFromNoneable(inputFile, null);
if (inputFileString != null) {
Expand Down Expand Up @@ -807,6 +808,7 @@ public CcCompilationContext createCcCompilationContext(
Object labelForMiddlemanNameObject,
Object externalIncludes,
Object virtualToOriginalHeaders,
Object virtualToOriginalDirs,
Sequence<?> dependentCcCompilationContexts,
Sequence<?> nonCodeInputs,
Sequence<?> looseHdrsDirsObject,
Expand Down Expand Up @@ -878,6 +880,9 @@ public CcCompilationContext createCcCompilationContext(
ccCompilationContext.addVirtualToOriginalHeaders(
Depset.cast(virtualToOriginalHeaders, Tuple.class, "virtual_to_original_headers"));

ccCompilationContext.addVirtualToOriginalDirs(
Depset.cast(virtualToOriginalDirs, Tuple.class, "virtual_to_original_dirs"));

ccCompilationContext.addDependentCcCompilationContexts(
Sequence.cast(
dependentCcCompilationContexts,
Expand Down
47 changes: 37 additions & 10 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Map;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.Tuple;

/** Enum covering all build variables we create for all various {@link CppCompileAction}. */
public enum CompileBuildVariables {
Expand Down Expand Up @@ -130,7 +131,12 @@ public enum CompileBuildVariables {
/** Path to the memprof profile artifact */
MEMPROF_PROFILE_PATH("memprof_profile_path"),
/** Variable for includes that compiler needs to include into sources. */
INCLUDES("includes");
INCLUDES("includes"),
/**
* A list of VIRTUAL=ORIGINAL include paths for "virtual" includes.
* Intended for use with gcc's -fdebug-prefix-map.
*/
VIRTUAL_TO_ORIGINAL_DIRS("virtual_to_original_dirs");

private final String variableName;

Expand Down Expand Up @@ -166,7 +172,8 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
NestedSet<PathFragment> systemIncludeDirs,
NestedSet<PathFragment> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines)
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs)
throws InterruptedException {
try {
if (usePic
Expand Down Expand Up @@ -201,7 +208,8 @@ public static CcToolchainVariables setupVariablesOrReportRuleError(
getSafePathStrings(systemIncludeDirs),
getSafePathStrings(frameworkIncludeDirs),
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
} catch (EvalException e) {
ruleErrorConsumer.ruleError(e.getMessage());
return CcToolchainVariables.EMPTY;
Expand Down Expand Up @@ -235,7 +243,8 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
NestedSet<String> systemIncludeDirs,
NestedSet<String> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines)
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs)
throws EvalException, InterruptedException {
if (usePic
&& !featureConfiguration.isEnabled(CppRuleClasses.PIC)
Expand Down Expand Up @@ -269,7 +278,8 @@ public static CcToolchainVariables setupVariablesOrThrowEvalException(
systemIncludeDirs,
frameworkIncludeDirs,
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
}

private static CcToolchainVariables setupVariables(
Expand Down Expand Up @@ -299,7 +309,8 @@ private static CcToolchainVariables setupVariables(
NestedSet<String> systemIncludeDirs,
NestedSet<String> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines) {
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs) {
CcToolchainVariables.Builder buildVariables = CcToolchainVariables.builder(parent);
setupCommonVariablesInternal(
buildVariables,
Expand All @@ -315,7 +326,8 @@ private static CcToolchainVariables setupVariables(
systemIncludeDirs,
frameworkIncludeDirs,
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
setupSpecificVariables(
buildVariables,
sourceFile,
Expand Down Expand Up @@ -430,7 +442,8 @@ public static void setupCommonVariables(
List<PathFragment> systemIncludeDirs,
List<PathFragment> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines) {
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs) {
setupCommonVariablesInternal(
buildVariables,
featureConfiguration,
Expand All @@ -445,7 +458,8 @@ public static void setupCommonVariables(
getSafePathStrings(systemIncludeDirs),
getSafePathStrings(frameworkIncludeDirs),
defines,
localDefines);
localDefines,
virtualToOriginalDirs);
}

private static void setupCommonVariablesInternal(
Expand All @@ -462,14 +476,16 @@ private static void setupCommonVariablesInternal(
NestedSet<String> systemIncludeDirs,
NestedSet<String> frameworkIncludeDirs,
Iterable<String> defines,
Iterable<String> localDefines) {
Iterable<String> localDefines,
NestedSet<Tuple> virtualToOriginalDirs) {
Preconditions.checkNotNull(directModuleMaps);
Preconditions.checkNotNull(includeDirs);
Preconditions.checkNotNull(quoteIncludeDirs);
Preconditions.checkNotNull(systemIncludeDirs);
Preconditions.checkNotNull(frameworkIncludeDirs);
Preconditions.checkNotNull(defines);
Preconditions.checkNotNull(localDefines);
Preconditions.checkNotNull(virtualToOriginalDirs);

if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS) && cppModuleMap != null) {
buildVariables.addStringVariable(MODULE_NAME.getVariableName(), cppModuleMap.getName());
Expand Down Expand Up @@ -510,6 +526,17 @@ private static void setupCommonVariablesInternal(

buildVariables.addStringSequenceVariable(PREPROCESSOR_DEFINES.getVariableName(), allDefines);

if (!virtualToOriginalDirs.isEmpty()) {
buildVariables.addStringSequenceVariable(
VIRTUAL_TO_ORIGINAL_DIRS.getVariableName(),
Iterables.transform(
virtualToOriginalDirs.toList(),
vToA -> vToA.get(0) + "=" + vToA.get(1)));
} else {
buildVariables.addStringSequenceVariable(VIRTUAL_TO_ORIGINAL_DIRS.getVariableName(),
ImmutableList.of());
}

buildVariables.addAllStringVariables(additionalBuildVariables);
for (VariablesExtension extension : variablesExtensions) {
extension.addVariables(buildVariables);
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ private static CcToolchainVariables getVariables(
ccToolchainProvider,
fdoBuildStamp,
codeCoverageEnabled),
/* localDefines= */ ImmutableList.of());
/* localDefines= */ ImmutableList.of(),
/* virtualToOriginalDirs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER));
} catch (EvalException e) {
throw new RuleErrorException(e.getMessage());
}
Expand Down
6 changes: 6 additions & 0 deletions ...main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcCompilationContextApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,12 @@ public interface CcCompilationContextApi<
useStarlarkThread = true)
Depset getStarlarkVirtualToOriginalHeaders(StarlarkThread thread) throws EvalException;

@StarlarkMethod(
name = "virtual_to_original_dirs",
documented = false,
useStarlarkThread = true)
Depset getStarlarkVirtualToOriginalDirs(StarlarkThread thread) throws EvalException;

@StarlarkMethod(
name = "module_map",
documented = false,
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/com/google/devtools/build/lib/starlarkbuildapi/cpp/CcModuleApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1279,6 +1279,12 @@ LinkingContextT createCcLinkingInfo(
positional = false,
named = true,
defaultValue = "unbound"),
@Param(
name = "virtual_to_original_dirs",
documented = false,
positional = false,
named = true,
defaultValue = "unbound"),
@Param(
name = "dependent_cc_compilation_contexts",
documented = false,
Expand Down Expand Up @@ -1363,6 +1369,7 @@ CompilationContextT createCcCompilationContext(
Object labelForMiddlemanNameObject,
Object externalIncludes,
Object virtualToOriginalHeaders,
Object virtualToOriginalDirs,
Sequence<?> dependentCcCompilationContexts,
Sequence<?> nonCodeInputs,
Sequence<?> looseHdrsDirs,
Expand Down
5 changes: 5 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/cc_common.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ def _create_compilation_context(
label = _UNBOUND,
external_includes = _UNBOUND,
virtual_to_original_headers = _UNBOUND,
virtual_to_original_dirs = _UNBOUND,
dependent_cc_compilation_contexts = _UNBOUND,
non_code_inputs = _UNBOUND,
headers_checking_mode = _UNBOUND,
Expand All @@ -447,6 +448,7 @@ def _create_compilation_context(
actions != _UNBOUND or \
external_includes != _UNBOUND or \
virtual_to_original_headers != _UNBOUND or \
virtual_to_original_dirs != _UNBOUND or \
dependent_cc_compilation_contexts != _UNBOUND or \
non_code_inputs != _UNBOUND or \
headers_checking_mode != _UNBOUND or \
Expand All @@ -471,6 +473,8 @@ def _create_compilation_context(
external_includes = depset()
if virtual_to_original_headers == _UNBOUND:
virtual_to_original_headers = depset()
if virtual_to_original_dirs == _UNBOUND:
virtual_to_original_dirs = depset()
if dependent_cc_compilation_contexts == _UNBOUND:
dependent_cc_compilation_contexts = []
if non_code_inputs == _UNBOUND:
Expand Down Expand Up @@ -508,6 +512,7 @@ def _create_compilation_context(
label = label,
external_includes = external_includes,
virtual_to_original_headers = virtual_to_original_headers,
virtual_to_original_dirs = virtual_to_original_dirs,
dependent_cc_compilation_contexts = dependent_cc_compilation_contexts,
non_code_inputs = non_code_inputs,
loose_hdrs_dirs = [],
Expand Down
Loading
Loading