diff options
9 files changed, 142 insertions, 41 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java index 76b57efceb..b0183e09e5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java @@ -144,8 +144,13 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { Artifact soImplArtifact = null; boolean supportsDynamicLinker = ruleContext.getFragment(CppConfiguration.class).supportsDynamicLinker(); + // TODO(djasper): This is hacky. We should actually try to figure out whether we generate + // ccOutputs. boolean createDynamicLibrary = - !linkStatic && appearsToHaveObjectFiles(ruleContext.attributes()) && supportsDynamicLinker; + !linkStatic + && supportsDynamicLinker + && (appearsToHaveObjectFiles(ruleContext.attributes()) + || featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN)); if (ruleContext.getRule().isAttrDefined("outs", Type.STRING_LIST)) { List<String> outs = ruleContext.attributes().get("outs", Type.STRING_LIST); if (outs.size() > 1) { @@ -258,8 +263,10 @@ public abstract class CcLibrary implements RuleConfiguredTargetFactory { LinkerInputs.toNonSolibArtifacts(linkedLibraries.getExecutionDynamicLibraries())); CcLinkingOutputs linkingOutputs = info.getCcLinkingOutputs(); - warnAboutEmptyLibraries( - ruleContext, info.getCcCompilationOutputs(), linkStatic); + if (!featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN)) { + warnAboutEmptyLibraries( + ruleContext, info.getCcCompilationOutputs(), linkStatic); + } NestedSet<Artifact> filesToBuild = filesBuilder.build(); Runfiles staticRunfiles = collectRunfiles(ruleContext, linkingOutputs, ccToolchain, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java index 0724b07a52..11fcd4777a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibraryHelper.java @@ -282,6 +282,7 @@ public final class CcLibraryHelper { private final List<LibraryToLink> picStaticLibraries = new ArrayList<>(); private final List<LibraryToLink> dynamicLibraries = new ArrayList<>(); + private boolean emitLinkActions = true; private boolean emitLinkActionsIfEmpty; private boolean emitCcNativeLibrariesProvider; private boolean emitCcSpecificLinkParamsProvider; @@ -833,6 +834,11 @@ public final class CcLibraryHelper { return this; } + public CcLibraryHelper setEmitLinkActions(boolean emitLinkActions) { + this.emitLinkActions = emitLinkActions; + return this; + } + /** * Enables or disables generation of link actions if there are no object files. Some rules declare * a <code>.a</code> or <code>.so</code> implicit output, which requires that these files are @@ -930,7 +936,7 @@ public final class CcLibraryHelper { // Create link actions (only if there are object files or if explicitly requested). CcLinkingOutputs ccLinkingOutputs = CcLinkingOutputs.EMPTY; - if (emitLinkActionsIfEmpty || !ccOutputs.isEmpty()) { + if (emitLinkActions && (emitLinkActionsIfEmpty || !ccOutputs.isEmpty())) { // On some systems, the linker gives an error message if there are no input files. Even with // the check above, this can still happen if there is a .nopic.o or .o files in srcs, but no // other files. To fix that, we'd have to check for each link action individually. @@ -1012,7 +1018,7 @@ public final class CcLibraryHelper { Map<String, NestedSet<Artifact>> outputGroups = new TreeMap<>(); if (shouldAddLinkerOutputArtifacts(ruleContext, ccOutputs)) { - addLinkerOutputArtifacts(outputGroups); + addLinkerOutputArtifacts(outputGroups, ccOutputs); } outputGroups.put(OutputGroupProvider.TEMP_FILES, getTemps(ccOutputs)); @@ -1059,7 +1065,7 @@ public final class CcLibraryHelper { /** * Returns true if the appropriate attributes for linker output artifacts are defined, and either * the compile action produces object files or the build is configured to produce an archive or - * dynamic library even in the absense of object files. + * dynamic library even in the absence of object files. */ private boolean shouldAddLinkerOutputArtifacts( RuleContext ruleContext, CcCompilationOutputs ccOutputs) { @@ -1072,7 +1078,8 @@ public final class CcLibraryHelper { * Adds linker output artifacts to the given map, to be registered on the configured target as * output groups. */ - private void addLinkerOutputArtifacts(Map<String, NestedSet<Artifact>> outputGroups) { + private void addLinkerOutputArtifacts(Map<String, NestedSet<Artifact>> outputGroups, + CcCompilationOutputs ccOutputs) { NestedSetBuilder<Artifact> archiveFile = new NestedSetBuilder<>(Order.STABLE_ORDER); NestedSetBuilder<Artifact> dynamicLibrary = new NestedSetBuilder<>(Order.STABLE_ORDER); @@ -1085,7 +1092,8 @@ public final class CcLibraryHelper { CppHelper.getLinuxLinkedArtifact(ruleContext, Link.LinkTargetType.STATIC_LIBRARY)); } - if (CppRuleClasses.shouldCreateDynamicLibrary(ruleContext.attributes())) { + if (!ruleContext.attributes().get("linkstatic", Type.BOOLEAN) + && !ccOutputs.isEmpty()) { dynamicLibrary.add( CppHelper.getLinuxLinkedArtifact(ruleContext, Link.LinkTargetType.DYNAMIC_LIBRARY)); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index c51edff2f9..3f8437087f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -123,6 +123,9 @@ public class CppCompileAction extends AbstractAction * A string constant for the c++ compilation action. */ public static final String CPP_COMPILE = "c++-compile"; + + /** A string constant for the c++ module compile action. */ + public static final String CPP_MODULE_CODEGEN = "c++-module-codegen"; /** * A string constant for the objc compilation action. @@ -167,6 +170,7 @@ public class CppCompileAction extends AbstractAction private final ImmutableMap<String, String> localShellEnvironment; private final boolean isCodeCoverageEnabled; protected final Artifact outputFile; + private final Artifact sourceFile; private final Label sourceLabel; private final Artifact optionalSourceFile; private final NestedSet<Artifact> mandatoryInputs; @@ -317,6 +321,7 @@ public class CppCompileAction extends AbstractAction this.localShellEnvironment = localShellEnvironment; this.isCodeCoverageEnabled = isCodeCoverageEnabled; this.sourceLabel = sourceLabel; + this.sourceFile = sourceFile; this.outputFile = Preconditions.checkNotNull(outputFile); this.optionalSourceFile = optionalSourceFile; this.context = context; @@ -436,7 +441,7 @@ public class CppCompileAction extends AbstractAction throws ActionExecutionException, InterruptedException { Executor executor = actionExecutionContext.getExecutor(); Iterable<Artifact> initialResult; - + actionExecutionContext .getExecutor() .getEventBus() @@ -471,13 +476,18 @@ public class CppCompileAction extends AbstractAction Set<Artifact> initialResultSet = Sets.newLinkedHashSet(initialResult); if (shouldPruneModules) { - usedModules = Sets.newLinkedHashSet(); - topLevelModules = null; - for (CppCompilationContext.TransitiveModuleHeaders usedModule : - context.getUsedModules(usePic, initialResultSet)) { - usedModules.add(usedModule.getModule()); + if (CppFileTypes.CPP_MODULE.matches(sourceFile.getFilename())) { + usedModules = ImmutableSet.of(sourceFile); + initialResultSet.add(sourceFile); + } else { + usedModules = Sets.newLinkedHashSet(); + topLevelModules = null; + for (CppCompilationContext.TransitiveModuleHeaders usedModule : + context.getUsedModules(usePic, initialResultSet)) { + usedModules.add(usedModule.getModule()); + } + initialResultSet.addAll(usedModules); } - initialResultSet.addAll(usedModules); } initialResult = initialResultSet; 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 235ca5e086..6e75edd809 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 @@ -285,6 +285,8 @@ public class CppCompileActionBuilder { return CppCompileAction.PREPROCESS_ASSEMBLE; } else if (CppFileTypes.CLIF_INPUT_PROTO.matches(sourcePath)) { return CppCompileAction.CLIF_MATCH; + } else if (CppFileTypes.CPP_MODULE.matches(sourcePath)) { + return CppCompileAction.CPP_MODULE_CODEGEN; } // CcLibraryHelper ensures CppCompileAction only gets instantiated for supported file types. throw new IllegalStateException(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index ea1a1e8fde..9893dfc1e0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -791,6 +791,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + " name: 'random_seed'" + " flag_set {" + " action: 'c++-compile'" + + " action: 'c++-module-codegen'" + " action: 'c++-module-compile'" + " flag_group {" + " flag: '-frandom-seed=%{output_file}'" @@ -808,6 +809,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + " flag_set {" + " action: 'c-compile'" + " action: 'c++-compile'" + + " action: 'c++-module-codegen'" + " action: 'c++-module-compile'" + " action: 'preprocess-assemble'" + " expand_if_all_available: 'pic'" diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java index dc174850db..b819aae443 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java @@ -192,7 +192,8 @@ public final class CppFileTypes { String fileName = source.getFilename(); return !ASSEMBLER.matches(fileName) && !PIC_ASSEMBLER.matches(fileName) - && !CLIF_INPUT_PROTO.matches(fileName); + && !CLIF_INPUT_PROTO.matches(fileName) + && !CPP_MODULE.matches(fileName); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index 1df1eb6fec..43eca8af3d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -483,9 +483,17 @@ public final class CppModel { AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); if (shouldProvideHeaderModules()) { - createModuleAction(result, context.getCppModuleMap(), /*addHeaderTokenFiles=*/ false); + Collection<Artifact> modules = createModuleAction(result, context.getCppModuleMap()); + if (featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULE_CODEGEN)) { + for (Artifact module : modules) { + createModuleCodegenAction(result, module); + } + } } else if (context.getVerificationModuleMap() != null) { - createModuleAction(result, context.getVerificationModuleMap(), /*addHeaderTokenFiles=*/ true); + Collection<Artifact> modules = createModuleAction(result, context.getVerificationModuleMap()); + for (Artifact module : modules) { + result.addHeaderTokenFile(module); + } } for (CppSource source : sourceFiles) { @@ -525,8 +533,7 @@ public final class CppModel { // info). In that case the LTOBackendAction will generate the dwo. /*generateDwo=*/ cppConfiguration.useFission() && !bitcodeOutput, CppFileTypes.mustProduceDotdFile(sourceArtifact), - source.getBuildVariables(), /*addHeaderTokenFile=*/ - false); + source.getBuildVariables()); break; } } else { @@ -583,8 +590,70 @@ public final class CppModel { result.addHeaderTokenFile(tokenFile); } - private void createModuleAction( - CcCompilationOutputs.Builder result, CppModuleMap cppModuleMap, boolean addHeaderTokenFiles) { + private void createModuleCodegenAction(CcCompilationOutputs.Builder result, Artifact module) { + if (fake) { + // We can't currently foresee a situation where we'd want nocompile tests for module codegen. + // If we find one, support needs to be added here. + return; + } + String outputName = semantics.getEffectiveSourcePath(module).getPathString(); + + // TODO(djasper): Make this less hacky after refactoring how the PIC/noPIC actions are created. + boolean pic = module.getFilename().contains(".pic."); + + // TODO(djasper): Investigate whether we need to use a label separate from that of the module + // map. It is used for per-file-copts. + CppCompileActionBuilder builder = + initializeCompileAction( + module, Label.parseAbsoluteUnchecked(context.getCppModuleMap().getName())); + builder.setSemantics(semantics); + builder.setPicMode(pic); + builder.setOutputs( + ruleContext, + ArtifactCategory.OBJECT_FILE, + outputName, + CppFileTypes.mustProduceDotdFile(module)); + PathFragment ccRelativeName = semantics.getEffectiveSourcePath(module); + + String gcnoFileName = + CppHelper.getArtifactNameForCategory( + ruleContext, ccToolchain, ArtifactCategory.COVERAGE_DATA_FILE, outputName); + // TODO(djasper): This is now duplicated. Refactor the various create..Action functions. + Artifact gcnoFile = + isCodeCoverageEnabled() && !cppConfiguration.isLipoOptimization() + ? CppHelper.getCompileOutputArtifact(ruleContext, gcnoFileName, configuration) + : null; + + boolean generateDwo = cppConfiguration.useFission(); + Artifact dwoFile = generateDwo ? getDwoFile(builder.getOutputFile()) : null; + + setupCompileBuildVariables( + builder, + /*usePic=*/ pic, + ccRelativeName, + module.getExecPath(), + gcnoFile, + dwoFile, + builder.getContext().getCppModuleMap(), + ImmutableMap.<String, String>of()); + + builder.setGcnoFile(gcnoFile); + builder.setDwoFile(dwoFile); + + semantics.finalizeCompileActionBuilder(ruleContext, builder); + CppCompileAction compileAction = builder.build(); + AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); + env.registerAction(compileAction); + Artifact objectFile = compileAction.getOutputFile(); + if (pic) { + result.addPicObjectFile(objectFile); + } else { + result.addObjectFile(objectFile); + } + } + + private Collection<Artifact> createModuleAction( + CcCompilationOutputs.Builder result, CppModuleMap cppModuleMap) { AnalysisEnvironment env = ruleContext.getAnalysisEnvironment(); Label moduleMapLabel = Label.parseAbsoluteUnchecked(context.getCppModuleMap().getName()); Artifact moduleMapArtifact = cppModuleMap.getArtifact(); @@ -595,7 +664,7 @@ public final class CppModel { // A header module compile action is just like a normal compile action, but: // - the compiled source file is the module map // - it creates a header module (.pcm file). - createSourceAction( + return createSourceAction( FileSystemUtils.removeExtension(semantics.getEffectiveSourcePath(moduleMapArtifact)) .getPathString(), result, @@ -608,8 +677,7 @@ public final class CppModel { /*enableCoverage=*/ false, /*generateDwo=*/ false, CppFileTypes.mustProduceDotdFile(moduleMapArtifact), - ImmutableMap.<String, String>of(), /*addHeaderTokenFile=*/ - addHeaderTokenFiles); + ImmutableMap.<String, String>of()); } private void createClifMatchAction( @@ -637,7 +705,7 @@ public final class CppModel { result.addHeaderTokenFile(tokenFile); } - private void createSourceAction( + private Collection<Artifact> createSourceAction( String outputName, CcCompilationOutputs.Builder result, AnalysisEnvironment env, @@ -649,8 +717,8 @@ public final class CppModel { boolean enableCoverage, boolean generateDwo, boolean generateDotd, - Map<String, String> sourceSpecificBuildVariables, - boolean addHeaderTokenFiles) { + Map<String, String> sourceSpecificBuildVariables) { + ImmutableList.Builder<Artifact> directOutputs = new ImmutableList.Builder<>(); PathFragment ccRelativeName = semantics.getEffectiveSourcePath(sourceArtifact); if (cppConfiguration.isLipoOptimization()) { // TODO(bazel-team): we shouldn't be needing this, merging context with the binary @@ -704,9 +772,7 @@ public final class CppModel { semantics.finalizeCompileActionBuilder(ruleContext, picBuilder); CppCompileAction picAction = picBuilder.buildAndValidate(ruleContext); env.registerAction(picAction); - if (addHeaderTokenFiles) { - result.addHeaderTokenFile(picAction.getOutputFile()); - } + directOutputs.add(picAction.getOutputFile()); if (addObject) { result.addPicObjectFile(picAction.getOutputFile()); @@ -770,9 +836,7 @@ public final class CppModel { CppCompileAction compileAction = builder.buildAndValidate(ruleContext); env.registerAction(compileAction); Artifact objectFile = compileAction.getOutputFile(); - if (addHeaderTokenFiles) { - result.addHeaderTokenFile(objectFile); - } + directOutputs.add(objectFile); if (addObject) { result.addObjectFile(objectFile); if (featureConfiguration.isEnabled(CppRuleClasses.THIN_LTO) @@ -789,6 +853,7 @@ public final class CppModel { } } } + return directOutputs.build(); } private Artifact createCompileActionTemplate(AnalysisEnvironment env, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 024f26dc3f..bb22ec2036 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -39,7 +39,6 @@ import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ImplicitOutputsFunction.SafeImplicitOutputsFunction; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.InstrumentationSpec; -import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; @@ -47,12 +46,6 @@ import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoM * Rule class definitions for C++ rules. */ public class CppRuleClasses { - - /** Returns true if this rule should create a dynamic library. */ - public static boolean shouldCreateDynamicLibrary(AttributeMap rule) { - return !rule.get("linkstatic", Type.BOOLEAN) && CcLibrary.appearsToHaveObjectFiles(rule); - } - /** * Implementation for the :lipo_context_collector attribute. */ @@ -197,6 +190,9 @@ public class CppRuleClasses { /** A string constant for the header_modules_compile feature. */ public static final String HEADER_MODULE_COMPILE = "header_module_compile"; + + /** A string constant for the header_module_codegen feature. */ + public static final String HEADER_MODULE_CODEGEN = "header_module_codegen"; /** * A string constant for the compile_all_modules feature. diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java index 6f8f136fd4..1441289562 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java @@ -110,6 +110,16 @@ public abstract class MockCcSupport { + " flag: '--woohoo_modules'" + " }" + " }" + + " flag_set {" + + " action: 'c++-module-codegen'" + + " flag_group {" + + " flag: '--this_is_modules_codegen'" + + " }" + + " }" + + "}" + + "feature {" + + " name: 'header_module_codegen'" + + " implies: 'header_modules'" + "}" + "feature {" + " name: 'module_maps'" |