Early support for code generation directly from C++ module files. This can

potentially reduce the linker input. The content of a C++ module (specifically
the inline definitions) is currently put into every single .o file that uses
them and thus can be input to the same final link many times. By generating a
separate .o file for the module itself, we can avoid this unnecessary
duplication of linker input.

--
PiperOrigin-RevId: 147579947
MOS_MIGRATED_REVID=147579947

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'"