[PATCH] D53688: [clangd] Add fallbackFlags initialization extension.

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 25 08:49:30 PDT 2018


ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D53688#1275793, @sammccall wrote:

> It's hard to reason about UX outside of concrete tools - the only use I know of (atom-ide-cpp) never needs to change it, and I'm having a hard time imagining when you'd want to change this, but still have it global to clangd. (I can imagine it being scoped to a subtree a la compile commands, but that's a whole different thing..)
>
> And yes, there are significant implementation concerns with making it mutable: ultimately we're going to want to have a CDB -> clangd compile command invalidation mechanism that can drive reindexing, reloading diagnostics etc. If the fallback command is mutable, such a mechanism needs to handle wildcards (or accept that this is a case it will get wrong, there are others).


Whatever the tool is, the choices for it are to allow changing some configuration at the init time or while running. Just realized that we're removing the compileCommandsDir for that specific reason, sorry for being out of context here.

The atom-ide-cpp use case is somewhat special anyway, since it ends up reading a file in a format we already support, albeit having a different name from the one clangd would expect (correct me if I'm wrong on this one).
In the long-run it seems better to communicate to the users that they should rename the file to make clangd work rather than add this (a bit different and more powerful?) extension to clangd.

We're aiming to allow users change their compile flags while running clangd anyway (assuming we're gonna get the file watches in at some point), so, again, in the long-run I don't see why that would complicate things. But sympathetic to keeping the code simpler anyway.

LGTM


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53688





More information about the cfe-commits mailing list