Add support for clang-cl-style compiler command lines when scanning includes
RELNOTES: Improve include scanner support for cl.exe and clang-cl command lines
PiperOrigin-RevId: 309271013
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 4434123..ea82fa7 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
@@ -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 @@
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,8 +785,8 @@
}
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
@@ -778,15 +794,24 @@
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 @@
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();
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
index 9cd9ef5..cf8e1d3 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcCommonTest.java
@@ -1057,6 +1057,44 @@
}
@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);
assertThat(getConfiguredTarget("//a:a")).isNotNull();
