diff options
4 files changed, 73 insertions, 19 deletions
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 820879e69f..d4ad88a217 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 @@ -407,7 +407,7 @@ public final class CcCompilationHelper { CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(privateHeader.getExecPath()); Preconditions.checkState(isHeader || isTextualInclude); - if (shouldProcessHeaders() && !isTextualInclude) { + if (ccToolchain.shouldProcessHeaders(featureConfiguration) && !isTextualInclude) { compilationUnitSources.add(CppSource.create(privateHeader, label, CppSource.Type.HEADER)); } this.privateHeaders.add(privateHeader); @@ -461,7 +461,7 @@ public final class CcCompilationHelper { CppFileTypes.CPP_HEADER.matches(header.getExecPath()) || header.isTreeArtifact(); boolean isTextualInclude = CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(header.getExecPath()); publicHeaders.add(header); - if (isTextualInclude || !isHeader || !shouldProcessHeaders()) { + if (isTextualInclude || !isHeader || !ccToolchain.shouldProcessHeaders(featureConfiguration)) { return; } compilationUnitSources.add(CppSource.create(header, label, CppSource.Type.HEADER)); @@ -500,14 +500,6 @@ public final class CcCompilationHelper { compilationUnitSources.add(CppSource.create(source, label, type)); } - private boolean shouldProcessHeaders() { - // 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.PREPROCESS_HEADERS) - || featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS)); - } - /** * Returns the compilation unit sources. That includes all compiled source files as well as * headers that will be parsed or preprocessed. Each source file contains the label it arises from diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index ad65e53ef0..307cb10016 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -264,6 +264,26 @@ public final class CcToolchainProvider extends ToolchainInfo implements CcToolch && supportsFission(); } + /** Whether the toolchains supports header parsing. */ + public boolean supportsHeaderParsing() { + return supportsHeaderParsing; + } + + /** + * Returns true if headers should be parsed in this build. + * + * <p>This means headers in 'srcs' and 'hdrs' will be "compiled" using {@link CppCompileAction}). + * It will run compiler's parser to ensure the header is self-contained. This is required for + * layering_check to work. + */ + public boolean shouldProcessHeaders(FeatureConfiguration featureConfiguration) { + // 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.PREPROCESS_HEADERS) + || featureConfiguration.isEnabled(CppRuleClasses.PARSE_HEADERS)); + } + /** * Returns true if Fission and PER_OBJECT_DEBUG_INFO are specified and supported by the CROSSTOOL * for the build implied by the given configuration, toolchain and feature configuration. @@ -449,13 +469,6 @@ public final class CcToolchainProvider extends ToolchainInfo implements CcToolch public boolean supportsParamFiles() { return supportsParamFiles; } - - /** - * Whether the toolchains supports header parsing. - */ - public boolean supportsHeaderParsing() { - return supportsHeaderParsing; - } /** * Returns the configured features of the toolchain. diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index 1bedc437fd..994e8ba999 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -80,6 +80,7 @@ public class CppCompileActionBuilder { @Nullable private String actionName; private ImmutableList<Artifact> builtinIncludeFiles; private Iterable<Artifact> inputsForInvalidation = ImmutableList.of(); + private Iterable<Artifact> additionalPrunableHeaders = ImmutableList.of(); // New fields need to be added to the copy constructor. /** @@ -290,6 +291,7 @@ public class CppCompileActionBuilder { NestedSetBuilder<Artifact> prunableHeadersBuilder = NestedSetBuilder.stableOrder(); prunableHeadersBuilder.addTransitive(ccCompilationContext.getDeclaredIncludeSrcs()); prunableHeadersBuilder.addTransitive(cppSemantics.getAdditionalPrunableIncludes()); + prunableHeadersBuilder.addAll(additionalPrunableHeaders); NestedSet<Artifact> prunableHeaders = prunableHeadersBuilder.build(); @@ -654,4 +656,13 @@ public class CppCompileActionBuilder { this.env = env; return this; } + + public void setAdditionalPrunableHeaders(Iterable<Artifact> additionalPrunableHeaders) { + this.additionalPrunableHeaders = Preconditions.checkNotNull(additionalPrunableHeaders); + } + + public boolean shouldCompileHeaders() { + Preconditions.checkNotNull(featureConfiguration); + return ccToolchain.shouldProcessHeaders(featureConfiguration); + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java index 5592c4f537..c1838770a0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; +import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.SourceCategory; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -82,13 +83,43 @@ public final class CppCompileActionTemplate implements ActionTemplate<CppCompile Iterable<TreeFileArtifact> inputTreeFileArtifacts, ArtifactOwner artifactOwner) throws ActionTemplateExpansionException { ImmutableList.Builder<CppCompileAction> expandedActions = new ImmutableList.Builder<>(); + + ImmutableList.Builder<TreeFileArtifact> sourcesBuilder = ImmutableList.builder(); + ImmutableList.Builder<Artifact> privateHeadersBuilder = ImmutableList.builder(); for (TreeFileArtifact inputTreeFileArtifact : inputTreeFileArtifacts) { + boolean isHeader = CppFileTypes.CPP_HEADER.matches(inputTreeFileArtifact.getExecPath()); + boolean isTextualInclude = + CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(inputTreeFileArtifact.getExecPath()); + boolean isSource = + SourceCategory.CC_AND_OBJC + .getSourceTypes() + .matches(inputTreeFileArtifact.getExecPathString()) + && !isHeader; + + if (isHeader) { + privateHeadersBuilder.add(inputTreeFileArtifact); + } + if (isSource || (isHeader && shouldCompileHeaders() && !isTextualInclude)) { + sourcesBuilder.add(inputTreeFileArtifact); + } else if (!isSource && !isHeader) { + throw new ActionTemplateExpansionException( + String.format( + "Artifact '%s' expanded from the directory artifact '%s' is neither header " + + "nor source file.", + inputTreeFileArtifact.getExecPathString(), sourceTreeArtifact.getExecPathString())); + } + } + ImmutableList<TreeFileArtifact> sources = sourcesBuilder.build(); + ImmutableList<Artifact> privateHeaders = privateHeadersBuilder.build(); + + for (TreeFileArtifact inputTreeFileArtifact : sources) { try { String outputName = outputTreeFileArtifactName(inputTreeFileArtifact); TreeFileArtifact outputTreeFileArtifact = ActionInputHelper.treeFileArtifact( outputTreeArtifact, PathFragment.create(outputName), artifactOwner); - expandedActions.add(createAction(inputTreeFileArtifact, outputTreeFileArtifact)); + expandedActions.add( + createAction(inputTreeFileArtifact, outputTreeFileArtifact, privateHeaders)); } catch (InvalidConfigurationException e) { throw new ActionTemplateExpansionException(e); } @@ -97,10 +128,17 @@ public final class CppCompileActionTemplate implements ActionTemplate<CppCompile return expandedActions.build(); } + private boolean shouldCompileHeaders() { + return cppCompileActionBuilder.shouldCompileHeaders(); + } + private CppCompileAction createAction( - Artifact sourceTreeFileArtifact, Artifact outputTreeFileArtifact) + Artifact sourceTreeFileArtifact, + Artifact outputTreeFileArtifact, + ImmutableList<Artifact> privateHeaders) throws ActionTemplateExpansionException { CppCompileActionBuilder builder = new CppCompileActionBuilder(cppCompileActionBuilder); + builder.setAdditionalPrunableHeaders(privateHeaders); builder.setSourceFile(sourceTreeFileArtifact); builder.setOutputs(outputTreeFileArtifact, null); |