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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to