[clang-tools-extra] [clangd] Re-land "support outgoing calls in call hierarchy" (PR #117673)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 2 07:30:26 PST 2024


https://github.com/kadircet requested changes to this pull request.

I am sorry for my poor choice of words around `config knob`, I was trying to imply a knob like https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/ClangdServer.h#L111. I don't think the knob needs to be exposed to end-users directly, only having programmatic access is fine initially. if you choose to expose it right away, i think a command line flag is the most natural way, rather than a config field. 

I'd rather plumb it explicitly through the interfaces of file-index/file-symbol/dex (and others), i think using `context` here is somewhat abusing the functionality. if you feel like that's too much (especially considering the size of the patch), feel free to leave a FIXME at least, to plumb it properly later on.

> Only a user config option is respected, not a project config option. I think this limitation is necessary, because the index is a global structure rather than a per-project structure. (Concretely, in the call to the Dex constructor that happens via BackgroundIndexRebuilder::maybeRebuild(), what path would I pass to the context provider that gets it to look at an appropriate project config file?)

you're absolutely right here. that's the main reason i don't think config mechanism actually works here. they're mostly meaningful for per-project/file configurable features. not for global ones like index.

> Only the Dex logic is conditioned on the option, not the binding of the outgoingCalls message. This is because the binding of the message happens fairly early, while we're processing the initialize request. At this point we haven't loaded config files yet, and if we try, that triggers a publishDiagnostics message for the config file, which confuses clients who are expecting an initialize response.

again this is resulting from my poor choice of words, if we treat this as part of option struct, we'll have the value at startup time and can decide what to do.

https://github.com/llvm/llvm-project/pull/117673


More information about the cfe-commits mailing list