Thanks, this is an interesting idea, which I think could be feasible. Transferring to the go issue as this would be implemented in gopls.

Hi, I'd like to give this a try.

I made a first attempt (without gopls) just listing all "links" in a package (including transitive imports): github.com/vikblom/linkname
Best I can tell, the placement of this directive does not matter, so just looping over comments is fine.

My idea was that a map of all links can be created ahead of time (and cached?), then used when analyzing a single file to decorate any decls that are part of a "link".

How would this work in gopls?

Hi @vikblom, thanks for looking into this!

Sorry for the slow response: had to do a bit of research to be able to make an informed response.

The compiler documentation for linkname is lacking (@adonovan is currently updating it in https://go.dev/cl/463136!), but I think we need to identify two different patterns for using //go:linkname, based on the number of arguments to //go:linkname and whether or not the function body is present:
https://go.dev/play/p/gFbJabVseig

In this playground link, where package foo imports package bar, the following two patterns are used:

1. Two arguments to go:linkname; no function body: access an unexported function in the link namespace of bar:

//go:linkname foo_bar play.ground/bar.bar
func foo_bar() string

2. Two go:linkname directives: one with two arguments to go:linkname + function body and one with one argument to go:linkname. This allows defining an unexported function in bar with a link name in foo. This is subtly different from (1), because the link name exists in the foo namespace, rather than the bar namespace. (we seem to use this pattern a lot to expose bits of the runtime package to other packages, such as time or rand).

In bar, use a //go:linkname directive with two arguments, and a function body

//go:linkname bar_foo play.ground/foo.foo
func bar_foo() string {
	return "foo, defined in bar"
}

In foo, use a //go:linkname directive with one argument, and no function body.

//go:linkname foo
func foo() string

I think we need to treat these two cases differently from gopls:

Handling pattern (1): (1) it is pretty straightforward how to find the symbol: in order to jump from foo_bar to bar, just search the forward dependencies of foo for a declaration with link name play.ground/bar.bar.

Handling pattern (2) is more complicated: in order to jump from foo to bar_foo, we need to search forward dependencies for //go:linkname directives matching the default linker name of foo.foo. In order to jump from bar_foo to foo, we need to look through reverse dependencies to find one with the desired link name.

I think pattern 1 is far more common than pattern 2, and so it would be OK to only implement support for pattern 1 in the initial implementation.

I don't think we should have to manage a linkname index. It would be simpler to just look through the forward dependencies of packages containing the //go:linkname directive, searching for packages whose package path matches the package path in the second argument. Then, type-check that package and extract the declaration defining the desired linker object.

The way to do this in gopls is to first find package information for packages containing the given file: snapshot.MetadataForFile. It should be sufficient to only consider the first result of that function, which should be the narrowest package containing the file. Then recursively walk those dependencies searching for packages with the desired package path. When found, type check the desired package (snapshot.TypeCheck in TypecheckFull mode), and look in scope for the object with desired linkname.

Since metadata and type-checked packages are cached, these should be cache hits most of the time, and therefore it should be fine to implement the search in this way, without any need for additional caching.

Sorry for the wall of text. Does that make sense?

Thank you @findleyr that makes a lot of sense 👍

A note on pattern 2, as I understand it, //go:linkname foo tells the compiler that "no body is fine here". Another way to get the same behaviour is if there is an assembly file in the package, like this: https://go.dev/play/p/l3GRMeW0X3-
So there is not always two linkname directives (maybe that is just "pattern 3", out of scope for now regardless).

But anyway, aiming for pattern 1 first seems like a good approach, I will start there.

Should this extend "goto definition" or be some kind of navigable hyperlink? If you have a package in mind where this should be implemented that would be helpful.

A note on pattern 2, as I understand it, //go:linkname foo tells the compiler that "no body is fine here". Another way to get the same behaviour is if there is an assembly file in the package, like this: https://go.dev/play/p/l3GRMeW0X3-
So there is not always two linkname directives.

Oh, interesting. Thank you, that makes sense. I actually tested exactly that, and saw the compiler error. I did not think of adding a .s file.

Should this extend to "goto definition" or be some kind of navigable hyperlink? If you have a package in mind where this should be implemented that would be helpful.

I think eventually it would be good to handle a variety of requests related to linknames, but let's start with handling the textDocument/definition request:
https://cs.opensource.google/go/x/tools/+/master:gopls/internal/lsp/definition.go;l=25;drc=b79893424cb0a12a4192f38bf959773a3a69c145

At that link, gopls delegates to a source.Identifier function to collect information about the identifier at point. This function collects a bunch of information about the identifier at the current cursor position, most of which is not applicable for the case we're considering. (Frankly, source.Identifier is also badly in need of refactoring, and I don't want to ask you to work on it). Instead, I think it would be sufficient to implement the linkname logic from scratch:

