[PATCH] D148663: [RFC][clangd] Use interpolation for CDB pushed via LSP protocol

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 28 06:38:43 PDT 2023


ilya-biryukov added a comment.

In D148663#4301589 <https://reviews.llvm.org/D148663#4301589>, @DmitryPolukhin wrote:

> And, if they do so, this diff will just does nothing because there is an exact match. It starts playing only if client provided partial CDB and inference is required.

(hypothesising below)
I think this depends on a client at question. If I were writing one and had an idea what I want to do for headers, e.g. I might have a project system that knows which build targets headers belong to,
I would rather see no compile errors for a header (if I have bugs in my integration) than for Clangd to kick in with interpolation and silently hide the bug.

>From what I can recollect, your case is different and you just want to "pipe" `compile_commands.json` to Clangd provided by some build system, but without `clangd` actually reading it.
I don't actually write an LSP client, though, would let @kadircet decide which path is preferable.

> Pushing command via LSP might be preferable in comparison with file based approach to avoid race condition with updating the file and clangd reading it

Ideally this should be handled with atomic writes (write to temp file and move to destination), at least on Linux and Mac. I'm not sure if build systems do that, though.
Does Clangd ever re-read the compilation database now? It used to load it only once, so rereading would require a restart of clangd anyway. Race on just one read is very unlikely (although not impossible).
However, the `compile_commands.json` file is parsed lazily, when the command for a particular file is actually requested, if you see crashes or inconsistencies in your scenarios, it highly likely due to this.

> + it works better with build systems that can generate compiles commands on the fly for files and generating all of them in advance may not be possible.

FYI, there is separate support for pushing command per-file in Clangd via an LSP extension <https://github.com/llvm/llvm-project/blob/fae9d4df8fcff802dc7b0c344c61c70cc2a20697/clang-tools-extra/clangd/Protocol.h#L577>.
I think this should cover build systems that generate commands on the fly better, but this also does not use interpolation.

A meta-comment: it would be really useful to understand how you use Clangd a bit better. Would it be hard to write down which kind of client you have and what are requirements for Clangd? Which build systems are at play, how does the client use them, etc?
To validate whether patches like this one are a good fit to solve the problem or there are better ways, it's really useful to have a higher level overview of how Clangd is being used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148663



More information about the cfe-commits mailing list