Parse /showIncludes output for MSVC compiler

Instead of parsing .d file generated by wrapper script,
we directly parse the output of /showIncludes option.

Change-Id: Id94e20a5cb05a494a793fd6a43756d44d27cea8a
PiperOrigin-RevId: 154161939

7 files changed, 192 insertions, 48 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 2593881203..8a59e04b90 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
@@ -1139,6 +1139,13 @@ public class CppCompileAction extends AbstractAction
 
     Executor executor = actionExecutionContext.getExecutor();
     CppCompileActionContext.Reply reply;
+    ShowIncludesFilter showIncludesFilter = null;
+    // If parse_showincludes feature is enabled, instead of parsing dotD file we parse the output of
+    // cl.exe caused by /showIncludes option.
+    if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+      showIncludesFilter = new ShowIncludesFilter(getSourceFile().getFilename());
+      actionExecutionContext.getFileOutErr().setOutputFilter(showIncludesFilter);
+    }
     try {
       reply = executor.getContext(actionContext).execWithReply(this, actionExecutionContext);
     } catch (ExecException e) {
@@ -1151,8 +1158,15 @@ public class CppCompileAction extends AbstractAction
     IncludeScanningContext scanningContext = executor.getContext(IncludeScanningContext.class);
     Path execRoot = executor.getExecRoot();
 
-    NestedSet<Artifact> discoveredInputs =
-        discoverInputsFromDotdFiles(execRoot, scanningContext.getArtifactResolver(), reply);
+    NestedSet<Artifact> discoveredInputs;
+    if (showIncludesFilter != null) {
+      discoveredInputs =
+          discoverInputsFromShowIncludesFilter(
+              execRoot, scanningContext.getArtifactResolver(), showIncludesFilter);
+    } else {
+      discoveredInputs =
+          discoverInputsFromDotdFiles(execRoot, scanningContext.getArtifactResolver(), reply);
+    }
     reply = null; // Clear in-memory .d files early.
 
     // Post-execute "include scanning", which modifies the action inputs to match what the compile
@@ -1171,6 +1185,29 @@ public class CppCompileAction extends AbstractAction
   }
 
   @VisibleForTesting
+  public NestedSet<Artifact> discoverInputsFromShowIncludesFilter(
+      Path execRoot, ArtifactResolver artifactResolver, ShowIncludesFilter showIncludesFilter)
+      throws ActionExecutionException {
+    if (!cppSemantics.needsDotdInputPruning()) {
+      return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+    }
+    HeaderDiscovery.Builder discoveryBuilder =
+        new HeaderDiscovery.Builder()
+            .setAction(this)
+            .setSourceFile(getSourceFile())
+            .setSpecialInputsHandler(specialInputsHandler)
+            .setDependencies(showIncludesFilter.getDependencies(execRoot))
+            .setPermittedSystemIncludePrefixes(getPermittedSystemIncludePrefixes(execRoot))
+            .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap());
+
+    if (cppSemantics.needsIncludeValidation()) {
+      discoveryBuilder.shouldValidateInclusions();
+    }
+
+    return discoveryBuilder.build().discoverInputsFromDependencies(execRoot, artifactResolver);
+  }
+
+  @VisibleForTesting
   public NestedSet<Artifact> discoverInputsFromDotdFiles(
       Path execRoot, ArtifactResolver artifactResolver, Reply reply)
       throws ActionExecutionException {
@@ -1180,10 +1217,9 @@ public class CppCompileAction extends AbstractAction
     HeaderDiscovery.Builder discoveryBuilder =
         new HeaderDiscovery.Builder()
             .setAction(this)
-            .setDotdFile(getDotdFile())
             .setSourceFile(getSourceFile())
             .setSpecialInputsHandler(specialInputsHandler)
-            .setDependencySet(processDepset(execRoot, reply))
+            .setDependencies(processDepset(execRoot, reply).getDependencies())
             .setPermittedSystemIncludePrefixes(getPermittedSystemIncludePrefixes(execRoot))
             .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap());
 
