gopls/internal/golang: Definition: jump to assembly
This CL adds support for jumping to the definition of a function
implemented in assembly. The first Definition query jumps to the
Go declaration, as usual; this func has no body. Executing a
second Definition query jumps to the assembly implementation,
if found.
+ Test, doc, relnote
Fixes golang/go#60531
Change-Id: I943a05d4a2a5b6a398450131831f49cc7c0754e4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/612537
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
diff --git a/gopls/doc/features/navigation.md b/gopls/doc/features/navigation.md
index 5daac90..0037134 100644
--- a/gopls/doc/features/navigation.md
+++ b/gopls/doc/features/navigation.md
@@ -22,6 +22,8 @@
(like [`hover`](passive.md#hover)) the location of the linked symbol.
- On a file name in a **[`go:embed` directive](https://pkg.go.dev/embed)**,
it returns the location of the embedded file.
+- On the declaration of a non-Go function (a `func` with no body),
+ it returns the location of the assembly implementation, if any,
<!-- On a built-in symbol such as `append` or `unsafe.Pointer`, `definition` reports
the location of the declaration in the builtin or unsafe pseudo-packages,
diff --git a/gopls/doc/release/v0.17.0.md b/gopls/doc/release/v0.17.0.md
index 45eecef..c575229 100644
--- a/gopls/doc/release/v0.17.0.md
+++ b/gopls/doc/release/v0.17.0.md
@@ -46,3 +46,11 @@
Now, function signature help can be used on any identifier with a function
signature, not just within the parentheses of a function being called.
+
+## Jump to assembly definition
+
+A Definition query on a reference to a function jumps to the
+function's Go `func` declaration. If the function is implemented in C
+or assembly, the function has no body. Executing a second Definition
+query (while already at the Go declaration) will navigate you to the
+assembly implementation.
diff --git a/gopls/internal/cache/load.go b/gopls/internal/cache/load.go
index 6c176fc..9373766 100644
--- a/gopls/internal/cache/load.go
+++ b/gopls/internal/cache/load.go
@@ -412,18 +412,15 @@
updates[id] = mp
- for _, filename := range pkg.CompiledGoFiles {
- uri := protocol.URIFromPath(filename)
- mp.CompiledGoFiles = append(mp.CompiledGoFiles, uri)
+ copyURIs := func(dst *[]protocol.DocumentURI, src []string) {
+ for _, filename := range src {
+ *dst = append(*dst, protocol.URIFromPath(filename))
+ }
}
- for _, filename := range pkg.GoFiles {
- uri := protocol.URIFromPath(filename)
- mp.GoFiles = append(mp.GoFiles, uri)
- }
- for _, filename := range pkg.IgnoredFiles {
- uri := protocol.URIFromPath(filename)
- mp.IgnoredFiles = append(mp.IgnoredFiles, uri)
- }
+ copyURIs(&mp.CompiledGoFiles, pkg.CompiledGoFiles)
+ copyURIs(&mp.GoFiles, pkg.GoFiles)
+ copyURIs(&mp.IgnoredFiles, pkg.IgnoredFiles)
+ copyURIs(&mp.OtherFiles, pkg.OtherFiles)
depsByImpPath := make(map[ImportPath]PackageID)
depsByPkgPath := make(map[PackagePath]PackageID)
diff --git a/gopls/internal/cache/metadata/metadata.go b/gopls/internal/cache/metadata/metadata.go
index 7860f33..e42aac3 100644
--- a/gopls/internal/cache/metadata/metadata.go
+++ b/gopls/internal/cache/metadata/metadata.go
@@ -45,10 +45,11 @@
PkgPath PackagePath
Name PackageName
- // these three fields are as defined by go/packages.Package
+ // These fields are as defined by go/packages.Package
GoFiles []protocol.DocumentURI
CompiledGoFiles []protocol.DocumentURI
IgnoredFiles []protocol.DocumentURI
+ OtherFiles []protocol.DocumentURI
ForTest PackagePath // q in a "p [q.test]" package, else ""
TypesSizes types.Sizes
diff --git a/gopls/internal/golang/definition.go b/gopls/internal/golang/definition.go
index 438fe2a..f20fe85 100644
--- a/gopls/internal/golang/definition.go
+++ b/gopls/internal/golang/definition.go
@@ -12,12 +12,15 @@
"go/parser"
"go/token"
"go/types"
+ "regexp"
+ "strings"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/metadata"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/file"
"golang.org/x/tools/gopls/internal/protocol"
+ "golang.org/x/tools/gopls/internal/util/astutil"
"golang.org/x/tools/gopls/internal/util/bug"
"golang.org/x/tools/internal/event"
)
@@ -92,6 +95,18 @@
return builtinDefinition(ctx, snapshot, obj)
}
+ // Non-go (e.g. assembly) symbols
+ //
+ // When already at the definition of a Go function without
+ // a body, we jump to its non-Go (C or assembly) definition.
+ for _, decl := range pgf.File.Decls {
+ if decl, ok := decl.(*ast.FuncDecl); ok &&
+ decl.Body == nil &&
+ astutil.NodeContains(decl.Name, pos) {
+ return nonGoDefinition(ctx, snapshot, pkg, decl.Name.Name)
+ }
+ }
+
// Finally, map the object position.
loc, err := mapPosition(ctx, pkg.FileSet(), snapshot, obj.Pos(), adjustedObjEnd(obj))
if err != nil {
@@ -353,3 +368,40 @@
m := protocol.NewMapper(fh.URI(), content)
return m.PosLocation(file, start, end)
}
+
+// nonGoDefinition returns the location of the definition of a non-Go symbol.
+// Only assembly is supported for now.
+func nonGoDefinition(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, symbol string) ([]protocol.Location, error) {
+ // Examples:
+ // TEXT runtime·foo(SB)
+ // TEXT ·foo<ABIInternal>(SB)
+ // TODO(adonovan): why does ^TEXT cause it not to match?
+ pattern := regexp.MustCompile("TEXT\\b.*·(" + regexp.QuoteMeta(symbol) + ")[\\(<]")
+
+ for _, uri := range pkg.Metadata().OtherFiles {
+ if strings.HasSuffix(uri.Path(), ".s") {
+ fh, err := snapshot.ReadFile(ctx, uri)
+ if err != nil {
+ return nil, err // context cancelled
+ }
+ content, err := fh.Content()
+ if err != nil {
+ continue // can't read file
+ }
+ if match := pattern.FindSubmatchIndex(content); match != nil {
+ mapper := protocol.NewMapper(uri, content)
+ loc, err := mapper.OffsetLocation(match[2], match[3])
+ if err != nil {
+ return nil, err
+ }
+ return []protocol.Location{loc}, nil
+ }
+ }
+ }
+
+ // TODO(adonovan): try C files
+
+ // This may be reached for functions that aren't implemented
+ // in assembly (e.g. compiler intrinsics like getg).
+ return nil, fmt.Errorf("can't find non-Go definition of %s", symbol)
+}
diff --git a/gopls/internal/test/integration/misc/definition_test.go b/gopls/internal/test/integration/misc/definition_test.go
index 6b364e2..71f255b 100644
--- a/gopls/internal/test/integration/misc/definition_test.go
+++ b/gopls/internal/test/integration/misc/definition_test.go
@@ -5,9 +5,11 @@
package misc
import (
+ "fmt"
"os"
"path"
"path/filepath"
+ "regexp"
"strings"
"testing"
@@ -595,3 +597,48 @@
}
})
}
+
+func TestAssemblyDefinition(t *testing.T) {
+ // This test cannot be expressed as a marker test because
+ // the expect package ignores markers (@loc) within a .s file.
+ const src = `
+-- go.mod --
+module mod.com
+
+-- foo_darwin_arm64.s --
+
+// assembly implementation
+TEXT ·foo(SB),NOSPLIT,$0
+ RET
+
+-- a.go --
+//go:build darwin && arm64
+
+package a
+
+// Go declaration
+func foo(int) int
+
+var _ = foo(123) // call
+`
+ Run(t, src, func(t *testing.T, env *Env) {
+ env.OpenFile("a.go")
+
+ locString := func(loc protocol.Location) string {
+ return fmt.Sprintf("%s:%s", filepath.Base(loc.URI.Path()), loc.Range)
+ }
+
+ // Definition at the call"foo(123)" takes us to the Go declaration.
+ callLoc := env.RegexpSearch("a.go", regexp.QuoteMeta("foo(123)"))
+ declLoc := env.GoToDefinition(callLoc)
+ if got, want := locString(declLoc), "a.go:5:5-5:8"; got != want {
+ t.Errorf("Definition(call): got %s, want %s", got, want)
+ }
+
+ // Definition a second time takes us to the assembly implementation.
+ implLoc := env.GoToDefinition(declLoc)
+ if got, want := locString(implLoc), "foo_darwin_arm64.s:2:6-2:9"; got != want {
+ t.Errorf("Definition(go decl): got %s, want %s", got, want)
+ }
+ })
+}
