Skip to content

Commit

Permalink
Add an option to elide a separate header compile action if a source f…
Browse files Browse the repository at this point in the history
…ile with

the same basename is found. This is meant to be used with coding styles that
prescribe #including the own header first to ensure it compiles by itself, e.g.
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

In those situations, creating an additional compile action for just the header
does not add value.

RELNOTES: None.
PiperOrigin-RevId: 351397392
  • Loading branch information
djasper-gh authored and copybara-github committed Jan 12, 2021
1 parent c2ba027 commit 8cdeb87
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.io.Files;
import com.google.devtools.build.lib.actions.ActionRegistry;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
Expand Down Expand Up @@ -53,6 +54,7 @@
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -1368,8 +1370,15 @@ private CcCompilationOutputs createCcCompileActions() throws RuleErrorException
}
outputNameMap = calculateOutputNameMapByType(compilationUnitSources, outputNamePrefixDir);

Set<String> compiledBasenames = new HashSet<>();
for (CppSource source : compilationUnitSources.values()) {
Artifact sourceArtifact = source.getSource();

// Headers compilations will be created in the loop below.
if (!sourceArtifact.isTreeArtifact() && source.getType() == CppSource.Type.HEADER) {
continue;
}

Label sourceLabel = source.getLabel();
CppCompileActionBuilder builder = initializeCompileAction(sourceArtifact);

Expand All @@ -1385,33 +1394,27 @@ private CcCompilationOutputs createCcCompileActions() throws RuleErrorException
String outputName = outputNameMap.get(sourceArtifact);

if (!sourceArtifact.isTreeArtifact()) {
switch (source.getType()) {
case HEADER:
createHeaderAction(sourceLabel, outputName, result, builder);
break;
default:
createSourceAction(
sourceLabel,
outputName,
result,
sourceArtifact,
builder,
// TODO(plf): Continue removing CLIF logic from C++. Follow up changes would include
// refactoring CppSource.Type and ArtifactCategory to be classes instead of enums
// that could be instantiated with arbitrary values.
source.getType() == CppSource.Type.CLIF_INPUT_PROTO
? ArtifactCategory.CLIF_OUTPUT_PROTO
: ArtifactCategory.OBJECT_FILE,
ccCompilationContext.getCppModuleMap(),
/* addObject= */ true,
isCodeCoverageEnabled,
// The source action does not generate dwo when it has bitcode
// output (since it isn't generating a native object with debug
// info). In that case the LtoBackendAction will generate the dwo.
ccToolchain.shouldCreatePerObjectDebugInfo(featureConfiguration, cppConfiguration),
bitcodeOutput);
break;
}
compiledBasenames.add(Files.getNameWithoutExtension(sourceArtifact.getExecPathString()));
createSourceAction(
sourceLabel,
outputName,
result,
sourceArtifact,
builder,
// TODO(plf): Continue removing CLIF logic from C++. Follow up changes would include
// refactoring CppSource.Type and ArtifactCategory to be classes instead of enums
// that could be instantiated with arbitrary values.
source.getType() == CppSource.Type.CLIF_INPUT_PROTO
? ArtifactCategory.CLIF_OUTPUT_PROTO
: ArtifactCategory.OBJECT_FILE,
ccCompilationContext.getCppModuleMap(),
/* addObject= */ true,
isCodeCoverageEnabled,
// The source action does not generate dwo when it has bitcode
// output (since it isn't generating a native object with debug
// info). In that case the LtoBackendAction will generate the dwo.
ccToolchain.shouldCreatePerObjectDebugInfo(featureConfiguration, cppConfiguration),
bitcodeOutput);
} else {
switch (source.getType()) {
case HEADER:
Expand Down Expand Up @@ -1455,6 +1458,26 @@ private CcCompilationOutputs createCcCompileActions() throws RuleErrorException
}
}
}
for (CppSource source : compilationUnitSources.values()) {
Artifact artifact = source.getSource();
if (source.getType() != CppSource.Type.HEADER || artifact.isTreeArtifact()) {
// These are already handled above.
continue;
}
if (cppConfiguration.getParseHeadersSkippedIfCorrespondingSrcsFound()
&& compiledBasenames.contains(
Files.getNameWithoutExtension(artifact.getExecPathString()))) {
continue;
}
CppCompileActionBuilder builder = initializeCompileAction(artifact);
builder
.setSemantics(semantics)
.addMandatoryInputs(additionalCompilationInputs)
.addAdditionalIncludeScanningRoots(additionalIncludeScanningRoots);

String outputName = outputNameMap.get(artifact);
createHeaderAction(source.getLabel(), outputName, result, builder);
}

return result.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,10 @@ public boolean getParseHeadersVerifiesModules() {
return cppOptions.parseHeadersVerifiesModules;
}

public boolean getParseHeadersSkippedIfCorrespondingSrcsFound() {
return cppOptions.parseHeadersSkippedIfCorrespondingSrcsFound;
}

public boolean getUseInterfaceSharedLibraries() {
return cppOptions.useInterfaceSharedObjects;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,16 @@ public Label getPropellerOptimizeLabel() {
)
public boolean parseHeadersVerifiesModules;

@Option(
name = "experimental_parse_headers_skipped_if_corresponding_srcs_found",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
help =
"If enabled, the parse_headers feature does not create a separate header compile action "
+ "if a source with the same basename is found in the same target.")
public boolean parseHeadersSkippedIfCorrespondingSrcsFound;

@Option(
name = "experimental_omitfp",
defaultValue = "false",
Expand Down Expand Up @@ -1142,6 +1152,7 @@ public FragmentOptions getHost() {
host.disableNoCopts = disableNoCopts;
host.loadCcRulesFromBzl = loadCcRulesFromBzl;
host.validateTopLevelHeaderInclusions = validateTopLevelHeaderInclusions;
host.parseHeadersSkippedIfCorrespondingSrcsFound = parseHeadersSkippedIfCorrespondingSrcsFound;

// Save host options for further use.
host.hostCoptList = hostCoptList;
Expand Down

0 comments on commit 8cdeb87

Please sign in to comment.