Something like this:

// At internal/lsp/definition.go:25:

if pkgPath, name := source.ParseLinkname(ctx, snapshot, fh, params.Position); pkgPath != "" {
  return source.FindLinkname(ctx, snapshot, fh, pkgPath, name)
}
// Otherwise, proceed as before
ident, err := source.Identifier(...)

// In the internal/lsp/source package:

// ParseLinkname attempts to parse a go:linkname declaration at the given pos.
// If successful, it returns the package path and object name referenced by the second
// argument of the linkname directive.
//
// If the position is not in a go:linkname directive, or parsing fails, it returns "", "".
func ParseLinkname(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position) (pkgPath, name string)

// FindLinkname searches dependencies of packages containing fh for an object 
// with linker name matching the given package path and name.
func FindLinkname(ctx context.Context, snapshot Snapshot, fh FileHandle, pos protocol.Position, pkgPath, name string) ([]protocol.Location, error)

In other words, just intercept the definition request and check whether this new linkname logic applies, before passing off to the normal handling. You can either use snapshot.ParseGo to parse the file, or better yet just read the file content and use go/scanner to quickly scan comments for linkname directives. Then you'll need to parse the linkname directives, and check whether the cursor is in the second argument. Then parse the second argument to extract package path and object name.

In FindLinkname, you can use snapshot.MetadataForFile and snapshot.TypeCheck as I suggested above.

In this way, I think this project can be implemented without having to get too far into the weeds.

You don't have to take my suggested API, but I would just advise keeping your change isolated from the existing logic.

Keeping the changes isolated sounds like a good idea 👍

I will put some effort into this (could take some time) and post back here when I have an update (or questions).

Thanks @vikblom!

This issue is not particularly urgent, but I have added you as assignee. Feel free to ping back here if you have any questions as you work on this.

Change https://go.dev/cl/463755 mentions this issue: gopls/internal/lsp: go to definition in linkname directive

For reference, I had a quick look at the standard library for examples. I found directives at:

Pattern 1

package time
//go:linkname runtimeNano runtime.nanotime
func runtimeNano() int64

https://cs.opensource.google/go/go/+/master:src/time/time.go;l=1099
where time imports runtime.

Pattern 2

package runtime
//go:linkname runtime_debug_freeOSMemory runtime/debug.freeOSMemory
func runtime_debug_freeOSMemory() { ... }

https://cs.opensource.google/go/go/+/master:src/runtime/mheap.go;l=1654
where runtime.debug imports runtime.

Edit: I wrote a script to walk through the standard library, checking if linkname directives (1st argument) funcs are with or without bodies, and if the referenced package (2nd argument) is a forward or reverse dependency. The numbers are

   body + forward dep: 21
   body + reverse dep: 263   // pattern (2) 
no body + forward dep: 95    // pattern (1)
no body + reverse dep: 33

