Adds abstractions for include processing to CppSemantics

--
PiperOrigin-RevId: 146694092
MOS_MIGRATED_REVID=146694092

9 files changed, 142 insertions, 43 deletions

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 03e3a42bee..5027335be1 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
@@ -24,7 +24,6 @@ import com.google.common.collect.Sets;
 import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionExecutionException;
-import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.ActionStatusMessage;
 import com.google.devtools.build.lib.actions.Artifact;
@@ -479,8 +478,8 @@ public class CppCompileAction extends AbstractAction
    * and clears the stored list. {@link #prepare} must be called before this method is called, on
    * each action execution.
    */
-  public Iterable<? extends ActionInput> getAdditionalInputs() {
-    Iterable<? extends ActionInput> result = Preconditions.checkNotNull(additionalInputs);
+  public Iterable<Artifact> getAdditionalInputs() {
+    Iterable<Artifact> result = Preconditions.checkNotNull(additionalInputs);
     additionalInputs = null;
     return result;
   }
@@ -506,22 +505,23 @@ public class CppCompileAction extends AbstractAction
       ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException, InterruptedException {
     Executor executor = actionExecutionContext.getExecutor();
-    Collection<Artifact> initialResult = null;
-
-    if (shouldScanIncludes()) {
-      // Switch running status to "analysis".
-      actionExecutionContext.getExecutor().getEventBus()
-          .post(ActionStatusMessage.analysisStrategy(this));
-
-      try {
-        initialResult = executor.getContext(actionContext)
-            .findAdditionalInputs(this, actionExecutionContext);
-      } catch (ExecException e) {
-        throw e.toActionExecutionException(
-            "Include scanning of rule '" + getOwner().getLabel() + "'",
-            executor.getVerboseFailures(),
-            this);
-      }
+    Iterable<Artifact> initialResult;
+
+    actionExecutionContext
+        .getExecutor()
+        .getEventBus()
+        .post(ActionStatusMessage.analysisStrategy(this));
+    try {
+      initialResult =
+          executor
+              .getContext(actionContext)
+              .findAdditionalInputs(
+                  this, actionExecutionContext, cppSemantics.getIncludeProcessing());
+    } catch (ExecException e) {
+      throw e.toActionExecutionException(
+          "Include scanning of rule '" + getOwner().getLabel() + "'",
+          executor.getVerboseFailures(),
+          this);
     }
 
     if (initialResult == null) {
@@ -566,12 +566,8 @@ public class CppCompileAction extends AbstractAction
     // TODO(ulfjack): This only works if include scanning is enabled; the cleanup is in progress,
     // and this needs to be fixed before we can even consider disabling it.
     resolvedInputs = ImmutableList.copyOf(result);
-    if (result.isEmpty()) {
-      result = initialResult;
-    } else {
-      result.addAll(initialResult);
-    }
-    return result;
+    Iterables.addAll(result, initialResult);
+    return Preconditions.checkNotNull(result);
   }
 
   @Override
@@ -1246,9 +1242,15 @@ public class CppCompileAction extends AbstractAction
     // action actually used by incorporating the results of .d file parsing.
     updateActionInputs(discoveredInputs);
 
-    // hdrs_check: This cannot be switched off, because doing so would allow for incorrect builds.
-    validateInclusions(
-        discoveredInputs, actionExecutionContext.getArtifactExpander(), executor.getEventHandler());
+    // hdrs_check: This cannot be switched off for C++ build actions,
+    // because doing so would allow for incorrect builds.
+    // HeadersCheckingMode.NONE should only be used for ObjC build actions.
+    if (cppSemantics.needsIncludeValidation()) {
+      validateInclusions(
+          discoveredInputs,
+          actionExecutionContext.getArtifactExpander(),
+          executor.getEventHandler());
+    }
   }
 
   @VisibleForTesting
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 af5c66eb42..1ab770de45 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
@@ -281,7 +281,11 @@ public class CppCompileActionBuilder {
 
     NestedSet<Artifact> realMandatoryInputs = realMandatoryInputsBuilder.build();
 
-    NestedSet<Artifact> prunableInputs = context.getDeclaredIncludeSrcs();
+    NestedSetBuilder<Artifact> prunableInputBuilder = NestedSetBuilder.stableOrder();
+    prunableInputBuilder.addTransitive(context.getDeclaredIncludeSrcs());
+    prunableInputBuilder.addTransitive(cppSemantics.getAdditionalPrunableIncludes());
+
+    NestedSet<Artifact> prunableInputs = prunableInputBuilder.build();
 
     // Copying the collections is needed to make the builder reusable.
     if (fake) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
index 33b83454b2..192a789c24 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionContext.java
@@ -20,10 +20,7 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.Executor.ActionContext;
 import com.google.devtools.build.lib.actions.ResourceSet;
-
 import java.io.IOException;
-import java.util.Collection;
-
 import javax.annotation.Nullable;
 
 /**
@@ -45,11 +42,12 @@ public interface CppCompileActionContext extends ActionContext {
    * Does include scanning to find the list of files needed to execute the action.
    *
    * <p>Returns null if additional inputs will only be found during action execution, not before.
-   * </p>
    */
   @Nullable
-  public Collection<Artifact> findAdditionalInputs(CppCompileAction action,
-      ActionExecutionContext actionExecutionContext)
+  public Iterable<Artifact> findAdditionalInputs(
+      CppCompileAction action,
+      ActionExecutionContext actionExecutionContext,
+      IncludeProcessing includeProcessing)
       throws ExecException, InterruptedException, ActionExecutionException;
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
index bc0e1ee11e..4f3a9c12eb 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppSemantics.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingMode;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
@@ -49,10 +50,18 @@ public interface CppSemantics {
       RuleContext ruleContext, CppCompilationContext.Builder contextBuilder);
 
   /**
+   * Returns the set of includes which are not mandatory and may be pruned by include processing.
+   */
+  NestedSet<Artifact> getAdditionalPrunableIncludes();
+
+  /**
    * Determines the applicable mode of headers checking for the passed in ruleContext.
    */
   HeadersCheckingMode determineHeadersCheckingMode(RuleContext ruleContext);
 
+  /** Returns the include processing closure, which handles include processing for this build */
+  IncludeProcessing getIncludeProcessing();
+
   /**
    * Returns true iff this build configuration requires inclusion extraction (for include scanning)
    * in the action graph.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeProcessing.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeProcessing.java
new file mode 100644
index 0000000000..e9721a5a3d
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/IncludeProcessing.java
@@ -0,0 +1,31 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.cpp;
+
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScannerSupplier;
+import javax.annotation.Nullable;
+
+/** Used as an interface to thin header inputs to compile actions for C++-like compiles. */
+public interface IncludeProcessing {
+  /** Performs include processing actions and returns the processed set of resulting headers. */
+  Iterable<Artifact> determineAdditionalInputs(
+      @Nullable IncludeScannerSupplier includeScannerSupplier,
+      CppCompileAction action,
+      ActionExecutionContext actionExecutionContext)
+      throws ExecException, InterruptedException;
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/NoProcessing.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/NoProcessing.java
new file mode 100644
index 0000000000..85bfcd9997
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/NoProcessing.java
@@ -0,0 +1,33 @@
+// Copyright 2014 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.cpp;
+
+import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ExecException;
+import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScannerSupplier;
+import javax.annotation.Nullable;
+
+/** Always performs no include processing and returns null. */
+public class NoProcessing implements IncludeProcessing {
+  @Override
+  public Iterable<Artifact> determineAdditionalInputs(
+      @Nullable IncludeScannerSupplier includeScannerSupplier,
+      CppCompileAction action,
+      ActionExecutionContext actionExecutionContext)
+      throws ExecException, InterruptedException {
+    return null;
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
index acf9f26974..905bf75ee0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategy.java
@@ -26,7 +26,6 @@ import com.google.devtools.build.lib.actions.Executor;
 import com.google.devtools.build.lib.actions.ResourceSet;
 import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
-import java.util.Collection;
 
 /**
  * A context for C++ compilation that calls into a {@link SpawnActionContext}.
@@ -57,8 +56,10 @@ public class SpawnGccStrategy implements CppCompileActionContext {
   }
 
   @Override
-  public Collection<Artifact> findAdditionalInputs(
-      CppCompileAction action, ActionExecutionContext actionExecutionContext)
+  public Iterable<Artifact> findAdditionalInputs(
+      CppCompileAction action,
+      ActionExecutionContext actionExecutionContext,
+      IncludeProcessing includeProcessing)
       throws ExecException, InterruptedException {
     return null;
   }
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 ec2a756c98..cc20a9512a 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
@@ -43,8 +43,10 @@ import com.google.devtools.build.lib.rules.cpp.CppHelper;
 import com.google.devtools.build.lib.rules.cpp.CppLinkAction;
 import com.google.devtools.build.lib.rules.cpp.CppLinkActionBuilder;
 import com.google.devtools.build.lib.rules.cpp.CppRuleClasses;
+import com.google.devtools.build.lib.rules.cpp.IncludeProcessing;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkStaticness;
 import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType;
+import com.google.devtools.build.lib.rules.cpp.NoProcessing;
 import com.google.devtools.build.lib.rules.cpp.PrecompiledFiles;
 import com.google.devtools.build.lib.rules.objc.ObjcProvider.Flag;
 import com.google.devtools.build.lib.rules.objc.ObjcVariablesExtension.VariableCategory;
@@ -247,11 +249,14 @@ public class CrosstoolCompilationSupport extends CompilationSupport {
     Collection<Artifact> privateHdrs =
         ImmutableSortedSet.copyOf(compilationArtifacts.getPrivateHdrs());
     Collection<Artifact> publicHdrs = ImmutableSortedSet.copyOf(attributes.hdrs());
+    IncludeProcessing includeProcessing = new NoProcessing();
     CcLibraryHelper result =
         new CcLibraryHelper(
                 ruleContext,
                 new ObjcCppSemantics(
-                    objcProvider, ruleContext.getFragment(ObjcConfiguration.class)),
+                    objcProvider,
+                    includeProcessing,
+                    ruleContext.getFragment(ObjcConfiguration.class)),
                 getFeatureConfiguration(ruleContext),
                 CcLibraryHelper.SourceCategory.CC_AND_OBJC)
             .addSources(arcSources, ImmutableMap.of("objc_arc", ""))
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
index 2259e4f5ef..bb3cd93c0d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java
@@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.rules.objc.ObjcProvider.STATIC_FRAME
 
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.rules.cpp.CppCompilationContext.Builder;
 import com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder;
 import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext;
@@ -28,6 +29,7 @@ import com.google.devtools.build.lib.rules.cpp.CppConfiguration.HeadersCheckingM
 import com.google.devtools.build.lib.rules.cpp.CppHelper;
 import com.google.devtools.build.lib.rules.cpp.CppSemantics;
 import com.google.devtools.build.lib.rules.cpp.HeaderDiscovery.DotdPruningMode;
+import com.google.devtools.build.lib.rules.cpp.IncludeProcessing;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 /**
@@ -35,6 +37,7 @@ import com.google.devtools.build.lib.vfs.PathFragment;
  */
 public class ObjcCppSemantics implements CppSemantics {
 
+  private final IncludeProcessing includeProcessing;
   private final ObjcProvider objcProvider;
   private final ObjcConfiguration config;
 
@@ -44,12 +47,16 @@ public class ObjcCppSemantics implements CppSemantics {
    * @param objcProvider the provider that should be used in determining objc-specific inputs to
    *     actions
    * @param config the ObjcConfiguration for this build
+   * @param includeProcessing the closure providing the strategy for processing of includes for
+   *     actions
    */
-  public ObjcCppSemantics(ObjcProvider objcProvider, ObjcConfiguration config) {
+  public ObjcCppSemantics(
+      ObjcProvider objcProvider, IncludeProcessing includeProcessing, ObjcConfiguration config) {
+    this.includeProcessing = includeProcessing;
     this.objcProvider = objcProvider;
     this.config = config;
   }
-  
+
   @Override
   public PathFragment getEffectiveSourcePath(Artifact source) {
     return source.getRootRelativePath();
@@ -65,7 +72,6 @@ public class ObjcCppSemantics implements CppSemantics {
     actionBuilder.addTransitiveMandatoryInputs(CppHelper.getToolchain(ruleContext).getCrosstool());
     actionBuilder.setShouldScanIncludes(false);
 
-    actionBuilder.addTransitiveMandatoryInputs(objcProvider.get(HEADER));
     actionBuilder.addTransitiveMandatoryInputs(objcProvider.get(STATIC_FRAMEWORK_FILE));
     actionBuilder.addTransitiveMandatoryInputs(objcProvider.get(DYNAMIC_FRAMEWORK_FILE));
   }
@@ -76,6 +82,11 @@ public class ObjcCppSemantics implements CppSemantics {
   }
 
   @Override
+  public NestedSet<Artifact> getAdditionalPrunableIncludes() {
+    return objcProvider.get(HEADER);
+  }
+
+  @Override
   public HeadersCheckingMode determineHeadersCheckingMode(RuleContext ruleContext) {
     // Currently, objc builds do not enforce strict deps.  To begin enforcing strict deps in objc,
     // switch this flag to STRICT.
@@ -83,6 +94,11 @@ public class ObjcCppSemantics implements CppSemantics {
   }
 
   @Override
+  public IncludeProcessing getIncludeProcessing() {
+    return includeProcessing;
+  }
+
+  @Override
   public boolean needsIncludeScanning(RuleContext ruleContext) {
     return false;
   }
@@ -91,7 +107,7 @@ public class ObjcCppSemantics implements CppSemantics {
   public boolean needsDotdInputPruning() {
     return config.getDotdPruningPlan() == DotdPruningMode.USE;
   }
-  
+
   @Override
   public void validateAttributes(RuleContext ruleContext) {
   }