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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks for looking into this!

we also had a related report about this in clangd <https://github.com/clangd/clangd/issues/632>, but didn't have time to dive into it. this is definitely fixing an issue in interpolating compilation database, but unfortunately there are still issues lurking around in clangd and command line adjusters.

- any adjuster inserting command line arguments should insert before `--` if exists, and any adjuster stripping flags should not strip anything after a `--`. these can be found in https://github.com/llvm/llvm-project/tree/main/clang/lib/Tooling/ArgumentsAdjusters.cpp
- for clangd we need to ensure resource-dir and sysroot related flags are inserted before `--`, if exists in https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/CompileCommands.cpp#L211
- insertion of extra flags should happen after `--` in https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/ConfigCompile.cpp#L271
- we should again stop stripping flags after `--` in https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/CompileCommands.cpp#L478

do you mind fixing those cases too? if not i can also make some follow-ups, but as-is this isn't really useful to tools, at least without the adjuster fixes i mentioned in the first item(as they are used by almost all clang-based tools), the rest is specific to clangd.



================
Comment at: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:182
+      if (Opt.matches(OPT__DASH_DASH)) {
+        continue;
+      }
----------------
isn't everything after double dash treated as inputs? i think this should be a `break` instead of a `continue`.


================
Comment at: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp:243
     }
     Result.CommandLine.push_back(std::string(Filename));
     return Result;
----------------
i think it actually makes sense to insert a double dash here too, if the filename starts with a `/` or `-` they might actually be treated as compile flags rather than filenames.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98824/new/

https://reviews.llvm.org/D98824