Switch from using set/getPchFile on CompilationArtifacts to a doNotUsePch option on CompilationSupport. In the process correctly respect whether to use the pch file under crosstool compilation mode.

PiperOrigin-RevId: 161156717

8 files changed, 59 insertions, 48 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java
index aa9eb8df03..7c927ae666 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationArtifacts.java
@@ -32,7 +32,6 @@ final class CompilationArtifacts {
     private Iterable<Artifact> additionalHdrs = ImmutableList.of();
     private Iterable<Artifact> privateHdrs = ImmutableList.of();
     private Iterable<Artifact> precompiledSrcs = ImmutableList.of();
-    private Optional<Artifact> pchFile;
     private IntermediateArtifacts intermediateArtifacts;
 
     Builder addSrcs(Iterable<Artifact> srcs) {
@@ -72,13 +71,6 @@ final class CompilationArtifacts {
       return this;
     }
 
-    Builder setPchFile(Optional<Artifact> pchFile) {
-      Preconditions.checkState(this.pchFile == null,
-          "pchFile is already set to: %s", this.pchFile);
-      this.pchFile = Preconditions.checkNotNull(pchFile);
-      return this;
-    }
-
     Builder setIntermediateArtifacts(IntermediateArtifacts intermediateArtifacts) {
       Preconditions.checkState(this.intermediateArtifacts == null,
           "intermediateArtifacts is already set to: %s", this.intermediateArtifacts);
@@ -102,7 +94,7 @@ final class CompilationArtifacts {
         archive = Optional.of(intermediateArtifacts.archive());
       }
       return new CompilationArtifacts(
-          srcs, nonArcSrcs, additionalHdrs, privateHdrs, precompiledSrcs, archive, pchFile);
+          srcs, nonArcSrcs, additionalHdrs, privateHdrs, precompiledSrcs, archive);
     }
   }
 
