Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inlay Hints for goto Definition #13010

Open
hapeeeeee opened this issue Dec 2, 2024 · 8 comments
Open

Inlay Hints for goto Definition #13010

hapeeeeee opened this issue Dec 2, 2024 · 8 comments
Labels
bug enhancement Improvement to an existing feature Feature: Inlay Hint An issue related to inlay hints for declarations and parameters. Language Service Visual Studio Inherited from Visual Studio

Comments

@hapeeeeee
Copy link

Feature Request

When I use auto as the variable type, Inlay Hints provide the inferred type. However, when I try to jump to the definition using Inlay Hints, it fails. Can the plugin support jumping to the type definition through Inlay Hints?

@sean-mcmanus sean-mcmanus added Language Service enhancement Improvement to an existing feature labels Dec 2, 2024
@sean-mcmanus sean-mcmanus added Feature: Inlay Hint An issue related to inlay hints for declarations and parameters. bug labels Dec 2, 2024
@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented Dec 2, 2024

Yeah, I've verified it's supported by VS Code (microsoft/vscode#129528) and works with TypeScript (via using the Ctrl key and hovering over the type in the inlay hint). I'm not sure why it's not working for our extension though.

@hapeeeeee
Copy link
Author

Yeah, I've verified it's supported by VS Code (microsoft/vscode#129528) and works with TypeScript (via using the Ctrl key and hovering over the type in the inlay hint). I'm not sure why it's not working for our extension though.

I have identified the issue and plan to submit a PR to complete the feature within a week.

@sean-mcmanus
Copy link
Contributor

@hapeeeeee Cool, that sounds good to me (assuming the issue is with our open source typescript handling). If there's changes we need to make in the cpptools process we could potentially do that ourselves if we know what changes to make.

@sean-mcmanus sean-mcmanus added the help wanted Can be fixed in the public (open source) repo. label Dec 3, 2024
@hapeeeeee
Copy link
Author

hapeeeeee commented Dec 3, 2024

@hapeeeeee Cool, that sounds good to me (assuming the issue is with our open source typescript handling). If there's changes we need to make in the cpptools process we could potentially do that ourselves if we know what changes to make.

@sean-mcmanus In fact, I have written a demo that supports InlayHints for goto definitions, and it currently supports this feature.
Image

However, I found that the CppInlayHint provided by the IntelliSenseResult does not include the file URL and line/column numbers of the definition. I am unable to find these required parameters in the project, so it seems like I can't proceed with my work.

How can I continue?

@hapeeeeee
Copy link
Author

@sean-mcmanus Can cpptools provide some support?

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented Dec 10, 2024

@hapeeeeee Do you have a pull request available? We could potentially modify cpptools to send the additional data.

However, the design seems incorrect -- I don't think we should have to provide the definition with the inlay hint -- we should get sent a definition request for the specific symbol that the go to def is invoked on. Otherwise, I think we may not be able to provide all the definitions for performance reasons.

Go to def (or calculating the definition location) for C/C++ can potentially be an expensive operation.

@hapeeeeee
Copy link
Author

@hapeeeeee Do you have a pull request available? We could potentially modify cpptools to send the additional data.

However, the design seems incorrect -- I don't think we should have to provide the definition with the inlay hint -- we should get sent a definition request for the specific symbol that the go to def is invoked on. Otherwise, I think we may not be able to provide all the definitions for performance reasons.

Go to def (or calculating the definition location) for C/C++ can potentially be an expensive operation.

I’m not clear about the implementation details of cpptools, but as far as I know, the compiler provides the location of the definition (a file path and line/column number) during compilation. Adding a command to inlay hints simply involves adding a jump instruction and a piece of string.

Compared to sending a request to fetch the definition location, carrying the jump instruction during generation is clearly more efficient.

@sean-mcmanus
Copy link
Contributor

sean-mcmanus commented Dec 11, 2024

@hapeeeeee When you open a file with our extension, it only compiles the current TU, so the definition may not be available to the code that generates the inlay hints, which only knows the types. Going to a definition outside of the TU requires a database operation, which can't scale to every single inlay hint -- the declaration would be available but our inlay hint implementation doesn't currently store that when it computes it (i.e. we'd probably need it to be implemented by VS, which currently doesn't support go to definition on inlay hints for C++).

Currently, you should be able to use go to definition on the "auto" -- do you have a case in which that doesn't work?

struct foo { int i;  };

foo func(foo ff)
{
	return ff;
}

int func2()
{
	auto f = func(foo{ 0 });
}

Image

@sean-mcmanus sean-mcmanus added Visual Studio Inherited from Visual Studio and removed help wanted Can be fixed in the public (open source) repo. labels Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug enhancement Improvement to an existing feature Feature: Inlay Hint An issue related to inlay hints for declarations and parameters. Language Service Visual Studio Inherited from Visual Studio
Projects
Status: No status
Development

No branches or pull requests

2 participants