If separate interface and implementation contexts are used, only the interface

context should generate a module map and module.

This whole code is a bit convoluted and the increasing number of boolean
parameters to initializeCppCompilationContext is smelly. I plan to clean this
up in a follow-up CL.

--
MOS_MIGRATED_REVID=138286169

1 files changed, 50 insertions, 34 deletions

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 13d6cbb701..136423222d 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
@@ -885,17 +885,25 @@ public final class CcLibraryHelper {
     }
 
     CppModel model = initializeCppModel();
-    CppCompilationContext cppCompilationContext =
-        initializeCppCompilationContext(model, /*forInterface=*/ false);
-    model.setContext(cppCompilationContext);
-
+    CppCompilationContext cppCompilationContext = null;
+    CppCompilationContext interfaceCompilationContext = null;
     // If we actually have different interface deps, we need a separate compilation context
     // for the interface. Otherwise, we can just re-use the normal cppCompilationContext.
-    CppCompilationContext interfaceCompilationContext = cppCompilationContext;
     // As implemenationDeps is a superset of interfaceDeps, comparing the size proves equality.
     if (implementationDeps.size() != interfaceDeps.size()) {
-      interfaceCompilationContext = initializeCppCompilationContext(model, /*forInterface=*/ true);
+      interfaceCompilationContext =
+          initializeCppCompilationContext(
+              model, /*forInterface=*/ true, /*createModuleMapActions=*/ true);
+      cppCompilationContext =
+          initializeCppCompilationContext(
+              model, /*forInterface=*/ false, /*createModuleMapActions=*/ false);
+    } else {
+      cppCompilationContext =
+          initializeCppCompilationContext(
+              model, /*forInterface=*/ false, /*createModuleMapActions=*/ true);
+      interfaceCompilationContext = cppCompilationContext;
     }
+    model.setContext(cppCompilationContext);
     model.setInterfaceContext(interfaceCompilationContext);
 
     boolean compileHeaderModules = featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES);
@@ -1103,7 +1111,7 @@ public final class CcLibraryHelper {
    * Create context for cc compile action from generated inputs.
    */
   private CppCompilationContext initializeCppCompilationContext(
-      CppModel model, boolean forInterface) {
+      CppModel model, boolean forInterface, boolean createModuleMapActions) {
     CppCompilationContext.Builder contextBuilder =
         new CppCompilationContext.Builder(ruleContext, forInterface);
 
@@ -1131,9 +1139,7 @@ public final class CcLibraryHelper {
     contextBuilder.mergeDependentContexts(
         AnalysisUtils.getProviders(
             forInterface ? interfaceDeps : implementationDeps, CppCompilationContext.class));
-    for (CppCompilationContext depContext : depContexts) {
-      contextBuilder.mergeDependentContext(depContext);
-    }
+    contextBuilder.mergeDependentContexts(depContexts);
     CppHelper.mergeToolchainDependentContext(ruleContext, contextBuilder);
 
     // But defines come after those inherited from deps.
@@ -1173,31 +1179,40 @@ public final class CcLibraryHelper {
               ? CppHelper.createDefaultCppModuleMap(ruleContext)
               : injectedCppModuleMap;
       contextBuilder.setCppModuleMap(cppModuleMap);
-      CppModuleMapAction action =
-          new CppModuleMapAction(
-              ruleContext.getActionOwner(),
-              cppModuleMap,
-              privateHeaders,
-              publicHeaders,
-              collectModuleMaps(),
-              additionalExportedHeaders,
-              featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES),
-              featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_HOME_CWD),
-              featureConfiguration.isEnabled(CppRuleClasses.GENERATE_SUBMODULES),
-              !featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_WITHOUT_EXTERN_MODULE));
-      ruleContext.registerAction(action);
-      if (model.getGeneratesPicHeaderModule()) {
-        contextBuilder.setPicHeaderModule(model.getPicHeaderModule(cppModuleMap.getArtifact()));
-      }
-      if (model.getGeneratesNoPicHeaderModule()) {
-        contextBuilder.setHeaderModule(model.getHeaderModule(cppModuleMap.getArtifact()));
-      }
-      contextBuilder.setUseHeaderModules(
-          featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES));
-      if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)
-          && featureConfiguration.isEnabled(CppRuleClasses.TRANSITIVE_MODULE_MAPS)) {
+      boolean useModules = featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES);
+      contextBuilder.setUseHeaderModules(useModules);
+      if (useModules && featureConfiguration.isEnabled(CppRuleClasses.TRANSITIVE_MODULE_MAPS)) {
         contextBuilder.setProvideTransitiveModuleMaps(true);
       }
+      if (createModuleMapActions) {
+        // TODO(djasper): The separation of interface and implementation dependencies doesn't work
+        // in conjunction with layering_check yet (and never has). In the long run to properly
+        // support layering_check together with interface/implementation dependencies, we need to
+        // write two modules to the module map, one with the headers and interface dependencies, the
+        // other with no headers, the implementation dependencies and an extra dependency on the
+        // first module. This division of two modules then needs to be properly used by the CppModel
+        // creating the CppCompileActions.
+        CppModuleMapAction action =
+            new CppModuleMapAction(
+                ruleContext.getActionOwner(),
+                cppModuleMap,
+                privateHeaders,
+                publicHeaders,
+                collectModuleMaps(),
+                additionalExportedHeaders,
+                featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES),
+                featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_HOME_CWD),
+                featureConfiguration.isEnabled(CppRuleClasses.GENERATE_SUBMODULES),
+                !featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAP_WITHOUT_EXTERN_MODULE));
+        ruleContext.registerAction(action);
+        if (model.getGeneratesPicHeaderModule()) {
+          contextBuilder.setPicHeaderModule(model.getPicHeaderModule(cppModuleMap.getArtifact()));
+        }
+        if (model.getGeneratesNoPicHeaderModule()) {
+          contextBuilder.setHeaderModule(model.getHeaderModule(cppModuleMap.getArtifact()));
+        }
+      }
+
     }
 
     semantics.setupCompilationContext(ruleContext, contextBuilder);
@@ -1208,7 +1223,8 @@ public final class CcLibraryHelper {
    * Creates context for cc compile action from generated inputs.
    */
   public CppCompilationContext initializeCppCompilationContext() {
-    return initializeCppCompilationContext(initializeCppModel(), /*forInterface=*/ false);
+    return initializeCppCompilationContext(
+        initializeCppModel(), /*forInterface=*/ false, /*createModuleMapActions=*/ true);
   }
 
   private Iterable<CppModuleMap> collectModuleMaps() {