@@ -1191,7 +1227,7 @@ public class CppCompileAction extends AbstractAction
       discoveryBuilder.shouldValidateInclusions();
     }
 
-    return discoveryBuilder.build().discoverInputsFromDotdFiles(execRoot, artifactResolver);
+    return discoveryBuilder.build().discoverInputsFromDependencies(execRoot, artifactResolver);
   }
 
   public DependencySet processDepset(Path execRoot, Reply reply) throws ActionExecutionException {
@@ -1331,8 +1367,7 @@ public class CppCompileAction extends AbstractAction
         CcToolchainFeatures.Variables variables,
         String actionName) {
       this.sourceFile = Preconditions.checkNotNull(sourceFile);
-      this.dotdFile = CppFileTypes.mustProduceDotdFile(sourceFile)
-                      ? Preconditions.checkNotNull(dotdFile) : null;
+      this.dotdFile = isGenerateDotdFile(sourceFile) ? Preconditions.checkNotNull(dotdFile) : null;
       this.copts = Preconditions.checkNotNull(copts);
       this.coptsFilter = coptsFilter;
       this.features = Preconditions.checkNotNull(features);
@@ -1340,6 +1375,12 @@ public class CppCompileAction extends AbstractAction
       this.actionName = actionName;
     }
 
+    /** Returns true if Dotd file should be generated. */
+    private boolean isGenerateDotdFile(Artifact sourceArtifact) {
+      return CppFileTypes.headerDiscoveryRequired(sourceArtifact)
+          && !featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
+    }
+
     /**
      * Returns the environment variables that should be set for C++ compile actions.
      */
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
index eb0de2cd29..597ebed521 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
@@ -185,7 +185,7 @@ public final class CppFileTypes {
         }
       };
 
