Support go-to-definition on type names in inlay hints

[HighCommander4]

Rust-analyzer supports go-to-definition on type names (where the type has a definition in the source) in inlay hints.

LSP 3.17 appears to support this using an optional feature called "interactive and composite labels", where InlayHint.label is an array of InlayHintLabelParts (which can have associated locations) rather than a string.

It would be nice to add similar support to clangd.

[zyn0217]

Interesting idea. Are you working on this right now? If not, perhaps I could have a try :)

[HighCommander4]

Please feel free, I probably wouldn't get to this for a while :)

[zyn0217]

(Apologies for the late reply, I've been little occupied recently. And please don't mind if I'm asking stupid question.)
I made some modification locally and it works for certain context where type hint occurs. I just want to ensure if I'm heading in the right direction: Can we just directly migrate the type of label field, just as defined in InlayHint protocol, from 'string' to 'InlayHintLabelPart[]'? I mean we may have to concern about compatibility if clients with InlayHint support (not only vscode) is not capable to parse the field as an array but only for string. It's highly unlikely such client exists but just in case, should we do anything to keep compatibility?

[HighCommander4]

It's a good question. Usually, the way this would work is that the client capabilities include an option that indicates whether the client supports the richer format.

However, since there is no such option in InlayHintClientCapabilities, and since the InlayHintLabelPart feature was part of the initial version of the inlay hints feature added to the spec in LSP 3.17, I think it means it's safe for a server to assume that a client that supports inlay hints at all supports InlayHintLabelPart as well.

[zyn0217]

I've hit another problem. IIUC, according to current LSP specification we don't have access to position information for linkable inlay hints. As a result, we may miss user's intended cursor location. For example, for class template with template parameters, we're unable to tell which declaration that user want to inspect.

auto f(auto p) { return p; }

struct C {};

void foo() {
  auto r = f(std::vector<C>()); // r: std::vector<C>
}

We could only provide one location for hint : std::vector<C>, which would either be definition of std::vector or that of C. This limitation restricts the use case for the feature and may lead to confusion for users.

Or we don't provide any clickable links on templates, at least before LSP specification provides cursor information for inlay hints.

---

I've installed rust-analyzer and tried with some generics, and it looks like fine to drop support for inner template parameters. :(

[HighCommander4]

I think the idea behind having an array of InlayHintLabelParts is that for a hint like std::vector<C>, we can have four label parts: std::vector, <, C, and >, with the std::vector and C parts having (different) locations.

[zyn0217]

Cool, that's it. Still refining logic before submitting a patch.
(I'm a bit confused that it looks much slower when go to definition by clicking on type std::optional than clicking on auto...)

[zyn0217]

So, I guess I'm in a pickle here and I'm looking for a (possible) solution.

As previously said, I'm trying to extract type hint to InlayHintLabelParts in order to make every possible class/struct jumpable. This is trivial for non-template types as we could simply put such type name into a single InlayHintLabelPart by calling QualType::getAsString(), which is currently in use. But what makes everything complicated is template types, for which we have to extract every template argument from (possibly deduced) a type and then apply locating logic on it.

Currently I'm rewriting a visiting logic(which is mostly borrowed from TypePrinter, a private class defined in a cpp file.) that attempts to inspect each parameter, find declaration, and print brackets and commas. Soon I find it hard to maintain because I have to reconsider every corner case that TypePrinter handles. And I don't think it is a promising approach which essentially is duplicating (part of) the TypePrinter.

I'm considering a workaround and here is what I've come up with:

1. Just rewrite the processing logic for templates, which is I'm doing now and appears to be quite hard to maintain.
2. Add a callback interface (similar to PPCallbacks) that would be passed into QualType::getAsString(), which intercepts possible write logic, then we could handle these symbols separately. (Cons: Interfaces might be not that generic to handle all the cases for simplicity)
3. Parse the template name manually by maintaining a stack, then extract every parameter that we want to get its location. (It's strange as we've already had QualType)
4. Write a custom raw_ostream that intercepts every streaming request to get symbol name. (Like 2, but we could leave TypePrinter as-is)

Do you have any suggestion? Or could we resort to any other facility that provides more flexibility on type names? Thank you very much in advance :)

[HighCommander4]

It would be handy if there existed a version of TypePrinter that printed the type into a virtual buffer that could be referenced via SourceLocations, and also constructed a TypeLoc representation of the type, with its SourceLocations pointing into the virtual buffer. Then, we would get most of what we want for free, by running ExplicitReferenceCollector on the TypeLoc, which would traverse the TypeLoc and identify every place in it which is a reference to some Decl. (To think about it a different way, these are the places that would get go-to-def targets if the type were printed into an appropriate context in the source code.)

Unfortunately I'm not aware of such a thing and it would probably be somewhat involved to write, so for now we may need to do something more manual.

One general thought: the solution doesn't need to be perfect / handle all edge cases. Any solution that provides links to type names in some common cases, is an improvement over the status quo. So, if it makes the task easier to "bail out" when encountering some edge cases and just fall back on printing a particular type component as a string with no links, I think that's fine.

Some thoughts on your specific options:

• I think (4) may not work because raw_ostream doesn't make the needed functions virtual for overriding
• (2) is probably the most powerful/flexible approach, but since it involves modifying an upstream clang API, it would need some discussion and buy-in on the LLVM Discourse first

Does the TypeVisitor interface help at all? My initial suggestion would probably be to use TypeVisitor to implement a traversal of the type that handles common cases (e.g. references, pointers, qualifiers, template specialization type) and falls back to getAsString() (with no links) for more complicated cases.

[zyn0217]

Thank you for sharing me ideas and my tentative implementation is using TemplateArgumentVisitor and TypeVisitor. I think I should take a further learning at clang CXX types, since I'm always cringed that I might mess up type printing here =)

...by running ExplicitReferenceCollector on the TypeLoc, which would traverse the TypeLoc and identify every place in it which is a reference to some Decl.

FWIW, I'm not using ExplicitReferenceCollector but locateSymbolForType from XRef.cpp to inspect declarations of symbol at the moment. I took a rough look at the collector and IIUC, the basic idea is to collect all of the type declarations once and then we could make it faster for query(i.e., generate hints for each type), right? Compared to locateSymbolForType, which is more optimal do you think? (I assume it is a trade-off as the collector may take more space while the locateSymbolForType has a higher time complexity, though)

[HighCommander4]

ExplicitReferenceCollector is not really useful for us in the absence of a way to get a TypeLoc for the type, because it operates on AST nodes that have associated SourceLocations, including Expr and Decl and TypeLoc (but not plain Type). In the hypothetical scenario where we have a TypeLoc, the utility of ExplicitReferenceCollector is that it could do the work of identifying the places in the type string that refer to some declaration (as well as getting those declarations), in essence replacing the need for new code to traverse over the type.

For the use case of getting a target declaration for a Type, locateSymbolForType is a good choice.

[HighCommander4]

@zyn0217 what do you think about making some incremental progress here by starting with a simple initial change, e.g. always producing just one InlayHintLabelPart for a type hint which takes you to a single target location (e.g. for vector<C> it might be <vector>)?

[zyn0217]

Ah, sure! Let me sort out the patch. And sorry for procrastinating...

[zyn0217]

Just have sent out a patch of the protocol part to reduce the review workloads. This part looks separated and backward compatible.

[zyn0217]

So, I have rewritten most of the logic (the previous implementation was sadly lost :( ), and the drafts are now on this branch. This is going to support links on template-type arguments for hopefully most common cases e.g.

                              vvvv   No link on basic types
std::map<MyClass, std::vector<bool>>
                  ^^^^^^^^^^^  link to the ClassTemplateSpecializationDecl
         ^^^^^^^ link to the CXXRecordDecl