Skip to content

Commit

Permalink
Automated rollback of commit 547d2a9.
Browse files Browse the repository at this point in the history 
*** Reason for rollback ***

b/179792876

*** Original change description ***

Add experimental facilities to create a second, smaller C++ module from the same
CcCompilationHelper and CcCompilationContext.

RELNOTES: None.
PiperOrigin-RevId: 356557738
  • Loading branch information
@tetromino @copybara-github
tetromino authored and copybara-github committed Feb 9, 2021
1 parent be96ade commit c6838bd
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 214 deletions.
102 changes: 10 additions & 92 deletions src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -438,12 +438,6 @@ public IncludeScanningHeaderData.Builder createIncludeScanningHeaderData(
treeArtifacts,
isModule,
modularHeaders);
handleHeadersForIncludeScanning(
transitiveHeaderInfo.separateModuleHeaders,
pathToLegalArtifact,
treeArtifacts,
isModule,
modularHeaders);
handleHeadersForIncludeScanning(
transitiveHeaderInfo.textualHeaders,
pathToLegalArtifact,
Expand All @@ -458,7 +452,6 @@ public IncludeScanningHeaderData.Builder createIncludeScanningHeaderData(
transitiveHeaderCount.compareAndSet(-1, pathToLegalArtifact.size());
removeArtifactsFromSet(modularHeaders, headerInfo.modularHeaders);
removeArtifactsFromSet(modularHeaders, headerInfo.textualHeaders);
removeArtifactsFromSet(modularHeaders, headerInfo.separateModuleHeaders);
return new IncludeScanningHeaderData.Builder(pathToLegalArtifact, modularHeaders);
}

Expand All @@ -467,13 +460,11 @@ public IncludeScanningHeaderData.Builder createIncludeScanningHeaderData(
* the modules that they are in.
*/
public Collection<Artifact.DerivedArtifact> computeUsedModules(
boolean usePic, Set<Artifact> includes, boolean separate) {
boolean usePic, Set<Artifact> includes) {
CompactHashSet<Artifact.DerivedArtifact> modules = CompactHashSet.create();
for (HeaderInfo transitiveHeaderInfo : headerInfo.getTransitiveCollection()) {
Artifact.DerivedArtifact module = transitiveHeaderInfo.getModule(usePic);
if (module == null) {
// If we don't have a main module, there is also not going to be a separate module. This is
// verified when constructing HeaderInfo instances.
continue;
}
// Not using range-based for loops here as often there is exactly one element in this list
Expand All @@ -485,18 +476,11 @@ public Collection<Artifact.DerivedArtifact> computeUsedModules(
break;
}
}
for (int i = 0; i < transitiveHeaderInfo.separateModuleHeaders.size(); i++) {
Artifact header = transitiveHeaderInfo.separateModuleHeaders.get(i);
if (includes.contains(header)) {
modules.add(transitiveHeaderInfo.getSeparateModule(usePic));
break;
}
}
}
// Do not add the module of the current rule for both:
// 1. the module compile itself
// 2. compiles of other translation units of the same rule.
modules.remove(separate ? headerInfo.getSeparateModule(usePic) : headerInfo.getModule(usePic));
modules.remove(headerInfo.getModule(usePic));
return modules;
}

Expand Down Expand Up @@ -537,28 +521,15 @@ public Iterable<Artifact> getDirectModuleMaps() {
return directModuleMaps;
}

Artifact.DerivedArtifact getHeaderModule(boolean usePic) {
return headerInfo.getModule(usePic);
}

Artifact.DerivedArtifact getSeparateHeaderModule(boolean usePic) {
return headerInfo.getSeparateModule(usePic);
}

/**
* @return all declared headers of the current module if the current target is compiled as a
* module.
*/
ImmutableList<Artifact> getHeaderModuleSrcs(boolean separateModule) {
if (separateModule) {
return headerInfo.separateModuleHeaders;
}
ImmutableSet<Artifact> getHeaderModuleSrcs() {
return new ImmutableSet.Builder<Artifact>()
.addAll(headerInfo.modularHeaders)
.addAll(headerInfo.textualHeaders)
.addAll(headerInfo.separateModuleHeaders)
.build()
.asList();
.build();
}

/**
Expand Down Expand Up @@ -752,14 +723,14 @@ public Builder mergeDependentCcCompilationContext(
looseHdrsDirs.addTransitive(otherCcCompilationContext.getLooseHdrsDirs());
declaredIncludeSrcs.addTransitive(otherCcCompilationContext.getDeclaredIncludeSrcs());
headerInfoBuilder.addDep(otherCcCompilationContext.headerInfo);

transitiveModules.addTransitive(otherCcCompilationContext.transitiveModules);
addIfNotNull(transitiveModules, otherCcCompilationContext.headerInfo.headerModule);
addIfNotNull(transitiveModules, otherCcCompilationContext.headerInfo.separateModule);

if (otherCcCompilationContext.headerInfo.headerModule != null) {
transitiveModules.add(otherCcCompilationContext.headerInfo.headerModule);
}
transitivePicModules.addTransitive(otherCcCompilationContext.transitivePicModules);
addIfNotNull(transitivePicModules, otherCcCompilationContext.headerInfo.picHeaderModule);
addIfNotNull(transitivePicModules, otherCcCompilationContext.headerInfo.separatePicModule);
if (otherCcCompilationContext.headerInfo.picHeaderModule != null) {
transitivePicModules.add(otherCcCompilationContext.headerInfo.picHeaderModule);
}

nonCodeInputs.addTransitive(otherCcCompilationContext.nonCodeInputs);

Expand All @@ -779,13 +750,6 @@ public Builder mergeDependentCcCompilationContext(
return this;
}

private static void addIfNotNull(
NestedSetBuilder<Artifact> builder, @Nullable Artifact artifact) {
if (artifact != null) {
builder.add(artifact);
}
}

/**
* Merges the given {@code CcCompilationContext}s into this one by adding the contents of their
* attributes.
Expand Down Expand Up @@ -927,14 +891,6 @@ public Builder addTextualHdrs(Collection<Artifact> headers) {
return this;
}

public Builder setSeparateModuleHdrs(
Collection<Artifact> headers,
Artifact.DerivedArtifact separateModule,
Artifact.DerivedArtifact separatePicModule) {
this.headerInfoBuilder.setSeparateModuleHdrs(headers, separateModule, separatePicModule);
return this;
}

/** Add a set of required non-code compilation input. */
public Builder addNonCodeInputs(Iterable<Artifact> inputs) {
nonCodeInputs.addAll(inputs);
Expand Down Expand Up @@ -1150,12 +1106,6 @@ public static final class HeaderInfo {
/** All textual header files that are contained in this module. */
private final ImmutableList<Artifact> textualHeaders;

/** Headers that can be compiled into a separate, smaller module for performance reasons. */
private final ImmutableList<Artifact> separateModuleHeaders;

private final Artifact.DerivedArtifact separateModule;
private final Artifact.DerivedArtifact separatePicModule;

/** HeaderInfos of direct dependencies of C++ target represented by this context. */
private final ImmutableList<HeaderInfo> deps;

Expand All @@ -1168,9 +1118,6 @@ public static final class HeaderInfo {
ImmutableList<Artifact> modularPublicHeaders,
ImmutableList<Artifact> modularPrivateHeaders,
ImmutableList<Artifact> textualHeaders,
ImmutableList<Artifact> separateModuleHeaders,
Artifact.DerivedArtifact separateModule,
Artifact.DerivedArtifact separatePicModule,
ImmutableList<HeaderInfo> deps) {
this.headerModule = headerModule;
this.picHeaderModule = picHeaderModule;
Expand All @@ -1179,20 +1126,13 @@ public static final class HeaderInfo {
this.modularHeaders =
ImmutableList.copyOf(Iterables.concat(modularPublicHeaders, modularPrivateHeaders));
this.textualHeaders = textualHeaders;
this.separateModuleHeaders = separateModuleHeaders;
this.separateModule = separateModule;
this.separatePicModule = separatePicModule;
this.deps = deps;
}

Artifact.DerivedArtifact getModule(boolean pic) {
return pic ? picHeaderModule : headerModule;
}

Artifact.DerivedArtifact getSeparateModule(boolean pic) {
return pic ? separatePicModule : separateModule;
}

public Collection<HeaderInfo> getTransitiveCollection() {
if (deps.isEmpty()) {
return ImmutableList.of(this);
Expand Down Expand Up @@ -1292,9 +1232,6 @@ public static class Builder {
private final Set<Artifact> modularPublicHeaders = new HashSet<>();
private final Set<Artifact> modularPrivateHeaders = new HashSet<>();
private final Set<Artifact> textualHeaders = new HashSet<>();
private Collection<Artifact> separateModuleHeaders = ImmutableList.of();
private Artifact.DerivedArtifact separateModule = null;
private Artifact.DerivedArtifact separatePicModule = null;
private final List<HeaderInfo> deps = new ArrayList<>();

Builder setHeaderModule(Artifact.DerivedArtifact headerModule) {
Expand Down Expand Up @@ -1343,16 +1280,6 @@ public Builder addTextualHeaders(Collection<Artifact> headers) {
return this;
}

public Builder setSeparateModuleHdrs(
Collection<Artifact> headers,
Artifact.DerivedArtifact separateModule,
Artifact.DerivedArtifact separatePicModule) {
this.separateModuleHeaders = headers;
this.separateModule = separateModule;
this.separatePicModule = separatePicModule;
return this;
}

/** Adds the headers of the given {@code HeaderInfo} into the one being built. */
public Builder mergeHeaderInfo(HeaderInfo otherHeaderInfo) {
this.modularPublicHeaders.addAll(otherHeaderInfo.modularPublicHeaders);
Expand All @@ -1362,21 +1289,12 @@ public Builder mergeHeaderInfo(HeaderInfo otherHeaderInfo) {
}

public HeaderInfo build() {
Preconditions.checkState(
(separateModule == null || headerModule != null)
&& (separatePicModule == null || picHeaderModule != null),
"Separate module cannot be used without main module",
separateModule,
separatePicModule);
return new HeaderInfo(
headerModule,
picHeaderModule,
ImmutableList.copyOf(modularPublicHeaders),
ImmutableList.copyOf(modularPrivateHeaders),
ImmutableList.copyOf(textualHeaders),
ImmutableList.copyOf(separateModuleHeaders),
separateModule,
separatePicModule,
ImmutableList.copyOf(deps));
}
}
Expand Down
Loading

0 comments on commit c6838bd

Please sign in to comment.