[PATCH] D75479: [clangd] go-to-def on names in comments etc that are used nearby.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D75479#1908774 <https://reviews.llvm.org/D75479#1908774>, @nridge wrote:

> This method only returns `Bar::uniqueMethodName()` because it's closer in terms of distance


Yeah, missing data is definitely bad:

- not finding results makes the feature feel flaky or unreliable
- finding *partial* results can be misleading and makes the feature untrustworthy

> Is that perhaps a reason to adjust the order in which we try the approaches (i.e. use this one as a fallback if index lookup fails)?

Well, it's even easier to construct examples where the index approach is missing some data (most symbols are not indexable) or gives wrong results (name collisions are common) which is probably worse.
So I'm not convinced by this that the index is more accurate.

> Or should we try to allow multiple results with this approach as well?

It's an interesting idea, but two immediate risks:

- it's clearly worse for things like comment navigation local variables
- it helps your example only quite narrowly: all the targets need to be in the current file

We have several building blocks and various ways for how to put them together.
I think we should go back to the use cases (e.g. dependent code) and work out how each is best served, my intuition is that pretty different. Discussing how the building blocks should relate to each other in the general case may run in circles a bit :-) I'll make a proposal on https://github.com/clangd/clangd/issues/241 for a bit more visibility.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75479