[PATCH] D51725: Allow un-setting the compilation database path

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D51725#1261798, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D51725#1255695, @simark wrote:
>
> > But I am wondering what to do with the feature that allows clangd to change compilation database path using the `didChangeConfiguration` notification.  In my opinion, it's a bug that it's not possible to switch back to use no explicit compilation database path, so I'd still like to get this patch merged.  Or, if we decide this is really not useful, then it should be removed.  I don't think we should keep the feature in the current state, either we fix it or remove it.
>
>
> I would question the usefulness of the option if Theia can live without it, so I'd vouch for removing it. @sammccall, WDYT? Especially wrt to auto-indexing and it being a customizable build directory?


Agree with you both here. Not being able to reset the initial state is weird, but *being able to* is complicated. (Even more so than switching between CDBs).
And since switching between CDBs invalidates almost everything, restarting clangd seems like a great simplification if it's viable for you.
Supporting a single CDB that's available on startup (either a flag, or maybe more likely set during initialize()?) would be a very useful simplification. (Even if it's not a lot of code, I find myself getting tangled in it often).

>> Even if right now it's not very expensive to restart clangd, do you foresee a situation in the future where it could become more expensive?
> 
> We should keep clangd restarts fast, I don't see any reason why they should be slow.

Agree, it's important that clangd starts fast for other reasons. And auto-indexing should stop/resume fine.
Shutdown is a slightly trickier case: actually doing a clean shutdown may require waiting for e.g. any current AST parse to finish. But if this is a problem, I think you can just send SIGTERM.

> Changing compile_commands.json files would probably always require both reparsing all the files and reloading all the indexes, so there's little hope we can make clangd faster in those cases, unless we're willing to keep multiple versions of the AST/indexes in memory.

The only thing I think we can gain is if you change CDBs, but actually many of the commands are *completely* identical. Then we could detect it and avoid invalidating ASTs etc for those files. But this seems like a really marginal edge-case.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51725