Add module map support to ExperimentalObjcLibrary, which contains experimental support

for building objc code using the c++ crosstool.  This will eventually replace the current module support in blaze.

Note: This required injecting a CppModuleMap into the cc logic.  The reason that objc cannot rely on standard CppModuleMap creation logic is that there is different naming semantics for module maps between cpp and objc.  In particular:

 - In cc, module maps can be inputs to a compilation action. Thus, the module maps are given labels.
 - In objc, if interoping with swift, module maps are explicitly referenced in swift code.  Thus, their names cannot contain illegal characters for swift source.  Those, some name mangling occurs to get rid of "//" and ":".  To enforce that this does not cause problems with compilation actions, the CPP_MODULE_COMPILE action has been disabled for the objc CcLibraryHelper.SourceCategory

--
MOS_MIGRATED_REVID=124177067

5 files changed, 129 insertions, 45 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 aaf5e95410..58205e7e6e 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
@@ -118,7 +118,6 @@ public final class CcLibraryHelper {
             CppCompileAction.OBJCPP_COMPILE,
             CppCompileAction.CPP_HEADER_PARSING,
             CppCompileAction.CPP_HEADER_PREPROCESSING,
-            CppCompileAction.CPP_MODULE_COMPILE,
             CppCompileAction.ASSEMBLE,
             CppCompileAction.PREPROCESS_ASSEMBLE,
             Link.LinkTargetType.STATIC_LIBRARY.getActionName(),
@@ -274,8 +273,9 @@ public final class CcLibraryHelper {
   private boolean emitDynamicLibrary = true;
   private boolean checkDepsGenerateCpp = true;
   private boolean emitCompileProviders;
-  private SourceCategory sourceCatagory;
+  private final SourceCategory sourceCategory;
   private List<VariablesExtension> variablesExtensions = new ArrayList<>();
+  @Nullable private CppModuleMap injectedCppModuleMap;
   
   private final FeatureConfiguration featureConfiguration;
 
@@ -296,7 +296,7 @@ public final class CcLibraryHelper {
     this.configuration = ruleContext.getConfiguration();
     this.semantics = Preconditions.checkNotNull(semantics);
     this.featureConfiguration = Preconditions.checkNotNull(featureConfiguration);
-    this.sourceCatagory = Preconditions.checkNotNull(sourceCatagory);
+    this.sourceCategory = Preconditions.checkNotNull(sourceCatagory);
   }
 
   public CcLibraryHelper(
@@ -443,7 +443,7 @@ public final class CcLibraryHelper {
   private void addSource(Artifact source, Label label) {
     boolean isHeader = CppFileTypes.CPP_HEADER.matches(source.getExecPath());
     boolean isTextualInclude = CppFileTypes.CPP_TEXTUAL_INCLUDE.matches(source.getExecPath());
-    boolean isCompiledSource = sourceCatagory.getSourceTypes().matches(source.getExecPathString());
+    boolean isCompiledSource = sourceCategory.getSourceTypes().matches(source.getExecPathString());
     if (isHeader || isTextualInclude) {
       privateHeaders.add(source);
     }
@@ -685,6 +685,15 @@ public final class CcLibraryHelper {
   }
   
   /**
+   * Sets the module map artifact for this build.
+   */
+  public CcLibraryHelper setCppModuleMap(CppModuleMap cppModuleMap) {
+    Preconditions.checkNotNull(cppModuleMap);
+    this.injectedCppModuleMap = cppModuleMap;
+    return this;
+  }
+  
+  /**
    * Overrides the path for the generated dynamic library - this should only be called if the
    * dynamic library is an implicit or explicit output of the rule, i.e., if it is accessible by
    * name from other rules in the same package. Set to {@code null} to use the default computation.
@@ -1031,24 +1040,24 @@ public final class CcLibraryHelper {
     }
 
     if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS)) {
-      CppModuleMap cppModuleMap = CppHelper.addCppModuleMapToContext(ruleContext, contextBuilder);
-      // TODO(bazel-team): addCppModuleMapToContext second-guesses whether module maps should
-      // actually be enabled, so we need to double-check here. Who would write code like this?
-      if (cppModuleMap != null) {
-        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);
-      }
+      CppModuleMap cppModuleMap =
+          injectedCppModuleMap == null
+              ? 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()));
       }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
index 7069c8fbcd..f554eb9c34 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java
@@ -418,17 +418,17 @@ public class CppHelper {
     return (dep != null) ? dep.getProvider(LipoContextProvider.class) : null;
   }
 
-  // Creates CppModuleMap object, and adds it to C++ compilation context.
-  public static CppModuleMap addCppModuleMapToContext(RuleContext ruleContext,
-      CppCompilationContext.Builder contextBuilder) {
+  /**
+   * Creates a CppModuleMap object for pure c++ builds.  The module map artifact becomes a
+   * candidate input to a CppCompileAction.
+   */
+  public static CppModuleMap createDefaultCppModuleMap(RuleContext ruleContext) {
     // Create the module map artifact as a genfile.
     Artifact mapFile = ruleContext.getPackageRelativeArtifact(
         ruleContext.getLabel().getName()
             + Iterables.getOnlyElement(CppFileTypes.CPP_MODULE_MAP.getExtensions()),
         ruleContext.getConfiguration().getGenfilesDirectory());
-        CppModuleMap moduleMap = new CppModuleMap(mapFile, ruleContext.getLabel().toString());
-    contextBuilder.setCppModuleMap(moduleMap);
-    return moduleMap;
+    return new CppModuleMap(mapFile, ruleContext.getLabel().toString());
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalObjcLibrary.java
index 6190ffddd3..83c1ede36b 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalObjcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ExperimentalObjcLibrary.java
@@ -21,7 +21,6 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
-import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
 import com.google.devtools.build.lib.rules.cpp.CcLibraryHelper;
@@ -30,6 +29,7 @@ import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.Bui
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.ValueSequence;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables.VariablesExtension;
 import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider;
+import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
 import com.google.devtools.build.lib.rules.cpp.PrecompiledFiles;
 
 import java.util.Collection;
@@ -41,6 +41,10 @@ public class ExperimentalObjcLibrary implements RuleConfiguredTargetFactory {
 
   private static final String PCH_FILE_VARIABLE_NAME = "pch_file";
   private static final String FRAMEWORKS_VARIABLE_NAME = "framework_paths";
+  private static final String MODULES_MAPS_DIR_NAME = "module_maps_dir";
+  private static final String OBJC_MODULE_CACHE_DIR_NAME = "_objc_module_cache";
+  private static final String OBJC_MODULE_CACHE_KEY = "modules_cache_path";
+  private static final String OBJC_MODULE_FEATURE_NAME = "use_objc_modules";
   private static final Iterable<String> ACTIVATED_ACTIONS =
       ImmutableList.of("objc-compile", "objc++-compile");
 
@@ -61,6 +65,9 @@ public class ExperimentalObjcLibrary implements RuleConfiguredTargetFactory {
     public void addVariables(Builder builder) {
       addPchVariables(builder);
       addFrameworkVariables(builder);
+      if (ObjcCommon.shouldUseObjcModules(ruleContext)) {
+        addModuleMapVariables(builder);
+      }
     }
 
     private void addPchVariables(Builder builder) {
@@ -80,11 +87,26 @@ public class ExperimentalObjcLibrary implements RuleConfiguredTargetFactory {
       }
       builder.addSequence(FRAMEWORKS_VARIABLE_NAME, frameworkSequence.build());
     }
+
+    private void addModuleMapVariables(Builder builder) {
+      builder.addVariable(
+          MODULES_MAPS_DIR_NAME,
+          ObjcRuleClasses.intermediateArtifacts(ruleContext)
+              .moduleMap()
+              .getArtifact()
+              .getExecPath()
+              .getParentDirectory()
+              .toString());
+      builder.addVariable(
+          OBJC_MODULE_CACHE_KEY,
+          ruleContext.getConfiguration().getGenfilesFragment() + "/" + OBJC_MODULE_CACHE_DIR_NAME);
+    }
   }
 
   @Override
   public ConfiguredTarget create(RuleContext ruleContext)
       throws InterruptedException, RuleErrorException {
+    validateAttributes(ruleContext);
 
     CompilationArtifacts compilationArtifacts =
         CompilationSupport.compilationArtifacts(ruleContext);
@@ -94,20 +116,6 @@ public class ExperimentalObjcLibrary implements RuleConfiguredTargetFactory {
 
     ObjcCommon common = common(ruleContext, compilationAttributes, compilationArtifacts);
 
-    CcToolchainProvider toolchain =
-        ruleContext
-            .getPrerequisite(":cc_toolchain", Mode.TARGET)
-            .getProvider(CcToolchainProvider.class);
-
-    ImmutableList.Builder<String> activatedCrosstoolSelectables =
-        ImmutableList.<String>builder().addAll(ACTIVATED_ACTIONS);
-    if (ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET) != null) {
-      activatedCrosstoolSelectables.add("pch");
-    }
-
-    FeatureConfiguration featureConfiguration =
-        toolchain.getFeatures().getFeatureConfiguration(activatedCrosstoolSelectables.build());
-
     Collection<Artifact> sources = Sets.newHashSet(compilationArtifacts.getSrcs());
     Collection<Artifact> privateHdrs = Sets.newHashSet(compilationArtifacts.getPrivateHdrs());
     Collection<Artifact> publicHdrs = Sets.newHashSet(compilationAttributes.hdrs());
@@ -116,7 +124,7 @@ public class ExperimentalObjcLibrary implements RuleConfiguredTargetFactory {
         new CcLibraryHelper(
                 ruleContext,
                 new ObjcCppSemantics(common.getObjcProvider()),
-                featureConfiguration,
+                getFeatureConfiguration(ruleContext),
                 CcLibraryHelper.SourceCategory.CC_AND_OBJC)
             .addSources(sources)
             .addSources(privateHdrs)
@@ -127,6 +135,10 @@ public class ExperimentalObjcLibrary implements RuleConfiguredTargetFactory {
             .addVariableExtension(
                 new ObjcVariablesExtension(ruleContext, common.getObjcProvider()));
 
+    if (ObjcCommon.shouldUseObjcModules(ruleContext)) {
+      helper.setCppModuleMap(ObjcRuleClasses.intermediateArtifacts(ruleContext).moduleMap());
+    }
+
     CcLibraryHelper.Info info = helper.build();
 
     NestedSetBuilder<Artifact> filesToBuild =
@@ -137,6 +149,40 @@ public class ExperimentalObjcLibrary implements RuleConfiguredTargetFactory {
         .build();
   }
 
+  private FeatureConfiguration getFeatureConfiguration(RuleContext ruleContext) {
+    CcToolchainProvider toolchain =
+        ruleContext
+            .getPrerequisite(":cc_toolchain", Mode.TARGET)
+            .getProvider(CcToolchainProvider.class);
+
+    ImmutableList.Builder<String> activatedCrosstoolSelectables =
+        ImmutableList.<String>builder().addAll(ACTIVATED_ACTIONS);
+
+    if (ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET) != null) {
+      activatedCrosstoolSelectables.add("pch");
+    }
+
+    if (ObjcCommon.shouldUseObjcModules(ruleContext)) {
+      activatedCrosstoolSelectables.add(OBJC_MODULE_FEATURE_NAME);
+    }
+    
+    // We create a module map by default to allow for swift interop.
+    activatedCrosstoolSelectables.add(CppRuleClasses.MODULE_MAPS);
+
+    return toolchain.getFeatures().getFeatureConfiguration(activatedCrosstoolSelectables.build());
+  }
+
+  /**
+   * Throws errors or warnings for bad attribute state.
+   */
+  private void validateAttributes(RuleContext ruleContext) {
+    for (String copt : ObjcCommon.getNonCrosstoolCopts(ruleContext)) {
+      if (copt.contains("-fmodules-cache-path")) {
+        ruleContext.ruleWarning(CompilationSupport.MODULES_CACHE_PATH_WARNING);
+      }
+    }
+  }
+
   private static ObjcCommon common(
       RuleContext ruleContext,
       CompilationAttributes compilationAttributes,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
index b86f124d87..effeefc035 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/IntermediateArtifacts.java
@@ -393,7 +393,7 @@ public final class IntermediateArtifacts {
    * {@link CppModuleMap} that provides the clang module map for this target.
    */
   public CppModuleMap moduleMap() {
-    if (!buildConfiguration.getFragment(ObjcConfiguration.class).moduleMapsEnabled()) {
+    if (!ObjcCommon.shouldUseObjcModules(ruleContext)) {
       throw new IllegalStateException();
     }
     String moduleName =
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
index 623bde4dd1..cbf98c928a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
@@ -75,6 +75,7 @@ import com.google.devtools.build.lib.rules.cpp.CppCompilationContext;
 import com.google.devtools.build.lib.rules.cpp.CppModuleMap;
 import com.google.devtools.build.lib.rules.cpp.CppRunfilesProvider;
 import com.google.devtools.build.lib.rules.cpp.LinkerInputs;
+import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
 import com.google.devtools.build.lib.util.Preconditions;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -607,6 +608,34 @@ public final class ObjcCommon {
     return Optional.absent();
   }
 
+  /**
+   * Returns effective compilation options that do not arise from the crosstool.
+   */
+  static Iterable<String> getNonCrosstoolCopts(RuleContext ruleContext) {
+    return Iterables.concat(
+        ruleContext.getFragment(ObjcConfiguration.class).getCopts(),
+        ruleContext.getTokenizedStringListAttr("copts"));
+  }
+
+  static boolean shouldUseObjcModules(RuleContext ruleContext) {
+    for (String copt : getNonCrosstoolCopts(ruleContext)) {
+      if (copt.contains("-fmodules")) {
+        return true;
+      }
+    }
+
+    if (ruleContext.attributes().has("enable_modules", Type.BOOLEAN)
+        && ruleContext.attributes().get("enable_modules", Type.BOOLEAN)) {
+      return true;
+    }
+
+    if (ruleContext.getFragment(ObjcConfiguration.class).moduleMapsEnabled()) {
+      return true;
+    }
+
+    return false;
+  }
+
   static ImmutableList<PathFragment> userHeaderSearchPaths(BuildConfiguration configuration) {
     return ImmutableList.of(new PathFragment("."), configuration.getGenfilesFragment());
   }