Add support for clang-cl-style compiler command lines when scanning i…

…ncludes

RELNOTES: Improve include scanner support for cl.exe and clang-cl command lines
PiperOrigin-RevId: 309271013

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.rules.cpp;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Ascii;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableCollection;
@@ -745,13 +746,28 @@ public List<PathFragment> getQuoteIncludeDirs() {
     return result.build();
   }
 
+  private static boolean startsWithIgnoreCase(String s, String prefix) {
+    return s.length() >= prefix.length()
+        && Ascii.equalsIgnoreCase(s.substring(0, prefix.length()), prefix);
+  }
+
   @Override
   public List<PathFragment> getIncludeDirs() {
     ImmutableList.Builder<PathFragment> result = ImmutableList.builder();
     result.addAll(ccCompilationContext.getIncludeDirs());
     for (String opt : compileCommandLine.getCopts()) {
-      if (opt.startsWith("-I") && opt.length() > 2) {
+      if (startsWithIgnoreCase(opt, "-I") || startsWithIgnoreCase(opt, "/I")) {
         // We insist on the combined form "-Idir".
+        String includeDir = opt.substring(2);
+        if (includeDir.isEmpty()) {
+          continue;
+        }
+        if (startsWithIgnoreCase(includeDir, "msvc")
+            || startsWithIgnoreCase(includeDir, "quote")
+            || startsWithIgnoreCase(includeDir, "system")) {
+          // This is actually a "-iquote", "-isystem", or "-imsvc"; it's handled elsewhere.
+          continue;
+        }
         result.add(PathFragment.create(opt.substring(2)));
       }
     }
@@ -769,24 +785,33 @@ List<PathFragment> getSystemIncludeDirs() throws CommandLineExpansionException {
   }
 
   private List<PathFragment> getSystemIncludeDirs(List<String> compilerOptions) {
-    // TODO(bazel-team): parsing the command line flags here couples us to gcc-style compiler
-    // command lines; use a different way to specify system includes (for example through a
+    // TODO(bazel-team): parsing the command line flags here couples us to gcc- and clang-cl-style
+    // compiler command lines; use a different way to specify system includes (for example through a
     // system_includes attribute in cc_toolchain); note that that would disallow users from
     // specifying system include paths via the copts attribute.
     // Currently, this works together with the include_paths features because getCommandLine() will
     // get the system include paths from the {@code CcCompilationContext} instead.
     ImmutableList.Builder<PathFragment> result = ImmutableList.builder();
     for (int i = 0; i < compilerOptions.size(); i++) {
       String opt = compilerOptions.get(i);
+      String systemIncludeFlag = null;
       if (opt.startsWith("-isystem")) {
-        if (opt.length() > 8) {
-          result.add(PathFragment.create(opt.substring(8).trim()));
-        } else if (i + 1 < compilerOptions.size()) {
-          i++;
-          result.add(PathFragment.create(compilerOptions.get(i)));
-        } else {
-          System.err.println("WARNING: dangling -isystem flag in options for " + prettyPrint());
-        }
+        systemIncludeFlag = "-isystem";
+      } else if (startsWithIgnoreCase(opt, "-imsvc") || startsWithIgnoreCase(opt, "/imsvc")) {
+        systemIncludeFlag = opt.substring(0, 6);
+      }
+      if (systemIncludeFlag == null) {
+        continue;
+      }
+
+      if (opt.length() > systemIncludeFlag.length()) {
+        result.add(PathFragment.create(opt.substring(systemIncludeFlag.length()).trim()));
+      } else if (i + 1 < compilerOptions.size()) {
+        i++;
+        result.add(PathFragment.create(compilerOptions.get(i)));
+      } else {
+        System.err.println(
+            "WARNING: dangling " + systemIncludeFlag + " flag in options for " + prettyPrint());
       }
     }
     return result.build();
@@ -798,6 +823,12 @@ private List<String> getCmdlineIncludes(List<String> args) {
       String arg = argi.next();
       if (arg.equals("-include") && argi.hasNext()) {
         cmdlineIncludes.add(argi.next());
+      } else if (arg.startsWith("-FI") || arg.startsWith("/FI")) {
+        if (arg.length() > 3) {
+          cmdlineIncludes.add(arg.substring(3).trim());
+        } else if (argi.hasNext()) {
+          cmdlineIncludes.add(argi.next());
+        }
       }
     }
     return cmdlineIncludes.build();

src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java

@@ -1056,6 +1056,44 @@ public void testCompilationParameterFile() throws Exception {
         .containsExactly("/usr/bin/mock-gcc", "@/k8-fastbuild/bin/a/_objs/foo/foo.o.params");
   }
 
+  @Test
+  public void testClangClParameters() throws Exception {
+    AnalysisMock.get()
+        .ccSupport()
+        .setupCcToolchainConfig(
+            mockToolsConfig,
+            CcToolchainConfig.builder()
+                .withFeatures(
+                    CppRuleClasses.TARGETS_WINDOWS,
+                    CppRuleClasses.COPY_DYNAMIC_LIBRARIES_TO_BINARY));
+    scratch.file(
+        "a/BUILD",
+        "cc_library(",
+        "    name='foo',",
+        "    srcs=['foo.cc'],",
+        "    copts=[",
+        "        '/imsvc', 'SYSTEM_INCLUDE_1',",
+        "        '-imsvcSYSTEM_INCLUDE_2',",
+        "        '/ISTANDARD_INCLUDE',",
+        "        '/FI', 'forced_include_1',",
+        "        '-FIforced_include_2',",
+        "    ],",
+        ")");
+    CppCompileAction cppCompileAction = getCppCompileAction("//a:foo");
+
+    PathFragment systemInclude1 = PathFragment.create("SYSTEM_INCLUDE_1");
+    PathFragment systemInclude2 = PathFragment.create("SYSTEM_INCLUDE_2");
+    PathFragment standardInclude = PathFragment.create("STANDARD_INCLUDE");
+
+    assertThat(cppCompileAction.getSystemIncludeDirs()).contains(systemInclude1);
+    assertThat(cppCompileAction.getSystemIncludeDirs()).contains(systemInclude2);
+    assertThat(cppCompileAction.getSystemIncludeDirs()).doesNotContain(standardInclude);
+
+    assertThat(cppCompileAction.getIncludeDirs()).doesNotContain(systemInclude1);
+    assertThat(cppCompileAction.getIncludeDirs()).doesNotContain(systemInclude2);
+    assertThat(cppCompileAction.getIncludeDirs()).contains(standardInclude);
+  }
+
   @Test
   public void testCcLibraryLoadedThroughMacro() throws Exception {
     setupTestCcLibraryLoadedThroughMacro(/* loadMacro= */ true);