nridge added a comment.

In D138546#4075681 <https://reviews.llvm.org/D138546#4075681>, @cpsauer wrote:

> Sounds like you're not concerned, then, that the 
> JSONCompilationDatabasePlugin will get invoked and the results then passed to 
> a SystemIncludeExtractor-enabled mangler?
> (I'd seen some fallback calls into the plugin registry in 
> GlobalCompilationDatabase, hence the original change, but, lacking broader 
> context, I was having trouble tracing the expected control flow.)

I did initially overlook the mentioned fallback codepath 
<https://searchfox.org/llvm/rev/eb8fcc1e835acc1f9ea1cb43902672dd9511d66b/clang-tools-extra/clangd/GlobalCompilationDatabase.cpp#317-339>,
 thanks for catching that. However, it looks like it's only taken if we've 
already explicitly checked for `compile_commands.json` and didn't find one, so 
`JsonCompilationDatabasePlugin` won't find anything either (i.e. the fallback 
is for potential other plugins).

As for consumers of the plugin registry outside of clangd, we can assume they 
won't use `SystemIncludeExtractor` which is currently a clangd-internal 
component. (There has been an idea floated about upstreaming 
`SystemIncludeExtractor` from clangd to libTooling, but it remains to be 
determined how that would look at the API level, and we can make adjustments as 
necessary if/when that happens.)

Anyways, +1 from me on the TargetAndMode related changes.


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

https://reviews.llvm.org/D138546

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

Reply via email to