[PATCH] D98824: [Tooling] Handle compilation databases with clang-cl commands generated by CMake 3.19+

[Janusz Nykiel via Phabricator via cfe-commits]

jnykiel created this revision.
jnykiel added a reviewer: aganea.
Herald added subscribers: usaxena95, kadircet, abidh.
jnykiel requested review of this revision.
Herald added subscribers: cfe-commits, ilya-biryukov.
Herald added a project: clang.

As of CMake commit https://gitlab.kitware.com/cmake/cmake/-/commit/d993ebd4,
which first appeared in CMake 3.19.x series, in the compile commands for clang-cl,
CMake puts `--` before the input file. When operating on such a database, the
`InterpolatingCompilationDatabase` - specifically, the `TransferableCommand`
constructor - does not recognize that pattern and so, does not strip the input,
or the double dash when 'transferring' the compile command. This results in a
incorrect compile command - with the double dash and old input file left in, and
the language options and new input file appended after them, where they're all
treated as inputs, including the language version option.

Test files for some tests have names similar enough to be matched to commands
from the database, e.g.:

`.../path-mappings.test.tmp/server/bar.cpp`

can be matched to:

`.../Driver/ToolChains/BareMetal.cpp`

etc. When that happens, the tool being tested tries to use the matched, and
incorrectly 'transferred' compile command, and fails, reporting errors similar
to:

`error: no such file or directory: '/std:c++14'; did you mean '/std:c++14'? [clang-diagnostic-error]`

This happens in at least 4 tests:

  Clang Tools :: clang-tidy/checkers/performance-trivially-destructible.cpp
  Clangd :: check-fail.test
  Clangd :: check.test
  Clangd :: path-mappings.test


The fix assumes that everything after the '--' is an input file and does not
include it in the 'transferred' command.

[1] https://docs.microsoft.com/en-us/visualstudio/releases/2019/release-notes#summary-of-whats-new-in-this-release-of-visual-studio-2019-version-1690


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98824

Files:
  clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===================================================================
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -812,6 +812,12 @@
   EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
 }
 
+TEST_F(InterpolateTest, StripDoubleDash) {
+  add("dir/foo.cpp", "-Wall -- dir/foo.cpp");
+  // everything after -- is stripped
+  EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+}
+
 TEST_F(InterpolateTest, Case) {
   add("FOO/BAR/BAZ/SHOUT.cc");
   add("foo/bar/baz/quiet.cc");
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -177,6 +177,11 @@
                            Opt.matches(OPT__SLASH_Fo))))
         continue;
 
+      // ...including when the inputs are passed after --
+      if (Opt.matches(OPT__DASH_DASH)) {
+        continue;
+      }
+
       // Strip -x, but record the overridden language.
       if (const auto GivenType = tryParseTypeArg(*Arg)) {
         Type = *GivenType;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98824.331395.patch
Type: text/x-patch
Size: 1260 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210317/de31a1c2/attachment.bin>