[PATCH] D128826: Go-to-type on smart_ptr<Foo> now also shows Foo

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added a comment.

In D128826#3619167 <https://reviews.llvm.org/D128826#3619167>, @sammccall wrote:

> Thanks! I just have a question if this behavior should be even "stronger":
>
> In most editors, if you return one result you go straight there, if you return two results you get a menu.
> A menu is significantly worse than going straight to the right result - the UI is clunkier and the choice interrupts your workflow.
>
> So my question is, do you think that we should *only* return the pointee in these cases?
>
> (It's possible to change this later, but I'd like to take a reasonable guess as this is forcing an API change from Type -> vector<Type>)
> cc @kadircet

I'm using this with neovim's builtin LSP, where when there are multiple results it takes me to the first one, and also opens the quickfix window that shows all results. But I can see how the current implementation could be annoying with other editors.

So here's what I thought: Say you have a variable that's a smart pointer and trigger textDocument/typeDefinition on it and there's only a single result that takes you straight to the type. In this case, you might not even know that the type is actually wrapped inside a smart pointer and might make wrong assumptions about the code. Of course you could've done a Hover beforehand, but in the case where there are 2 results, you don't need that extra step.

Maybe it would be best to make this configurable via the config file? Since most people are using "most editors", I think taking you straight to the type would be better, but I'm really not sure.

Btw, I think it would be useful to extend this feature to not only smart pointers, but also containers like std::vector. Then there'd also be the question what would happen for std::vector<std::unique_ptr<T>>: should there be 1, 2 or 3 results?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128826