Skip to content

Commit

Permalink
Remove functionality to implement parse_headers via building a C++ mo…
Browse files Browse the repository at this point in the history
…dule. This

has never been used and is not worth keeping around.

RELNOTES: None.
PiperOrigin-RevId: 351452688
  • Loading branch information
djasper authored and copybara-github committed Jan 12, 2021
1 parent 7b5677e commit da38ab6
Show file tree
Hide file tree
Showing 7 changed files with 7 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif
private final NestedSet<Artifact> transitivePicModules;

private final CppModuleMap cppModuleMap;
private final CppModuleMap verificationModuleMap;

private final boolean propagateModuleMapAsActionInput;

Expand Down Expand Up @@ -129,7 +128,6 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif
ImmutableList<Artifact> directModuleMaps,
ImmutableList<CppModuleMap> exportingModuleMaps,
CppModuleMap cppModuleMap,
@Nullable CppModuleMap verificationModuleMap,
boolean propagateModuleMapAsActionInput,
CppConfiguration.HeadersCheckingMode headersCheckingMode,
NestedSet<Pair<String, String>> virtualToOriginalHeaders) {
Expand All @@ -144,7 +142,6 @@ public final class CcCompilationContext implements CcCompilationContextApi<Artif
this.transitivePicModules = transitivePicModules;
this.cppModuleMap = cppModuleMap;
this.nonCodeInputs = nonCodeInputs;
this.verificationModuleMap = verificationModuleMap;
this.compilationPrerequisites = compilationPrerequisites;
this.propagateModuleMapAsActionInput = propagateModuleMapAsActionInput;
this.headersCheckingMode = headersCheckingMode;
Expand Down Expand Up @@ -582,7 +579,6 @@ public static CcCompilationContext disallowUndeclaredHeaders(
ccCompilationContext.directModuleMaps,
ccCompilationContext.exportingModuleMaps,
ccCompilationContext.cppModuleMap,
ccCompilationContext.verificationModuleMap,
ccCompilationContext.propagateModuleMapAsActionInput,
ccCompilationContext.headersCheckingMode,
ccCompilationContext.virtualToOriginalHeaders);
Expand All @@ -593,11 +589,6 @@ public CppModuleMap getCppModuleMap() {
return cppModuleMap;
}

/** @return the C++ module map of the owner. */
public CppModuleMap getVerificationModuleMap() {
return verificationModuleMap;
}

/** Returns the list of dependencies' C++ module maps re-exported by this compilation context. */
public ImmutableList<CppModuleMap> getExportingModuleMaps() {
return exportingModuleMaps;
Expand Down Expand Up @@ -689,7 +680,6 @@ public static class Builder {
private final TransitiveSetHelper<String> defines = new TransitiveSetHelper<>();
private final Set<String> localDefines = new LinkedHashSet<>();
private CppModuleMap cppModuleMap;
private CppModuleMap verificationModuleMap;
private boolean propagateModuleMapAsActionInput = true;
private CppConfiguration.HeadersCheckingMode headersCheckingMode =
CppConfiguration.HeadersCheckingMode.STRICT;
Expand Down Expand Up @@ -944,12 +934,6 @@ public Builder setCppModuleMap(CppModuleMap cppModuleMap) {
return this;
}

/** Sets the C++ module map used to verify that headers are modules compatible. */
public Builder setVerificationModuleMap(CppModuleMap verificationModuleMap) {
this.verificationModuleMap = verificationModuleMap;
return this;
}

/** Causes the module map to be passed as an action input to dependant compilations. */
public Builder setPropagateCppModuleMapAsActionInput(boolean propagateModuleMap) {
this.propagateModuleMapAsActionInput = propagateModuleMap;
Expand Down Expand Up @@ -1020,7 +1004,6 @@ public CcCompilationContext build(ActionOwner owner, MiddlemanFactory middlemanF
ImmutableList.copyOf(directModuleMaps),
ImmutableList.copyOf(exportingModuleMaps),
cppModuleMap,
verificationModuleMap,
propagateModuleMapAsActionInput,
headersCheckingMode,
virtualToOriginalHeaders.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1049,8 +1049,7 @@ private CcCompilationContext initializeCcCompilationContext(RuleContext ruleCont
if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS)) {
if (cppModuleMap == null) {
cppModuleMap =
CppHelper.createDefaultCppModuleMap(
actionConstructionContext, configuration, label, /*suffix=*/ "");
CppHelper.createDefaultCppModuleMap(actionConstructionContext, configuration, label);
}

ccCompilationContextBuilder.setPropagateCppModuleMapAsActionInput(
Expand Down Expand Up @@ -1083,21 +1082,6 @@ private CcCompilationContext initializeCcCompilationContext(RuleContext ruleCont
if (getGeneratesNoPicHeaderModule()) {
ccCompilationContextBuilder.setHeaderModule(getHeaderModule(cppModuleMap.getArtifact()));
}
if (!compiled
&& featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS)
&& featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)
&& cppConfiguration.getParseHeadersVerifiesModules()) {
// Here, we are creating a compiled module to verify that headers are self-contained and
// modules ready, but we don't use the corresponding module map or compiled file anywhere
// else.
CppModuleMap verificationMap =
CppHelper.createDefaultCppModuleMap(
actionConstructionContext, configuration, label, /*suffix=*/ ".verify");
actionRegistry.registerAction(
createModuleMapAction(
verificationMap, publicHeaders, dependentModuleMaps, /*compiledModule=*/ true));
ccCompilationContextBuilder.setVerificationModuleMap(verificationMap);
}
}
ccCompilationContextBuilder.setPurpose(purpose);
return ccCompilationContextBuilder.build();
Expand Down Expand Up @@ -1352,12 +1336,6 @@ private CcCompilationOutputs createCcCompileActions() throws RuleErrorException
createModuleCodegenAction(result, moduleMapLabel, module);
}
}
} else if (ccCompilationContext.getVerificationModuleMap() != null) {
Collection<Artifact> modules =
createModuleAction(result, ccCompilationContext.getVerificationModuleMap());
for (Artifact module : modules) {
result.addHeaderTokenFile(module);
}
}

ImmutableMap<Artifact, String> outputNameMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,7 @@ public boolean supportsHeaderParsing() {
*/
public boolean shouldProcessHeaders(
FeatureConfiguration featureConfiguration, CppConfiguration cppConfiguration) {
// If parse_headers_verifies_modules is switched on, we verify that headers are
// self-contained by building the module instead.
return !cppConfiguration.getParseHeadersVerifiesModules()
&& featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS);
return featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,12 @@ public String getActionName() {
return CppActionNames.CPP_MODULE_COMPILE;
} else if (CppFileTypes.CPP_HEADER.matches(sourcePath)) {
// TODO(bazel-team): Handle C headers that probably don't work in C++ mode.
if (!cppConfiguration.getParseHeadersVerifiesModules()
&& featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS)) {
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS)) {
return CppActionNames.CPP_HEADER_PARSING;
} else {
// CcCommon.collectCAndCppSources() ensures we do not add headers to
// the compilation artifacts unless 'parse_headers' is set.
throw new IllegalStateException();
}
// CcCommon.collectCAndCppSources() ensures we do not add headers to
// the compilation artifacts unless 'parse_headers' is set.
throw new IllegalStateException();
} else if (CppFileTypes.C_SOURCE.matches(sourcePath)) {
return CppActionNames.C_COMPILE;
} else if (CppFileTypes.CPP_SOURCE.matches(sourcePath)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -441,10 +441,6 @@ public boolean getInmemoryDotdFiles() {
return cppOptions.inmemoryDotdFiles;
}

public boolean getParseHeadersVerifiesModules() {
return cppOptions.parseHeadersVerifiesModules;
}

public boolean getParseHeadersSkippedIfCorrespondingSrcsFound() {
return cppOptions.parseHeadersSkippedIfCorrespondingSrcsFound;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,14 +567,12 @@ public static boolean usePicForBinaries(
public static CppModuleMap createDefaultCppModuleMap(
ActionConstructionContext actionConstructionContext,
BuildConfiguration configuration,
Label label,
String suffix) {
Label label) {
// Create the module map artifact as a genfile.
Artifact mapFile =
actionConstructionContext.getPackageRelativeArtifact(
PathFragment.create(
label.getName()
+ suffix
+ Iterables.getOnlyElement(CppFileTypes.CPP_MODULE_MAP.getExtensions())),
configuration.getGenfilesDirectory(label.getRepository()));
return new CppModuleMap(mapFile, label.toString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -731,17 +731,6 @@ public Label getPropellerOptimizeLabel() {
)
public boolean inmemoryDotdFiles;

@Option(
name = "parse_headers_verifies_modules",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.CHANGES_INPUTS},
help =
"If enabled, the parse_headers feature verifies that a header module can be built for the "
+ "target in question instead of doing a separate compile of the header."
)
public boolean parseHeadersVerifiesModules;

@Option(
name = "experimental_parse_headers_skipped_if_corresponding_srcs_found",
defaultValue = "false",
Expand Down

0 comments on commit da38ab6

Please sign in to comment.