-  public static final boolean mustProduceDotdFile(Artifact source) {
+  public static final boolean headerDiscoveryRequired(Artifact source) {
     // Sources from TreeArtifacts and TreeFileArtifacts will not generate dotd file.
     if (source.isTreeArtifact() || source.hasParent()) {
       return false;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
index c3702dbc0e..1495309af2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java
@@ -409,8 +409,8 @@ public final class CppModel {
       buildVariables.addStringVariable("output_object_file", realOutputFilePath);
     }
 
-    DotdFile dotdFile = CppFileTypes.mustProduceDotdFile(sourceFile)
-        ? Preconditions.checkNotNull(builder.getDotdFile()) : null;
+    DotdFile dotdFile =
+        isGenerateDotdFile(sourceFile) ? Preconditions.checkNotNull(builder.getDotdFile()) : null;
     // Set dependency_file to enable <object>.d file generation.
     if (dotdFile != null) {
       buildVariables.addStringVariable(
@@ -496,6 +496,12 @@ public final class CppModel {
     CcToolchainFeatures.Variables variables = buildVariables.build();
     builder.setVariables(variables);
   }
+
+  /** Returns true if Dotd file should be generated. */
+  private boolean isGenerateDotdFile(Artifact sourceArtifact) {
+    return CppFileTypes.headerDiscoveryRequired(sourceArtifact)
+        && !featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
+  }
   
   /**
    * Constructs the C++ compiler actions. It generally creates one action for every specified source
@@ -533,8 +539,8 @@ public final class CppModel {
       if (!sourceArtifact.isTreeArtifact()) {
         switch (source.getType()) {
           case HEADER:
-            createHeaderAction(outputName, result, env, builder,
-                CppFileTypes.mustProduceDotdFile(sourceArtifact));
+            createHeaderAction(
+                outputName, result, env, builder, isGenerateDotdFile(sourceArtifact));
             break;
           case CLIF_INPUT_PROTO:
             createClifMatchAction(outputName, result, env, builder);
@@ -557,7 +563,7 @@ public final class CppModel {
                 // output (since it isn't generating a native object with debug
                 // info). In that case the LTOBackendAction will generate the dwo.
                 /*generateDwo=*/ cppConfiguration.useFission() && !bitcodeOutput,
-                CppFileTypes.mustProduceDotdFile(sourceArtifact),
+                isGenerateDotdFile(sourceArtifact),
                 source.getBuildVariables());
             break;
         }
@@ -635,10 +641,7 @@ public final class CppModel {
     builder.setSemantics(semantics);
     builder.setPicMode(pic);
     builder.setOutputs(
-        ruleContext,
-        ArtifactCategory.OBJECT_FILE,
-        outputName,
-        CppFileTypes.mustProduceDotdFile(module));
+        ruleContext, ArtifactCategory.OBJECT_FILE, outputName, isGenerateDotdFile(module));
     PathFragment ccRelativeName = semantics.getEffectiveSourcePath(module);
 
     String gcnoFileName =
@@ -708,7 +711,7 @@ public final class CppModel {
         /*addObject=*/ false,
         /*enableCoverage=*/ false,
         /*generateDwo=*/ false,
-        CppFileTypes.mustProduceDotdFile(moduleMapArtifact),
+        isGenerateDotdFile(moduleMapArtifact),
         ImmutableMap.<String, String>of());
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
index 502ed9ffea..04c3368cc2 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java
@@ -285,6 +285,9 @@ public class CppRuleClasses {
    */
   public static final String GENERATE_PDB_FILE = "generate_pdb_file";
 
+  /** A string constant for /showIncludes parsing feature, should only be used for MSVC toolchain */
+  public static final String PARSE_SHOWINCLUDES = "parse_showincludes";
+
   /*
    * A string constant for the fdo_instrument feature.
    */
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 720c9d24a6..f35954589d 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
@@ -156,10 +156,9 @@ public class FakeCppCompileAction extends CppCompileAction {
       HeaderDiscovery.Builder discoveryBuilder =
           new HeaderDiscovery.Builder()
               .setAction(this)
-              .setDotdFile(getDotdFile())
               .setSourceFile(getSourceFile())
               .setSpecialInputsHandler(specialInputsHandler)
-              .setDependencySet(processDepset(execRoot, reply))
+              .setDependencies(processDepset(execRoot, reply).getDependencies())
               .setPermittedSystemIncludePrefixes(getPermittedSystemIncludePrefixes(execRoot))
               .setAllowedDerivedinputsMap(getAllowedDerivedInputsMap());
 
@@ -170,7 +169,7 @@ public class FakeCppCompileAction extends CppCompileAction {
       discoveredInputs =
           discoveryBuilder
               .build()
-              .discoverInputsFromDotdFiles(execRoot, scanningContext.getArtifactResolver());
+              .discoverInputsFromDependencies(execRoot, scanningContext.getArtifactResolver());
     }
      
     reply = null; // Clear in-memory .d files early.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
index 5ae21d1b03..8072e560d7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/HeaderDiscovery.java
@@ -26,16 +26,18 @@ import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
-import com.google.devtools.build.lib.rules.cpp.CppCompileAction.DotdFile;
 import com.google.devtools.build.lib.rules.cpp.CppCompileAction.SpecialInputsHandler;
-import com.google.devtools.build.lib.util.DependencySet;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
-/** Manages the process of obtaining inputs used in a compilation from .d files. */
+/**
+ * Manages the process of obtaining inputs used in a compilation from a dependency set parsed from
+ * either .d files or /showIncludes output.
+ */
 public class HeaderDiscovery {
 
   /** Indicates if a compile should perform dotd pruning. */
@@ -46,12 +48,11 @@ public class HeaderDiscovery {
   
   private final Action action;
   private final Artifact sourceFile;
-  private final DotdFile dotdFile;
 
   private final SpecialInputsHandler specialInputsHandler;
   private final boolean shouldValidateInclusions;
 
-  private final DependencySet depSet;
+  private final Collection<Path> dependencies;
   private final List<Path> permittedSystemIncludePrefixes;
   private final Map<PathFragment, Artifact> allowedDerivedInputsMap;
   
@@ -60,32 +61,30 @@ public class HeaderDiscovery {
    *
    * @param action the action instance requiring header discovery
    * @param sourceFile the source file for the compile
-   * @param dotdFile the .d file used for header discovery
    * @param specialInputsHandler the SpecialInputsHandler for the build
    * @param shouldValidateInclusions true if include validation should be performed
    */
   public HeaderDiscovery(
       Action action,
       Artifact sourceFile,
-      DotdFile dotdFile,
       SpecialInputsHandler specialInputsHandler,
       boolean shouldValidateInclusions,
-      DependencySet depSet,
+      Collection<Path> dependencies,
       List<Path> permittedSystemIncludePrefixes,
       Map<PathFragment, Artifact> allowedDerivedInputsMap) {
     this.action = Preconditions.checkNotNull(action);
     this.sourceFile = Preconditions.checkNotNull(sourceFile);
-    this.dotdFile = Preconditions.checkNotNull(dotdFile);
     this.specialInputsHandler = specialInputsHandler;
     this.shouldValidateInclusions = shouldValidateInclusions;
-    this.depSet = depSet;
+    this.dependencies = dependencies;
     this.permittedSystemIncludePrefixes = permittedSystemIncludePrefixes;
     this.allowedDerivedInputsMap = allowedDerivedInputsMap;
   }
 
   /**
    * Returns a collection with additional input artifacts relevant to the action by reading the
-   * dynamically-discovered dependency information from the .d file after the action has run.
+   * dynamically-discovered dependency information from the parsed dependency set after the action
+   * has run.
    *
    * <p>Artifacts are considered inputs but not "mandatory" inputs.
    *
@@ -94,17 +93,17 @@ public class HeaderDiscovery {
    */
   @VisibleForTesting
   @ThreadCompatible
-  public NestedSet<Artifact> discoverInputsFromDotdFiles(
+  public NestedSet<Artifact> discoverInputsFromDependencies(
       Path execRoot, ArtifactResolver artifactResolver) throws ActionExecutionException {
     NestedSetBuilder<Artifact> inputs = NestedSetBuilder.stableOrder();
-    if (dotdFile == null) {
+    if (dependencies == null) {
       return inputs.build();
     }
     List<Path> systemIncludePrefixes = permittedSystemIncludePrefixes;
 
     // Check inclusions.
     IncludeProblems problems = new IncludeProblems();
-    for (Path execPath : depSet.getDependencies()) {
+    for (Path execPath : dependencies) {
       PathFragment execPathFragment = execPath.asFragment();
       if (execPathFragment.isAbsolute()) {
         // Absolute includes from system paths are ignored.
@@ -157,11 +156,10 @@ public class HeaderDiscovery {
   public static class Builder {
     private Action action;
     private Artifact sourceFile;
-    private DotdFile dotdFile;
     private SpecialInputsHandler specialInputsHandler;
     private boolean shouldValidateInclusions = false;
 
-    private DependencySet depSet;
+    private Collection<Path> dependencies;
     private List<Path> permittedSystemIncludePrefixes;
     private Map<PathFragment, Artifact> allowedDerivedInputsMap;
 
@@ -177,12 +175,6 @@ public class HeaderDiscovery {
       return this;
     }
 
-    /** Sets the dotd file to be used to discover inputs. */
-    public Builder setDotdFile(DotdFile dotdFile) {
-      this.dotdFile = dotdFile;
-      return this;
-    }
-
     /** Sets the SpecialInputsHandler for inputs to this build. */
     public Builder setSpecialInputsHandler(SpecialInputsHandler specialInputsHandler) {
       this.specialInputsHandler = specialInputsHandler;
@@ -195,9 +187,9 @@ public class HeaderDiscovery {
       return this;
     }
 
-    /** Sets the DependencySet capturing used headers by this compile. */
-    public Builder setDependencySet(DependencySet depSet) {
-      this.depSet = depSet;
+    /** Sets the dependencies capturing used headers by this compile. */
+    public Builder setDependencies(Collection<Path> dependencies) {
+      this.dependencies = dependencies;
       return this;
     }
 
@@ -218,10 +210,9 @@ public class HeaderDiscovery {
       return new HeaderDiscovery(
           action,
           sourceFile,
-          dotdFile,
           specialInputsHandler,
           shouldValidateInclusions,
-          depSet,
+          dependencies,
           permittedSystemIncludePrefixes,
           allowedDerivedInputsMap);
     }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java
new file mode 100644
index 0000000000..14b1417191
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java
@@ -0,0 +1,107 @@
+// 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.util.io.FileOutErr;
+import com.google.devtools.build.lib.vfs.Path;
+import java.io.ByteArrayOutputStream;
+import java.io.FilterOutputStream;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+
+/**
+ * A Class for filtering the output of /showIncludes from MSVC compiler.
+ *
+ * <p>A discovered header file will be printed with prefix "Note: including file:", the path is
+ * collected, and the line is suppressed from the actual output users can see.
+ *
+ * <p>Also suppress the basename of source file, which is printed unconditionally by MSVC compiler,
+ * there is no way to turn it off.
+ */
+public class ShowIncludesFilter implements FileOutErr.OutputFilter {
+
+  private FilterShowIncludesOutputStream filterShowIncludesOutputStream;
+  private final String sourceFileName;
+
+  public ShowIncludesFilter(String sourceFileName) {
+    this.sourceFileName = sourceFileName;
+  }
+
+  /**
+   * Use this class to filter and collect the headers discovered by MSVC compiler, also filter out
+   * the source file name printed unconditionally by the compiler.
+   */
+  public static class FilterShowIncludesOutputStream extends FilterOutputStream {
+
+    private final ByteArrayOutputStream buffer = new ByteArrayOutputStream(4096);
+    private final Collection<String> dependencies = new ArrayList<>();
+    private static final int NEWLINE = '\n';
+    private static final String SHOW_INCLUDES_PREFIX = "Note: including file:";
+    private final String sourceFileName;
+
+    public FilterShowIncludesOutputStream(OutputStream out, String sourceFileName) {
+      super(out);
+      this.sourceFileName = sourceFileName;
+    }
+
+    @Override
+    public void write(int b) throws IOException {
+      buffer.write(b);
+      if (b == NEWLINE) {
+        String line = buffer.toString(StandardCharsets.UTF_8.name());
+        if (line.startsWith(SHOW_INCLUDES_PREFIX)) {
+          dependencies.add(line.substring(SHOW_INCLUDES_PREFIX.length()).trim());
+        } else if (!line.trim().equals(sourceFileName)) {
+          buffer.writeTo(out);
+        }
+        buffer.reset();
+      }
+    }
+
+    @Override
+    public void flush() throws IOException {
+      String line = buffer.toString(StandardCharsets.UTF_8.name());
+      if (!line.startsWith(SHOW_INCLUDES_PREFIX) && !line.startsWith(sourceFileName)) {
+        buffer.writeTo(out);
+      }
+      out.flush();
+    }
+
+    public Collection<String> getDependencies() {
+      return this.dependencies;
+    }
+  }
+
+  @Override
+  public FilterOutputStream getFilteredOutputStream(OutputStream outputStream) {
+    filterShowIncludesOutputStream =
+        new FilterShowIncludesOutputStream(outputStream, sourceFileName);
+    return filterShowIncludesOutputStream;
+  }
+
+  public Collection<Path> getDependencies(Path root) {
+    Collection<Path> dependenciesInPath = new ArrayList<>();
+    if (filterShowIncludesOutputStream != null) {
+      for (String dep : filterShowIncludesOutputStream.getDependencies()) {
+        dependenciesInPath.add(root.getRelative(dep));
+      }
+    }
+    return Collections.unmodifiableCollection(dependenciesInPath);
+  }
+}