[PATCH] D39836: [clangd] Drop impossible completions (unavailable or inaccessible)

[Ilya Biryukov via Phabricator via cfe-commits]

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

This is probably one of the things that I'd like to be configurable.

In https://reviews.llvm.org/D39836#920313, @sammccall wrote:

> - they bulk up tests and debugging output


I'd argue that the more you put into tests, the more potential errors you'll see. E.g. if we're not showing private members, we might miss some presentation issues that only those exhibit. And you can always leave them out in tests in case you only want to test the public ones (though that's not a good idea, probably).
As for debugging, I don't LSP is any good as a debugging format anyway, we probably want some different form of output.

> - they complicate the scoring scheme - we *never* want them to appear above a legal completion, so a 1-dimensional score needs special handling as here.

We could always use a 2-dimensional score `(accessible, sort_score)`. I'd love to have some editor support to grey-out some items (not only inaccessible, deprecated items could have been somehow signalled in completion UI as well), probably requires some changes to LSP too and not a priority, though.

> - compute: we spend time scoring them, computing their strings, writing them over the wire, and then the client has to process them

I don't think it matters much, given that there aren't many items that are inaccessible compared to the total number of completion items when it's slow (they are all class members, but the biggest portion of items come from namespaces). And after-dot completion is usually fast (and when it's slow it's because reparse is slow, not because we take too much time to process the items).

That being said, I don't think there's a chance I'll argue my way through this. Let's turn them off and see if someone (besides me) complains. LGTM.


https://reviews.llvm.org/D39836