I took the first implementation for a spin and I'd say it works as expected in some/most cases.
There are definitely (as mentioned in https://go.dev/cl/463136) edge cases and wierd setups in the standard library (for example time/tzdata).

One problem is that just naively opening a package and invoking Go To Definition on a linkname directive can fail because the referenced package is not found. The jump succeeds after manually opening the referenced package.

I'm not sure if its worth chasing down or if I should stick to the plan and look at implementing jump from single argument directives first.

@findleyr what do you think is the next move here?

wierd setups in the standard library (for example time/tzdata)

For the record, this (tzdata) is almost an example of the second scenario in the doc---the upper package grabbing a symbol from the lower one---except that in this case the upper package doesn't actually have an import dependency on the lower one, presumably just because it doesn't need it. But that does mean gopls won't be able to find one from the other.

I'm not sure if its worth chasing down or if I should stick to the plan and look at implementing jump from single argument directives first.

I suspect this is a relatively rare case and not worth worrying about.

What would "jump from" look like. Changing the 'references' query to regard linknames as references?

Hi @adonovan

For reference here is one error I zeroed in on:

go run ./gopls/main.go definition (go env GOROOT)/src/time/tzdata/tzdata.go:33:46

Sine (I'm assuming) time does not appear in

go list -f '{{ join .Deps "\n" }}' (go env GOROOT)/src/time/tzdata

and vice versa. Is there some alternative approach to finding the symbol referenced in the linkname directive?

I suspect this is a relatively rare case

Sorry I'm not getting which you're referring to, the tzdata example or single-arg directives?

About the "jump from" see rfindley's post above, at "Handling pattern (2)".
I interpret that as a go to definition from a single argument linkname directive.
By 'references' do you mean that gopls could maybe list references to a symbol (in a linkname directive), that is an interesting idea! I'm not sure what makes sense from a user perspective?

Is there some alternative approach to finding the symbol referenced in the linkname directive?

You could filter AllMetadata based on the PkgPath named in the directive, perhaps as a third option if ReverseDependencies finds nothing.

I suspect this is a relatively rare case
Sorry I'm not getting which you're referring to, the tzdata example or single-arg directives?

I mean tzdata is a rare example in which there is no import relationship between the two packages.

I agree with Rob that supporting case 1 (as you have already done) is the more important case by far. Case 2 requires that you search the source code of dependencies to find a 2-arg directive containing the right package name, whereas the search from the 2-arg directive can be done on the metadata. I'm inclined to say the complexity is not worth the trouble.

By 'references' do you mean that gopls could maybe list references to a symbol (in a linkname directive), that is an interesting idea! I'm not sure what makes sense from a user perspective?

I wasn't sure whether you were proposing to treat linkname like a reference, but I would prefer that we don't.

Okok, focusing on the common case makes sense!

You could filter AllMetadata based on the PkgPath named in the directive, perhaps as a third option if ReverseDependencies finds nothing.

I can give this a go and post back here if I come up with something or get stuck. Thanks!

@adonovan AllMetadata works like a charm 👍
Should I put up a CL for this right away?

While implementing this I bumped into a problem with gopls definition ... on linkname directives.
For example

# go run ./gopls/main.go definition (go env GOROOT)/src/time/time.go:1098:27
gopls: /home/viktor/local/go/src/time/time.go:1098:27: not an identifier
exit status 2

Best I can tell this is because definition gets a nil hover back from a Hover query, which uses source.Identifier. Both you and @findleyr mentioned that source.Identifier would be the correct place for linkname logic (but we're staying out of there while its being refactored).

Is this something to troubleshoot+patch right now or is it a side effect of being "outside" source.Identifier that could wait until its ready to integrate?

Edit: I'm not familiar with Hover but I noticed that gopls/internal/lsp/cmd/definition.go:103 uses loc (the "from" location), should it instead be locs[0] (the "to" location)?

@vikblom I am eliminated source.Identifier in https://go.dev/cl/464415, which should be ready soon. I'll take a look at your example to make sure it works.

Change https://go.dev/cl/469695 mentions this issue: gopls/internal/lsp: support more cases of definition on linkname directive.

Change https://go.dev/cl/479695 mentions this issue: gopls/internal/lsp: hover over linkname directives.

Hi @findleyr

I just uploaded a patch to support hover on linkname directives.

I stumbled across a problem in Emacs LSP Mode while testing this patch. When using this hover feature there is some kind of race and the editor drops into a busy loop sending and cancelling requests. Both Emacs with Eglot and VS Code (very basic testing) works fine as far as I can tell, no loss of responsive-ness.

Should this kind of potential client regression be handled before review? Before submit? I could try to raise it in the LSP Mode issue tracker after narrowing it down some more.

Edit: This might just have been poor handling of protocol.Range on my part, cannot reproduce with the latest submitted patch.

Thanks very much @vikblom for this contribution. It looks like this issue is complete!