[PATCH] D93393: [clangd] Ignore the static index refs from the dynamic index files.

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 17 04:28:43 PST 2020


sammccall added a subscriber: kbobyrev.
sammccall added a comment.

In D93393#2460066 <https://reviews.llvm.org/D93393#2460066>, @ArcsinX wrote:

> Thanks for your reply. I have updated the patch (changed signature of `hasFile`), but remote-index scenario is not clear for me yet.

Sorry, I didn't see this before comments.

To be clear, I'm *not* asking you to add remote support in this patch, I'm just thinking aloud at what it might be like.
The "return false" behavior is fine for now. The remote index is basically at the bottom of the stack, so it's mostly moot. And "return false" yields the old behavior anyway.

I just want to think aloud about how we can add it on :-)

> In D93393#2458168 <https://reviews.llvm.org/D93393#2458168>, @sammccall wrote:
>
>> Implementing a pseudo-batch form of hasFiles as an RPC for remote-index: we'd need to give up on getting answers for files one-by-one. That would be OK for an N-way merge (since we'd fetch all results before merging, we could build a list). But servers typically index *everything* under a certain directory, so fetching the list of directories and evaluating per-file queries client-side is a cheap and reasonable alternative (that's more accurate than `return false`!)
>
> Seems I need some clarifications here.
> Is it should be like this?:
>
> - [client]->[server] Give me all directories and subdirectories
> - [client]<-[server] All directories are .....
>
> (we need this only once)

Right - we could fetch it when the connection goes up, or on every request, or even configure it entirely client-side (the client knows about the "mount point" of the index and maybe we could assume it's fully-populated). I'm a little hazy on this, @kbobyrev @kadircet would know what's best here once back from vacation.

>   IndexClient::indexedFiles() {
>   <returns function which checks if the file is inside a directory from the list (i.e. full filename starts with a directory from the list)>
>   }
>
> What structure should be used for directories list? I think we can use Trie

Probably just a string or flat list - I think this is exactly one directory today, maybe a short list in future.
(We have the assumption of a single root for path-translation purposes, not sure if we've seen any need for that root to be "partly-populated" yet. If so we may just end up creating one `Index` instance per directory and share stubs between them.)

But again, this isn't something you need to build now, unless you're really keen :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93393



More information about the cfe-commits mailing list