@@ -112,7 +104,6 @@ final class CompilationArtifacts {
   private final Iterable<Artifact> additionalHdrs;
   private final Iterable<Artifact> privateHdrs;
   private final Iterable<Artifact> precompiledSrcs;
-  private final Optional<Artifact> pchFile;
 
   private CompilationArtifacts(
       Iterable<Artifact> srcs,
@@ -120,15 +111,13 @@ final class CompilationArtifacts {
       Iterable<Artifact> additionalHdrs,
       Iterable<Artifact> privateHdrs,
       Iterable<Artifact> precompiledSrcs,
-      Optional<Artifact> archive,
-      Optional<Artifact> pchFile) {
+      Optional<Artifact> archive) {
     this.srcs = Preconditions.checkNotNull(srcs);
     this.nonArcSrcs = Preconditions.checkNotNull(nonArcSrcs);
     this.additionalHdrs = Preconditions.checkNotNull(additionalHdrs);
     this.privateHdrs = Preconditions.checkNotNull(privateHdrs);
     this.precompiledSrcs = Preconditions.checkNotNull(precompiledSrcs);
     this.archive = Preconditions.checkNotNull(archive);
-    this.pchFile = Preconditions.checkNotNull(pchFile);
   }
 
   public Iterable<Artifact> getSrcs() {
@@ -170,8 +159,4 @@ final class CompilationArtifacts {
     return archive;
   }
 
-  public Optional<Artifact> getPchFile() {
-    return pchFile;
-  }
-
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
index 1fd9ee4d8a..dcaa523911 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java
@@ -259,7 +259,6 @@ public abstract class CompilationSupport {
         .addPrivateHdrs(srcs.filter(HEADERS).list())
         .addPrecompiledSrcs(srcs.filter(PRECOMPILED_SRCS_TYPE).list())
         .setIntermediateArtifacts(intermediateArtifacts)
-        .setPchFile(Optional.fromNullable(ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET)))
         .build();
   }
 
@@ -303,6 +302,7 @@ public abstract class CompilationSupport {
   protected final boolean useDeps;
   protected final Map<String, NestedSet<Artifact>> outputGroupCollector;
   protected final boolean isTestRule;
+  protected final boolean usePch;
 
   /**
    * Creates a new compilation support for the given rule and build configuration.
@@ -324,7 +324,8 @@ public abstract class CompilationSupport {
       CompilationAttributes compilationAttributes,
       boolean useDeps,
       Map<String, NestedSet<Artifact>> outputGroupCollector,
-      boolean isTestRule) {
+      boolean isTestRule,
+      boolean usePch) {
     this.ruleContext = ruleContext;
     this.buildConfiguration = buildConfiguration;
     this.objcConfiguration = buildConfiguration.getFragment(ObjcConfiguration.class);
@@ -334,6 +335,7 @@ public abstract class CompilationSupport {
     this.useDeps = useDeps;
     this.isTestRule = isTestRule;
     this.outputGroupCollector = outputGroupCollector;
+    this.usePch = usePch;
   }
 
   /** Builder for {@link CompilationSupport} */
@@ -346,6 +348,7 @@ public abstract class CompilationSupport {
     private Map<String, NestedSet<Artifact>> outputGroupCollector;
     private boolean isObjcLibrary = false;
     private boolean isTestRule = false;
+    private boolean usePch = true;
 
     /** Sets the {@link RuleContext} for the calling target. */
     public Builder setRuleContext(RuleContext ruleContext) {
@@ -381,6 +384,15 @@ public abstract class CompilationSupport {
     }
 
     /**
+     * Sets that this {@link CompilationSupport} will not use the pch from the rule context in
+     * determining compilation actions.
+     */
+    public Builder doNotUsePch() {
+      this.usePch = false;
+      return this;
+    }
+
+    /**
      * Indicates that this CompilationSupport is for use in an objc_library target. This will cause
      * CrosstoolCompilationSupport to be used if --experimental_objc_crosstool=library
      */
@@ -446,7 +458,8 @@ public abstract class CompilationSupport {
             compilationAttributes,
             useDeps,
             outputGroupCollector,
-            isTestRule);
+            isTestRule,
+            usePch);
       } else {
         return new LegacyCompilationSupport(
             ruleContext,
@@ -455,7 +468,8 @@ public abstract class CompilationSupport {
             compilationAttributes,
             useDeps,
             outputGroupCollector,
-            isTestRule);
+            isTestRule,
+            usePch);
       }
     }
   }
@@ -1211,6 +1225,17 @@ public abstract class CompilationSupport {
     return this;
   }
 
+  protected Optional<Artifact> getPchFile() {
+    if (!usePch) {
+      return Optional.absent();
+    }
+    Artifact pchHdr = null;
+    if (ruleContext.attributes().has("pch", BuildType.LABEL)) {
+      pchHdr = ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET);
+    }
+    return Optional.fromNullable(pchHdr);
+  }
+
   /**
    * Registers an action that will generate a clang module map.
    * @param moduleMap the module map to generate
@@ -1379,7 +1404,7 @@ public abstract class CompilationSupport {
             .addInputs(compilationArtifacts.getPrivateHdrs())
             .addTransitiveInputs(attributes.hdrs())
             .addTransitiveInputs(objcProvider.get(ObjcProvider.HEADER))
-            .addInputs(compilationArtifacts.getPchFile().asSet())
+            .addInputs(getPchFile().asSet())
             .addTransitiveInputs(objcProvider.get(ObjcProvider.STATIC_FRAMEWORK_FILE))
             .addTransitiveInputs(objcProvider.get(ObjcProvider.DYNAMIC_FRAMEWORK_FILE))
             .build(ruleContext));
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java
index d26eec9dbf..2718464fa8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java
@@ -39,7 +39,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions.AppleBitcodeMode;
 import com.google.devtools.build.lib.rules.apple.AppleConfiguration;
@@ -133,7 +132,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
         CompilationAttributes.Builder.fromRuleContext(ruleContext).build(),
         /*useDeps=*/ true,
         outputGroupCollector,
-        /*isTestRule=*/ false);
+        /*isTestRule=*/ false,
+        /*usePch=*/ true);
   }
 
   /**
@@ -144,6 +144,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
    * @param intermediateArtifacts IntermediateArtifacts for deriving artifact paths
    * @param compilationAttributes attributes of the calling target
    * @param useDeps true if deps should be used
+   * @param usePch true if pch should be used
    */
   public CrosstoolCompilationSupport(
       RuleContext ruleContext,
@@ -152,7 +153,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
       CompilationAttributes compilationAttributes,
       boolean useDeps,
       Map<String, NestedSet<Artifact>> outputGroupCollector,
-      boolean isTestRule) {
+      boolean isTestRule,
+      boolean usePch) {
     super(
         ruleContext,
         buildConfiguration,
@@ -160,7 +162,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
         compilationAttributes,
         useDeps,
         outputGroupCollector,
-        isTestRule);
+        isTestRule,
+        usePch);
   }
 
   @Override
