[PATCH] D49523: [clangd] Add support for per-file override compilation command

Marc-Andre Laperle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jul 20 07:35:09 PDT 2018


malaperle added a comment.

In https://reviews.llvm.org/D49523#1169000, @arphaman wrote:

> In https://reviews.llvm.org/D49523#1167553, @malaperle wrote:
>
> > Interesting! We also have a need for passing compilation commands in a context where there is no compile_commands.json, but we were thinking of putting this in a "didChangeConfiguration" message so that all the commands would be available even before files are opened. This would be allow Clangd to have the necessary information for background indexing which would include unopened files. Subsequent changes to compilation commands would probably go through a similar didChangeConfiguration and the appropriate (opened) files would get reparsed (not unlike https://reviews.llvm.org/D49267). I'm making a few guesses here: I assume that in the context of XCode, you would not do background indexing in Clangd but let XCode do it as it can also coordinate (and not overlap) with build tasks. Is that correct? In any case, I think the approach in the patch is not incompatible with what we had in mind, i.e. we could also reuse "overrideCompilationCommandForFile" for each file specified in didChangeConfiguration. I'm point this out because if you *do* end up needing all the compilation commands beforehand like I mentioned, then maybe we can skip the approach of specifying them with didOpen and send them all with didChangeConfiguration from start.
>
>
> Thanks for your response,
>  As it stands right now we will not run the indexer in Clangd for our use case, and it's unclear if we can even assemble a set of compile commands, so we would like to provide the commands when a file is open. We might be interested in a "didChangeConfiguration" message extension in the future (ideally it would be possible to change the subset of the constructed compilation database).


Sounds good, I think sending it in didOpen makes sense then. And yes, I agree that we would need to support changing a subset of commands through didChangeConfiguration, eventually!

In https://reviews.llvm.org/D49523#1169620, @jkorous wrote:

> Hi Marc-Andre, what is a structure of data you are passing as parameter of didChangeConfiguration message?


We don't yet :) But we will need to send the information per-file through it instead of using the compile_commands.json. Right now, what is supported in the didChangeConfiguration is pointing to a different compile_commands.json, in order to support multiple configuration (actually pointing to the folder containing the json file).

> All we need is to pass per-file compilation command to clangd. Maybe we could send didChangeConfiguration message right after didOpen.
> 
> EDIT: Well, provided we would find a way how not to parse the file twice.

The idea would be to send the per-file commands to clangd *before* anything is opened. So the didOpen would just read the latest command in memory. And subsequent changes to commands would be communicated with didChangeConfiguration and then files would get reparsed. I'm actually thinking we might want to send the commands in the "initialize" request for all the initial commands and then update them with didChangeConfiguration whenever they change. That way, there is no risk for reparsing as we should not do anything (indexing!) before the initialize.
But it doesn't sounds like you need this right now :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49523





More information about the cfe-commits mailing list