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

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

Reply via email to