@@ -391,10 +394,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
                 Streams.stream(attributes.hdrs()),
                 Streams.stream(compilationArtifacts.getAdditionalHdrs()))
             .collect(toImmutableSortedSet(naturalOrder()));
-    Artifact pchHdr = null;
-    if (ruleContext.attributes().has("pch", BuildType.LABEL)) {
-      pchHdr = ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET);
-    }
+    Artifact pchHdr = getPchFile().orNull();
     ObjcCppSemantics semantics =
         new ObjcCppSemantics(
             objcProvider,
@@ -474,8 +474,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
     if (configuration.getFragment(ObjcConfiguration.class).shouldStripBinary()) {
       activatedCrosstoolSelectables.add(DEAD_STRIP_FEATURE_NAME);
     }
-    if (ruleContext.attributes().has("pch", BuildType.LABEL)
-        && ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET) != null) {
+    if (getPchFile().isPresent()) {
       activatedCrosstoolSelectables.add("pch");
     }
     if (!isTestRule) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
index bd3ecd4309..81b9bfb19d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java
@@ -271,6 +271,7 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
             new CompilationSupport.Builder()
                 .setRuleContext(ruleContext)
                 .setIntermediateArtifacts(ObjcRuleClasses.j2objcIntermediateArtifacts(ruleContext))
+                .doNotUsePch()
                 .build();
 
         compilationSupport
@@ -792,7 +793,6 @@ public class J2ObjcAspect extends NativeAspectClass implements ConfiguredAspectF
       CompilationArtifacts compilationArtifacts = new CompilationArtifacts.Builder()
           .addNonArcSrcs(transpiledSources)
           .setIntermediateArtifacts(intermediateArtifacts)
-          .setPchFile(Optional.<Artifact>absent())
           .addAdditionalHdrs(transpiledHeaders)
           .build();
       builder.setCompilationArtifacts(compilationArtifacts);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibrary.java
index 612e400c60..c4b5e9a361 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcLibrary.java
@@ -18,7 +18,6 @@ import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.JRE_LIBRARY;
 import static com.google.devtools.build.lib.rules.objc.ObjcProvider.LIBRARY;
 
-import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -92,12 +91,12 @@ public class J2ObjcLibrary implements RuleConfiguredTargetFactory {
     CompilationArtifacts moduleMapCompilationArtifacts =
         new CompilationArtifacts.Builder()
             .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
-            .setPchFile(Optional.<Artifact>absent())
             .build();
 
     new CompilationSupport.Builder()
         .setRuleContext(ruleContext)
         .setIntermediateArtifacts(ObjcRuleClasses.intermediateArtifacts(ruleContext))
+        .doNotUsePch()
         .build()
         .registerFullyLinkAction(
             common.getObjcProvider(),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
index 76f3234155..dc6045f7d8 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java
@@ -123,7 +123,6 @@ public class LegacyCompilationSupport extends CompilationSupport {
         .addPrivateHdrs(srcs.filter(HEADERS).list())
         .addPrecompiledSrcs(srcs.filter(PRECOMPILED_SRCS_TYPE).list())
         .setIntermediateArtifacts(intermediateArtifacts)
-        .setPchFile(Optional.fromNullable(ruleContext.getPrerequisiteArtifact("pch", Mode.TARGET)))
         .build();
   }
 
@@ -147,7 +146,8 @@ public class LegacyCompilationSupport extends CompilationSupport {
       CompilationAttributes compilationAttributes,
       boolean useDeps,
       Map<String, NestedSet<Artifact>> outputGroupCollector,
-      boolean isTestRule) {
+      boolean isTestRule,
+      boolean usePch) {
     super(
         ruleContext,
         buildConfiguration,
@@ -155,7 +155,8 @@ public class LegacyCompilationSupport extends CompilationSupport {
         compilationAttributes,
         useDeps,
         outputGroupCollector,
-        isTestRule);
+        isTestRule,
+        usePch);
   }
 
   @Override
@@ -371,7 +372,7 @@ public class LegacyCompilationSupport extends CompilationSupport {
             objcProvider,
             priorityHeaders,
             moduleMap,
-            compilationArtifacts.getPchFile(),
+            getPchFile(),
             Optional.of(dotdFile.artifact()),
             otherFlags,
             runCodeCoverage,
@@ -399,7 +400,7 @@ public class LegacyCompilationSupport extends CompilationSupport {
             .addTransitiveMandatoryInputs(objcProvider.get(STATIC_FRAMEWORK_FILE))
             .addTransitiveMandatoryInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE))
             .setDotdFile(dotdFile)
-            .addMandatoryInputs(compilationArtifacts.getPchFile().asSet());
+            .addMandatoryInputs(getPchFile().asSet());
 
     Artifact headersListFile = null;
     if (isHeaderThinningEnabled()
@@ -456,7 +457,7 @@ public class LegacyCompilationSupport extends CompilationSupport {
             objcProvider,
             priorityHeaders,
             moduleMap,
-            compilationArtifacts.getPchFile(),
+            getPchFile(),
             Optional.<Artifact>absent(),
             otherFlags,
             /* runCodeCoverage=*/ false,
@@ -483,7 +484,7 @@ public class LegacyCompilationSupport extends CompilationSupport {
             .addCommonInputs(compilationArtifacts.getPrivateHdrs())
             .addCommonTransitiveInputs(objcProvider.get(STATIC_FRAMEWORK_FILE))
             .addCommonTransitiveInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE))
-            .addCommonInputs(compilationArtifacts.getPchFile().asSet())
+            .addCommonInputs(getPchFile().asSet())
             .build(ruleContext.getActionOwner()));
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
index 35ba9f9c70..4a1c4bb7c4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java
@@ -172,7 +172,6 @@ final class ProtobufSupport {
     CompilationArtifacts.Builder moduleMapCompilationArtifacts =
         new CompilationArtifacts.Builder()
             .setIntermediateArtifacts(intermediateArtifacts)
-            .setPchFile(Optional.<Artifact>absent())
             .addAdditionalHdrs(getProtobufHeaders())
             .addAdditionalHdrs(
                 getGeneratedProtoOutputs(inputsToOutputsMap.values(), HEADER_SUFFIX));
@@ -181,6 +180,7 @@ final class ProtobufSupport {
         new CompilationSupport.Builder()
             .setRuleContext(ruleContext)
             .setCompilationAttributes(new CompilationAttributes.Builder().build())
+            .doNotUsePch()
             .build();
 
     compilationSupport.registerGenerateModuleMapAction(moduleMapCompilationArtifacts.build());
@@ -211,7 +211,8 @@ final class ProtobufSupport {
               new CompilationAttributes.Builder().build(),
               /*useDeps=*/ false,
               new TreeMap<String, NestedSet<Artifact>>(),
-              /*isTestRule=*/ false)
+              /*isTestRule=*/ false,
+              /*usePch=*/ false)
           .registerCompileAndArchiveActions(common, userHeaderSearchPaths);
 
       actionId++;
@@ -399,7 +400,6 @@ final class ProtobufSupport {
     CompilationArtifacts.Builder compilationArtifacts =
         new CompilationArtifacts.Builder()
             .setIntermediateArtifacts(intermediateArtifacts)
-            .setPchFile(Optional.<Artifact>absent())
             .addAdditionalHdrs(getGeneratedProtoOutputs(filteredInputProtos, HEADER_SUFFIX))
             .addAdditionalHdrs(getProtobufHeaders());
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java
index d3f8bbf085..8139368952 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtocolBuffers2Support.java
@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.rules.objc;
 
-import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
@@ -87,7 +86,11 @@ final class ProtocolBuffers2Support {
   public ProtocolBuffers2Support registerCompilationActions()
       throws RuleErrorException, InterruptedException {
     CompilationSupport compilationSupport =
-        new CompilationSupport.Builder().setRuleContext(ruleContext).doNotUseDeps().build();
+        new CompilationSupport.Builder()
+            .setRuleContext(ruleContext)
+            .doNotUseDeps()
+            .doNotUsePch()
+            .build();
 
     compilationSupport.registerCompileAndArchiveActions(getCommon());
     return this;
@@ -132,7 +135,6 @@ final class ProtocolBuffers2Support {
     Iterable<Artifact> generatedSources = getGeneratedProtoOutputs(getSourceExtension());
     return new CompilationArtifacts.Builder()
         .setIntermediateArtifacts(new IntermediateArtifacts(ruleContext, ""))
-        .setPchFile(Optional.<Artifact>absent())
         .addAdditionalHdrs(getGeneratedProtoOutputs(getHeaderExtension()))
         .addAdditionalHdrs(generatedSources)
         .addNonArcSrcs(generatedSources)