[PATCH] D70222: [clangd] Add support for .rsp files in compile_commands.json

[liu hui via Phabricator via cfe-commits]

lh123 marked an inline comment as done.
lh123 added inline comments.


================
Comment at: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp:167
+    for (auto &Cmd : Cmds) {
+      expandResponseFiles(Cmd, Tokenizer);
+    }
----------------
kadircet wrote:
> so it looks like we already have `ExpandResponseFiles` exposed in `llvm/include/llvm/Support/CommandLine.h` could you make use of it in here instead of re-implementing it again?
> 
> I see that it has a different signature, but we can do the conversion back and forth in here, going from `std::string` to `char*` is cheap anyways, coming back is expensive though, and we can limit that to iff we have seen an argument starting with an `@`. So this would be something like:
> 
> ```
> llvm::SmallVector<const char*, 20> Argvs;
> Argvs.reserve(Cmd.CommandLine.size());
> bool SeenRSPFile = false;
> for(auto &Argv : Cmd.CommandLine) {
>   Argvs.push_back(Argv.c_str());
>   SeenRSPFile |= Argv.front() == '@';
> }
> if(!SeenRSPFile)
>   continue;
> 
> // call ExpandResponseFiles with Argvs, convert back to vector<std::string> and assign to Cmd.CommandLine
> ```
I think it's not easy to reuse `ExpandResponseFiles` without changing code because it uses llvm::sys::fs::current_path to resolve relative paths.

In fact, most of the code here is copied from `CommandLine`


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

https://reviews.llvm.org/D70222