textDocument/definition returns only one definition

[smhc]

For virtual functions with multiple definitions, clangd appears to return only the declaration for a textDocument/definition request.

The language specification seems to allow returning an array of locations. I believe clangd should respond with all definition locations.

[sam-mccall]

Formally speaking, a function can only have one definition (or none). This is what clangd returns: the definition if it's visible or indexed, otherwise the declaration.

Showing overrides of a virtual member function is a useful feature, and one we should certainly support somehow. We need to add more info to the index for this.

(There's a pending patch adding base-derived relationships to the index, we should be able to reuse that structure. So let's wait until that lands).

This seems like a better fit for lsp's textDocument/implementations method than for definition to me. The main counterargument i can see is that's a fairly new and unclear piece of the protocol, and many more editors support go to definition. I'm not sure that's a strong enough argument to stretch its semantics though.

[smhc]

The 'definition' command does support returning a list of locations. Some documentation does refer to an overridden virtual function as a 're-definition'. Regardless, the behaviour needs to be in the spec and agreed upon. Interestingly I couldn't find any mention of this problem in the LSP spec or associated project.

I have only found mention in the cquery LSP, which implements a "$cquery/derived" command for listing derived classes or virtual function overloads - which I assume does what I am referring to.

From a UI perspective you likely only want one command to jump to the best candidate implementation for a given function, regardless of whether there are overrides or not (sometimes this can be statically determined, other times not. I'm unsure if clang has this capability). Jumping to the base declaration and then another command to list all overrides might be a bit of a hassle when trying to navigate around a code base. Although I suppose an LSP client could do this automatically for you if it knew when it was appropriate.

I have worked on some code which uses pure virtual functions in base classes for almost all functions. This is done to better support unit testing. Unfortunately navigating this code base is quite difficult with an LSP as 'goto definition' almost always takes you to the base class declaration instead of what you are are really interested in.

[sam-mccall]

Happy to revisit this when #29 (textDocument/implementation) is implemented.

[sam-mccall]

This is done now (for pure virtual methods only, and find-implementation works on all virtual methods)