[clang-tools-extra] [clangd] Expand response files before CDB interpolation (PR #75753)

Nathan Ridge via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 18 01:13:50 PST 2023


https://github.com/HighCommander4 requested changes to this pull request.

Thanks for having a look at this!

Instead of performing the "expand response files" operation twice for commands coming from a `compile_commands.json` source, what do you think about the following:

 * Restore the call in `parseJSON()` (i.e. what the patch is doing now), **and**
 * Move the call in `CommandMangler` to a place that is only used for commands coming from the LSP, such as around [here](https://searchfox.org/llvm/rev/b6cce87110072a2db19276e042cd40b06285abbc/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp#745)?
   * We could consider abstracting `OverlayCDB::Commands` into a dedicated `tooling::CompilationDatabase` implementation so that we can simply wrap it using `clang::tooling::expandResponseFiles()`, but feel free to leave that for later; a direct call to `tooling::addExpandedResponseFiles`, as we had in the mangler itself, is fine for now.

It would be nice to have a unit test for this regression as well. We have tests exercising `OverlayCDB` in [GlobalCompilationDatabaseTests.cpp](https://searchfox.org/llvm/source/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp), I think that should be suitable.

https://github.com/llvm/llvm-project/pull/75753


More information about the cfe-commits mailing list