Disable include scanning for assembler-without-preprocessor source.

The scanner only looks for C preprocessor directives, but most assemblers allow '.include' assembly directives, and those aren't found by the scanner.
So skip the include scanner for assembly files that don't want C preprocessing, because correctly declared inclusions are to be preferred anyway.

--
MOS_MIGRATED_REVID=103944189

3 files changed, 29 insertions, 11 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 f6ce80f000..9c292e3ce3 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
@@ -145,6 +145,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
   private final Artifact dwoFile;
   private final Artifact optionalSourceFile;
   private final NestedSet<Artifact> mandatoryInputs;
+  private final boolean shouldScanIncludes;
   private final CppCompilationContext context;
   private final Collection<PathFragment> extraSystemIncludePrefixes;
   private final Iterable<IncludeScannable> lipoScannables;
@@ -181,6 +182,8 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
    *        emitted it
    * @param sourceFile the source file that should be compiled. {@code mandatoryInputs} must
    *        contain this file
+   * @param shouldScanIncludes a boolean indicating whether scanning of {@code sourceFile}
+   *        is to be performed looking for inclusions.
    * @param sourceLabel the label of the rule the source file is generated by
    * @param mandatoryInputs any additional files that need to be present for the
    *        compilation to succeed, can be empty but not null, for example, extra sources for FDO.
@@ -205,6 +208,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
       FeatureConfiguration featureConfiguration,
       CcToolchainFeatures.Variables variables,
       Artifact sourceFile,
+      boolean shouldScanIncludes,
       Label sourceLabel,
       NestedSet<Artifact> mandatoryInputs,
       Artifact outputFile,
@@ -240,9 +244,12 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
     this.extraSystemIncludePrefixes = extraSystemIncludePrefixes;
     this.includeResolver = includeResolver;
     this.cppConfiguration = cppConfiguration;
-    if (cppConfiguration != null && !cppConfiguration.shouldScanIncludes()) {
-      inputsKnown = true;
-    }
+    // inputsKnown begins as the logical negation of shouldScanIncludes.
+    // When scanning includes, the inputs begin as not known, and become
+    // known after inclusion scanning. When *not* scanning includes,
+    // the inputs are as declared, hence known, and remain so.
+    this.shouldScanIncludes = shouldScanIncludes;
+    this.inputsKnown = !shouldScanIncludes;
     this.cppCompileCommandLine =
         new CppCompileCommandLine(
             sourceFile,
@@ -308,7 +315,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
   }
 
   public boolean shouldScanIncludes() {
-    return cppConfiguration.shouldScanIncludes();
+    return shouldScanIncludes;
   }
 
   @Override
@@ -657,7 +664,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
   public void validateInclusions(
       MiddlemanExpander middlemanExpander, EventHandler eventHandler)
       throws ActionExecutionException {
-    if (!cppConfiguration.shouldScanIncludes() || !inputsKnown()) {
+    if (!shouldScanIncludes() || !inputsKnown()) {
       return;
     }
 
@@ -807,7 +814,7 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable
   public final synchronized void updateActionInputs(Path execRoot,
       ArtifactResolver artifactResolver, CppCompileActionContext.Reply reply)
       throws ActionExecutionException {
-    if (!cppConfiguration.shouldScanIncludes()) {
+    if (!shouldScanIncludes()) {
       return;
     }
     inputsKnown = false;
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 861d8f5a0a..e7c1b9b067 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
@@ -248,8 +248,16 @@ public class CppCompileActionBuilder {
     // Configuration can be null in tests.
     NestedSetBuilder<Artifact> realMandatoryInputsBuilder = NestedSetBuilder.compileOrder();
     realMandatoryInputsBuilder.addTransitive(mandatoryInputsBuilder.build());
-    if (tempOutputFile == null && configuration != null
-        && !configuration.getFragment(CppConfiguration.class).shouldScanIncludes()) {
+    String filename = sourceFile.getFilename();
+    // Assembler without C preprocessing can use the '.include' pseudo-op which is not
+    // understood by the include scanner, so we'll disable scanning, and instead require
+    // the declared sources to state (possibly overapproximate) the dependencies.
+    // Assembler with preprocessing can also use '.include', but supporting both kinds
+    // of inclusion for that use-case is ridiculous.
+    boolean shouldScanIncludes = !CppFileTypes.ASSEMBLER.matches(filename)
+        && configuration != null
+        && configuration.getFragment(CppConfiguration.class).shouldScanIncludes();
+    if (tempOutputFile == null && !shouldScanIncludes) {
       realMandatoryInputsBuilder.addTransitive(context.getDeclaredIncludeSrcs());
     }
     realMandatoryInputsBuilder.addTransitive(context.getAdditionalInputs());
@@ -260,7 +268,8 @@ public class CppCompileActionBuilder {
     // Copying the collections is needed to make the builder reusable.
     if (fake) {
       return new FakeCppCompileAction(owner, ImmutableList.copyOf(features), featureConfiguration,
-          variables, sourceFile, sourceLabel, realMandatoryInputsBuilder.build(), outputFile,
+          variables, sourceFile, shouldScanIncludes, sourceLabel,
+          realMandatoryInputsBuilder.build(), outputFile,
           tempOutputFile, dotdFile, configuration, cppConfiguration, context, actionContext,
           ImmutableList.copyOf(copts), ImmutableList.copyOf(pluginOpts),
           getNocoptPredicate(nocopts), extraSystemIncludePrefixes, fdoBuildStamp, ruleContext,
@@ -269,8 +278,8 @@ public class CppCompileActionBuilder {
       NestedSet<Artifact> realMandatoryInputs = realMandatoryInputsBuilder.build();
 
       return new CppCompileAction(owner, ImmutableList.copyOf(features), featureConfiguration,
-          variables, sourceFile, sourceLabel, realMandatoryInputs, outputFile, dotdFile,
-          gcnoFile, getDwoFile(ruleContext, outputFile, cppConfiguration),
+          variables, sourceFile, shouldScanIncludes, sourceLabel, realMandatoryInputs,
+          outputFile, dotdFile, gcnoFile, getDwoFile(ruleContext, outputFile, cppConfiguration),
           optionalSourceFile, configuration, cppConfiguration, context,
           actionContext, ImmutableList.copyOf(copts),
           ImmutableList.copyOf(pluginOpts),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
index cbcb823510..e2446cc127 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java
@@ -65,6 +65,7 @@ public class FakeCppCompileAction extends CppCompileAction {
       FeatureConfiguration featureConfiguration,
       CcToolchainFeatures.Variables variables,
       Artifact sourceFile,
+      boolean shouldScanIncludes,
       Label sourceLabel,
       NestedSet<Artifact> mandatoryInputs,
       Artifact outputFile,
@@ -86,6 +87,7 @@ public class FakeCppCompileAction extends CppCompileAction {
         featureConfiguration,
         variables,
         sourceFile,
+        shouldScanIncludes,
         sourceLabel,
         mandatoryInputs,
         outputFile,