From 53eeb316f8c13ef858834888bfb29c31e7cf41b1 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Sun, 13 Apr 2025 11:28:15 +0800 Subject: [PATCH 01/14] gopls: add CompiledAsmFiles in cache.Package The CompiledAsmFiles field supports storing assembly files. Fixes golang/go#71754 --- gopls/internal/cache/check.go | 25 +++++++++++++++++------ gopls/internal/cache/load.go | 9 ++++++++ gopls/internal/cache/metadata/metadata.go | 1 + gopls/internal/cache/package.go | 2 ++ gopls/internal/cache/parse.go | 21 +++++++++++++++++++ 5 files changed, 52 insertions(+), 6 deletions(-) diff --git a/gopls/internal/cache/check.go b/gopls/internal/cache/check.go index fae6d57a8f8..4b6ec559d2a 100644 --- a/gopls/internal/cache/check.go +++ b/gopls/internal/cache/check.go @@ -1456,12 +1456,12 @@ type typeCheckInputs struct { id PackageID // Used for type checking: - pkgPath PackagePath - name PackageName - goFiles, compiledGoFiles []file.Handle - sizes types.Sizes - depsByImpPath map[ImportPath]PackageID - goVersion string // packages.Module.GoVersion, e.g. "1.18" + pkgPath PackagePath + name PackageName + goFiles, compiledGoFiles, asmFiles []file.Handle + sizes types.Sizes + depsByImpPath map[ImportPath]PackageID + goVersion string // packages.Module.GoVersion, e.g. "1.18" // Used for type check diagnostics: // TODO(rfindley): consider storing less data in gobDiagnostics, and @@ -1491,6 +1491,10 @@ func (s *Snapshot) typeCheckInputs(ctx context.Context, mp *metadata.Package) (* if err != nil { return nil, err } + asmFiles, err := readFiles(ctx, s, mp.AsmFiles) + if err != nil { + return nil, err + } goVersion := "" if mp.Module != nil && mp.Module.GoVersion != "" { @@ -1506,6 +1510,7 @@ func (s *Snapshot) typeCheckInputs(ctx context.Context, mp *metadata.Package) (* name: mp.Name, goFiles: goFiles, compiledGoFiles: compiledGoFiles, + asmFiles: asmFiles, sizes: mp.TypesSizes, depsByImpPath: mp.DepsByImpPath, goVersion: goVersion, @@ -1558,6 +1563,10 @@ func localPackageKey(inputs *typeCheckInputs) file.Hash { for _, fh := range inputs.goFiles { fmt.Fprintln(hasher, fh.Identity()) } + fmt.Fprintf(hasher, "asmFiles:%d\n", len(inputs.asmFiles)) + for _, fh := range inputs.asmFiles { + fmt.Fprintln(hasher, fh.Identity()) + } // types sizes wordSize := inputs.sizes.Sizeof(types.Typ[types.Int]) @@ -1614,6 +1623,10 @@ func (b *typeCheckBatch) checkPackage(ctx context.Context, fset *token.FileSet, pkg.parseErrors = append(pkg.parseErrors, pgf.ParseErr) } } + pkg.asmFiles, err = parseAsmFiles(ctx, inputs.asmFiles...) + if err != nil { + return nil, err + } // Use the default type information for the unsafe package. if inputs.pkgPath == "unsafe" { diff --git a/gopls/internal/cache/load.go b/gopls/internal/cache/load.go index ac905fa39de..844c55c971e 100644 --- a/gopls/internal/cache/load.go +++ b/gopls/internal/cache/load.go @@ -454,6 +454,15 @@ func buildMetadata(updates map[PackageID]*metadata.Package, loadDir string, stan *dst = append(*dst, protocol.URIFromPath(filename)) } } + copyAsmURIs := func(dst *[]protocol.DocumentURI, src []string) { + for _, filename := range src { + if !strings.HasSuffix(filename, ".s") { + continue + } + *dst = append(*dst, protocol.URIFromPath(filename)) + } + } + copyAsmURIs(&mp.AsmFiles, pkg.OtherFiles) copyURIs(&mp.CompiledGoFiles, pkg.CompiledGoFiles) copyURIs(&mp.GoFiles, pkg.GoFiles) copyURIs(&mp.IgnoredFiles, pkg.IgnoredFiles) diff --git a/gopls/internal/cache/metadata/metadata.go b/gopls/internal/cache/metadata/metadata.go index ae847d579be..711838094a8 100644 --- a/gopls/internal/cache/metadata/metadata.go +++ b/gopls/internal/cache/metadata/metadata.go @@ -48,6 +48,7 @@ type Package struct { // These fields are as defined by go/packages.Package GoFiles []protocol.DocumentURI CompiledGoFiles []protocol.DocumentURI + AsmFiles []protocol.DocumentURI IgnoredFiles []protocol.DocumentURI OtherFiles []protocol.DocumentURI diff --git a/gopls/internal/cache/package.go b/gopls/internal/cache/package.go index dde6e1fa671..be4c1ebef96 100644 --- a/gopls/internal/cache/package.go +++ b/gopls/internal/cache/package.go @@ -19,6 +19,7 @@ import ( "golang.org/x/tools/gopls/internal/cache/testfuncs" "golang.org/x/tools/gopls/internal/cache/xrefs" "golang.org/x/tools/gopls/internal/protocol" + "golang.org/x/tools/gopls/internal/util/asm" "golang.org/x/tools/gopls/internal/util/safetoken" ) @@ -50,6 +51,7 @@ type syntaxPackage struct { fset *token.FileSet // for now, same as the snapshot's FileSet goFiles []*parsego.File compiledGoFiles []*parsego.File + asmFiles []*asm.File diagnostics []*Diagnostic parseErrors []scanner.ErrorList typeErrors []types.Error diff --git a/gopls/internal/cache/parse.go b/gopls/internal/cache/parse.go index d733ca76799..0f41824d7ea 100644 --- a/gopls/internal/cache/parse.go +++ b/gopls/internal/cache/parse.go @@ -13,6 +13,7 @@ import ( "golang.org/x/tools/gopls/internal/cache/parsego" "golang.org/x/tools/gopls/internal/file" + "golang.org/x/tools/gopls/internal/util/asm" ) // ParseGo parses the file whose contents are provided by fh. @@ -43,3 +44,23 @@ func parseGoImpl(ctx context.Context, fset *token.FileSet, fh file.Handle, mode pgf, _ := parsego.Parse(ctx, fset, fh.URI(), content, mode, purgeFuncBodies) // ignore 'fixes' return pgf, nil } + +// parseAsmFiles pases the assembly files whose contents are provided by fhs. +func parseAsmFiles(ctx context.Context, fhs ...file.Handle) ([]*asm.File, error) { + pgfs := make([]*asm.File, len(fhs)) + + for i, fh := range fhs { + var err error + content, err := fh.Content() + if err != nil { + return nil, err + } + // Check for context cancellation before actually doing the parse. + if ctx.Err() != nil { + return nil, ctx.Err() + } + pgfs[i] = asm.Parse(content) + } + return pgfs, nil + +} From 40a7ea40c782b4c58ae92ca85fcf5397a511bf73 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Mon, 14 Apr 2025 08:05:12 +0800 Subject: [PATCH 02/14] gopls: add asmFiles in cache.Package The asmFiles field supports storing assembly files. Fixes golang/go#71754 --- gopls/internal/cache/load.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/gopls/internal/cache/load.go b/gopls/internal/cache/load.go index 844c55c971e..50ce955cc58 100644 --- a/gopls/internal/cache/load.go +++ b/gopls/internal/cache/load.go @@ -454,15 +454,13 @@ func buildMetadata(updates map[PackageID]*metadata.Package, loadDir string, stan *dst = append(*dst, protocol.URIFromPath(filename)) } } - copyAsmURIs := func(dst *[]protocol.DocumentURI, src []string) { - for _, filename := range src { - if !strings.HasSuffix(filename, ".s") { - continue - } - *dst = append(*dst, protocol.URIFromPath(filename)) + // Copy SFiles to AsmFiles. + for _, filename := range pkg.OtherFiles { + if !strings.HasSuffix(filename, ".s") { + continue } + mp.AsmFiles = append(mp.AsmFiles, protocol.URIFromPath(filename)) } - copyAsmURIs(&mp.AsmFiles, pkg.OtherFiles) copyURIs(&mp.CompiledGoFiles, pkg.CompiledGoFiles) copyURIs(&mp.GoFiles, pkg.GoFiles) copyURIs(&mp.IgnoredFiles, pkg.IgnoredFiles) From 9e2e104a5478011552e37cad98d41b7a9a75ec39 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Thu, 24 Apr 2025 23:19:05 +0800 Subject: [PATCH 03/14] gopls: augmentation of the xrefs index to include cross-package references from assembly files. This commit enhances the xrefs index to include references from assembly (.s) files. It processes assembly files, extracts cross-package references, and updates the index. Updates golang/go#71754 --- gopls/internal/cache/metadata/metadata.go | 4 +- gopls/internal/cache/package.go | 2 +- gopls/internal/cache/parse.go | 10 ++--- gopls/internal/cache/xrefs/xrefs.go | 45 +++++++++++++++++++++-- gopls/internal/goasm/definition.go | 2 +- gopls/internal/util/asm/parse.go | 13 ++++++- gopls/internal/util/asm/parse_test.go | 2 +- 7 files changed, 62 insertions(+), 16 deletions(-) diff --git a/gopls/internal/cache/metadata/metadata.go b/gopls/internal/cache/metadata/metadata.go index 711838094a8..f9a2a399d70 100644 --- a/gopls/internal/cache/metadata/metadata.go +++ b/gopls/internal/cache/metadata/metadata.go @@ -48,10 +48,12 @@ type Package struct { // These fields are as defined by go/packages.Package GoFiles []protocol.DocumentURI CompiledGoFiles []protocol.DocumentURI - AsmFiles []protocol.DocumentURI IgnoredFiles []protocol.DocumentURI OtherFiles []protocol.DocumentURI + // These fields ares as defined by asm.File + AsmFiles []protocol.DocumentURI + ForTest PackagePath // q in a "p [q.test]" package, else "" TypesSizes types.Sizes Errors []packages.Error // must be set for packages in import cycles diff --git a/gopls/internal/cache/package.go b/gopls/internal/cache/package.go index be4c1ebef96..645b114ecd9 100644 --- a/gopls/internal/cache/package.go +++ b/gopls/internal/cache/package.go @@ -72,7 +72,7 @@ type syntaxPackage struct { func (p *syntaxPackage) xrefs() []byte { p.xrefsOnce.Do(func() { - p._xrefs = xrefs.Index(p.compiledGoFiles, p.types, p.typesInfo) + p._xrefs = xrefs.Index(p.compiledGoFiles, p.types, p.typesInfo, p.asmFiles) }) return p._xrefs } diff --git a/gopls/internal/cache/parse.go b/gopls/internal/cache/parse.go index 0f41824d7ea..ec94b182997 100644 --- a/gopls/internal/cache/parse.go +++ b/gopls/internal/cache/parse.go @@ -45,10 +45,9 @@ func parseGoImpl(ctx context.Context, fset *token.FileSet, fh file.Handle, mode return pgf, nil } -// parseAsmFiles pases the assembly files whose contents are provided by fhs. +// parseAsmFiles parses the assembly files whose contents are provided by fhs. func parseAsmFiles(ctx context.Context, fhs ...file.Handle) ([]*asm.File, error) { - pgfs := make([]*asm.File, len(fhs)) - + pafs := make([]*asm.File, len(fhs)) for i, fh := range fhs { var err error content, err := fh.Content() @@ -59,8 +58,7 @@ func parseAsmFiles(ctx context.Context, fhs ...file.Handle) ([]*asm.File, error) if ctx.Err() != nil { return nil, ctx.Err() } - pgfs[i] = asm.Parse(content) + pafs[i] = asm.Parse(fh.URI(), content) } - return pgfs, nil - + return pafs, nil } diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index d9b7051737a..33fda837af3 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -17,13 +17,15 @@ import ( "golang.org/x/tools/gopls/internal/cache/metadata" "golang.org/x/tools/gopls/internal/cache/parsego" "golang.org/x/tools/gopls/internal/protocol" + "golang.org/x/tools/gopls/internal/util/asm" "golang.org/x/tools/gopls/internal/util/bug" "golang.org/x/tools/gopls/internal/util/frob" + "golang.org/x/tools/gopls/internal/util/morestrings" ) // Index constructs a serializable index of outbound cross-references // for the specified type-checked package. -func Index(files []*parsego.File, pkg *types.Package, info *types.Info) []byte { +func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles []*asm.File) []byte { // pkgObjects maps each referenced package Q to a mapping: // from each referenced symbol in Q to the ordered list // of references to that symbol from this package. @@ -70,7 +72,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info) []byte { // (e.g. local const/var/type). continue } - gobObj = &gobObject{Path: path} + gobObj = &gobObject{Path: path, isAsm: false} objects[obj] = gobObj } @@ -112,6 +114,37 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info) []byte { } } + for fileIndex, af := range asmFiles { + for _, id := range af.Idents { + _, name, ok := morestrings.CutLast(id.Name, ".") + if id.Kind != asm.Text { + continue + } + obj := pkg.Scope().Lookup(name) + if obj == nil { + continue + } + objects := getObjects(pkg) + gobObj, ok := objects[obj] + if !ok { + path, err := objectpathFor(obj) + if err != nil { + // Capitalized but not exported + // (e.g. local const/var/type). + continue + } + gobObj = &gobObject{Path: path, isAsm: true} + objects[obj] = gobObj + } + if rng, err := af.NodeRange(id); err == nil { + gobObj.Refs = append(gobObj.Refs, gobRef{ + FileIndex: fileIndex, + Range: rng, + }) + } + } + } + // Flatten the maps into slices, and sort for determinism. var packages []*gobPackage for p := range pkgObjects { @@ -148,6 +181,9 @@ func Lookup(mp *metadata.Package, data []byte, targets map[metadata.PackagePath] if _, ok := objectSet[gobObj.Path]; ok { for _, ref := range gobObj.Refs { uri := mp.CompiledGoFiles[ref.FileIndex] + if gobObj.isAsm { + uri = mp.AsmFiles[ref.FileIndex] + } locs = append(locs, protocol.Location{ URI: uri, Range: ref.Range, @@ -184,8 +220,9 @@ type gobPackage struct { // A gobObject records all references to a particular symbol. type gobObject struct { - Path objectpath.Path // symbol name within package; "" => import of package itself - Refs []gobRef // locations of references within P, in lexical order + Path objectpath.Path // symbol name within package; "" => import of package itself + Refs []gobRef // locations of references within P, in lexical order + isAsm bool // true if this is an assembly object } type gobRef struct { diff --git a/gopls/internal/goasm/definition.go b/gopls/internal/goasm/definition.go index b3c5251ed39..91bd554aac5 100644 --- a/gopls/internal/goasm/definition.go +++ b/gopls/internal/goasm/definition.go @@ -45,7 +45,7 @@ func Definition(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p // // TODO(adonovan): make this just another // attribute of the type-checked cache.Package. - file := asm.Parse(content) + file := asm.Parse(fh.URI(), content) // Figure out the selected symbol. // For now, just find the identifier around the cursor. diff --git a/gopls/internal/util/asm/parse.go b/gopls/internal/util/asm/parse.go index 11c59a7cc3d..448a26bc6d2 100644 --- a/gopls/internal/util/asm/parse.go +++ b/gopls/internal/util/asm/parse.go @@ -11,6 +11,8 @@ import ( "fmt" "strings" "unicode" + + "golang.org/x/tools/gopls/internal/protocol" ) // Kind describes the nature of an identifier in an assembly file. @@ -43,12 +45,19 @@ var kindString = [...]string{ // A file represents a parsed file of Go assembly language. type File struct { + URI protocol.DocumentURI Idents []Ident + Mapper *protocol.Mapper // may map fixed Src, not file content + // TODO(adonovan): use token.File? This may be important in a // future in which analyzers can report diagnostics in .s files. } +func (f *File) NodeRange(ident Ident) (protocol.Range, error) { + return f.Mapper.OffsetRange(ident.Offset, ident.End()) +} + // Ident represents an identifier in an assembly file. type Ident struct { Name string // symbol name (after correcting [·∕]); Name[0]='.' => current package @@ -61,7 +70,7 @@ func (id Ident) End() int { return id.Offset + len(id.Name) } // Parse extracts identifiers from Go assembly files. // Since it is a best-effort parser, it never returns an error. -func Parse(content []byte) *File { +func Parse(uri protocol.DocumentURI, content []byte) *File { var idents []Ident offset := 0 // byte offset of start of current line @@ -192,7 +201,7 @@ func Parse(content []byte) *File { _ = scan.Err() // ignore scan errors - return &File{Idents: idents} + return &File{Idents: idents, Mapper: protocol.NewMapper(uri, content), URI: uri} } // isIdent reports whether s is a valid Go assembly identifier. diff --git a/gopls/internal/util/asm/parse_test.go b/gopls/internal/util/asm/parse_test.go index 67a1286d28b..6d9bd7b8b93 100644 --- a/gopls/internal/util/asm/parse_test.go +++ b/gopls/internal/util/asm/parse_test.go @@ -39,7 +39,7 @@ TEXT ·g(SB),NOSPLIT,$0 `[1:]) const filename = "asm.s" m := protocol.NewMapper(protocol.URIFromPath(filename), src) - file := asm.Parse(src) + file := asm.Parse("", src) want := ` asm.s:5:6-11: data "hello" From 0beb6d852fddf52dc4f18f81b621f03fca469303 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Sun, 11 May 2025 15:16:44 +0800 Subject: [PATCH 04/14] gopls: Implement reference jumping between Go functions and assembly functions. This commit modifies the addition of reference relationships in assembly files within the references file. Updates golang/go#71754 --- gopls/internal/cache/package.go | 4 ++++ gopls/internal/cache/xrefs/xrefs.go | 3 +++ gopls/internal/golang/references.go | 25 +++++++++++++++++++++++++ 3 files changed, 32 insertions(+) diff --git a/gopls/internal/cache/package.go b/gopls/internal/cache/package.go index 645b114ecd9..7a0bb2a6187 100644 --- a/gopls/internal/cache/package.go +++ b/gopls/internal/cache/package.go @@ -214,3 +214,7 @@ func (p *Package) ParseErrors() []scanner.ErrorList { func (p *Package) TypeErrors() []types.Error { return p.pkg.typeErrors } + +func (p *Package) AsmFiles() []*asm.File { + return p.pkg.asmFiles +} diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index 33fda837af3..52728d69100 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -117,6 +117,9 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles for fileIndex, af := range asmFiles { for _, id := range af.Idents { _, name, ok := morestrings.CutLast(id.Name, ".") + if !ok { + continue + } if id.Kind != asm.Text { continue } diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index 140e20c01fd..b6823c36c40 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -33,7 +33,9 @@ import ( "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/asm" "golang.org/x/tools/gopls/internal/util/cursorutil" + "golang.org/x/tools/gopls/internal/util/morestrings" "golang.org/x/tools/gopls/internal/util/safetoken" "golang.org/x/tools/internal/astutil" "golang.org/x/tools/internal/event" @@ -608,6 +610,29 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo } } } + + for _, pgf := range pkg.AsmFiles() { + for _, id := range pgf.Idents { + _, name, ok := morestrings.CutLast(id.Name, ".") + if !ok { + continue + } + if id.Kind != asm.Text { + continue + } + obj := pkg.Types().Scope().Lookup(name) + if obj == nil { + continue + } + if rng, err := pgf.NodeRange(id); err == nil && matches(obj) { + asmLocation := protocol.Location{ + URI: pgf.URI, + Range: rng, + } + report(asmLocation, false) + } + } + } return nil } From abc48dec670b948344a4977adf9346e47bc24b80 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Sun, 25 May 2025 13:19:28 +0800 Subject: [PATCH 05/14] gopls: implement reference support for goasm files This commit adds support for reference relationships in Go assembly (.goasm) files within the references file. It implements the logic to identify and record references in goasm file types, enabling reference lookups and navigation for Go assembly code. Updates golang/go#71754 --- gopls/internal/cache/package.go | 14 ++++ gopls/internal/goasm/references.go | 117 ++++++++++++++++++++++++++++ gopls/internal/golang/references.go | 2 +- gopls/internal/server/references.go | 3 + 4 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 gopls/internal/goasm/references.go diff --git a/gopls/internal/cache/package.go b/gopls/internal/cache/package.go index 7a0bb2a6187..95ab27e4e61 100644 --- a/gopls/internal/cache/package.go +++ b/gopls/internal/cache/package.go @@ -218,3 +218,17 @@ func (p *Package) TypeErrors() []types.Error { func (p *Package) AsmFiles() []*asm.File { return p.pkg.asmFiles } + +func (p *Package) AsmFile(uri protocol.DocumentURI) (*asm.File, error) { + return p.pkg.AsmFile(uri) +} + +func (pkg *syntaxPackage) AsmFile(uri protocol.DocumentURI) (*asm.File, error) { + for _, af := range pkg.asmFiles { + if af.URI == uri { + return af, nil + } + } + + return nil, fmt.Errorf("no parsed file for %s in %v", uri, pkg.id) +} diff --git a/gopls/internal/goasm/references.go b/gopls/internal/goasm/references.go new file mode 100644 index 00000000000..db72ba6512f --- /dev/null +++ b/gopls/internal/goasm/references.go @@ -0,0 +1,117 @@ +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Package goasm provides language-server features for files in Go +// assembly language (https://go.dev/doc/asm). +package goasm + +import ( + "context" + "fmt" + "go/ast" + + "golang.org/x/tools/gopls/internal/cache" + "golang.org/x/tools/gopls/internal/cache/metadata" + "golang.org/x/tools/gopls/internal/file" + "golang.org/x/tools/gopls/internal/protocol" + "golang.org/x/tools/gopls/internal/util/asm" + "golang.org/x/tools/gopls/internal/util/morestrings" + "golang.org/x/tools/internal/event" +) + +func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position) ([]protocol.Location, error) { + ctx, done := event.Start(ctx, "goasm.References") + defer done() + + pkg, asmFile, err := GetPackageID(ctx, snapshot, fh.URI()) + if err != nil { + return nil, err + } + + // Read the file. + content, err := fh.Content() + if err != nil { + return nil, err + } + mapper := protocol.NewMapper(fh.URI(), content) + offset, err := mapper.PositionOffset(position) + if err != nil { + return nil, err + } + + // // Parse the assembly. + // file := asm.Parse(fh.URI(), content) + + // Figure out the selected symbol. + // For now, just find the identifier around the cursor. + var found *asm.Ident + for _, id := range asmFile.Idents { + if id.Offset <= offset && offset <= id.End() { + found = &id + break + } + } + if found == nil { + return nil, fmt.Errorf("not an identifier") + } + + sym := found.Name + var locations []protocol.Location + _, name, ok := morestrings.CutLast(sym, ".") + if !ok { + return nil, fmt.Errorf("not found") + } + // return localReferences(pkg, targets, true, report) + for _, pgf := range pkg.CompiledGoFiles() { + for curId := range pgf.Cursor.Preorder((*ast.Ident)(nil)) { + id := curId.Node().(*ast.Ident) + if id.Name == name { + loc, err := pgf.NodeLocation(id) + if err != nil { + return nil, err + } + locations = append(locations, loc) + } + } + } + + for _, asmFile := range pkg.AsmFiles() { + for _, id := range asmFile.Idents { + if id.Name != sym { + continue + } + if rng, err := asmFile.NodeRange(id); err == nil { + asmLocation := protocol.Location{ + URI: asmFile.URI, + Range: rng, + } + locations = append(locations, asmLocation) + } + } + } + + return locations, nil +} + +func GetPackageID(ctx context.Context, snapshot *cache.Snapshot, uri protocol.DocumentURI) (*cache.Package, *asm.File, error) { + mps, err := snapshot.MetadataForFile(ctx, uri) + if err != nil { + return nil, nil, err + } + metadata.RemoveIntermediateTestVariants(&mps) + if len(mps) == 0 { + return nil, nil, fmt.Errorf("no package metadata for file %s", uri) + } + mp := mps[0] + pkgs, err := snapshot.TypeCheck(ctx, mp.ID) + if err != nil { + return nil, nil, err + } + pkg := pkgs[0] + asmFile, err := pkg.AsmFile(uri) + if err != nil { + return nil, nil, err // "can't happen" + } + return pkg, asmFile, nil +} diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index b6823c36c40..74ea0607c40 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -629,7 +629,7 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo URI: pgf.URI, Range: rng, } - report(asmLocation, false) + report(asmLocation, true) } } } diff --git a/gopls/internal/server/references.go b/gopls/internal/server/references.go index 2916f27c093..bb4981a5514 100644 --- a/gopls/internal/server/references.go +++ b/gopls/internal/server/references.go @@ -8,6 +8,7 @@ import ( "context" "golang.org/x/tools/gopls/internal/file" + "golang.org/x/tools/gopls/internal/goasm" "golang.org/x/tools/gopls/internal/golang" "golang.org/x/tools/gopls/internal/label" "golang.org/x/tools/gopls/internal/protocol" @@ -35,6 +36,8 @@ func (s *server) References(ctx context.Context, params *protocol.ReferenceParam return template.References(ctx, snapshot, fh, params) case file.Go: return golang.References(ctx, snapshot, fh, params.Position, params.Context.IncludeDeclaration) + case file.Asm: + return goasm.References(ctx, snapshot, fh, params.Position) } return nil, nil // empty result } From cd50d90aaaac63a658a0091b8f947814cb701594 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Wed, 28 May 2025 23:00:48 +0800 Subject: [PATCH 06/14] gopls: Add test cases for references in assembly and Go files Improve code comments for better readability.Remove type checking logic from assembly file reference lookup.Update xrefs index retrieval for consistency Updates golang/go#71754 --- gopls/internal/cache/metadata/metadata.go | 3 +- gopls/internal/cache/package.go | 2 +- gopls/internal/cache/xrefs/xrefs.go | 25 +++----- gopls/internal/goasm/references.go | 59 ++++++++----------- gopls/internal/golang/references.go | 7 +-- gopls/internal/server/references.go | 2 +- .../marker/testdata/references/asm_ref.txt | 25 ++++++++ gopls/internal/util/asm/parse.go | 4 +- 8 files changed, 65 insertions(+), 62 deletions(-) create mode 100644 gopls/internal/test/marker/testdata/references/asm_ref.txt diff --git a/gopls/internal/cache/metadata/metadata.go b/gopls/internal/cache/metadata/metadata.go index f9a2a399d70..521b240d5d3 100644 --- a/gopls/internal/cache/metadata/metadata.go +++ b/gopls/internal/cache/metadata/metadata.go @@ -51,8 +51,7 @@ type Package struct { IgnoredFiles []protocol.DocumentURI OtherFiles []protocol.DocumentURI - // These fields ares as defined by asm.File - AsmFiles []protocol.DocumentURI + AsmFiles []protocol.DocumentURI // *.s subset of OtherFiles ForTest PackagePath // q in a "p [q.test]" package, else "" TypesSizes types.Sizes diff --git a/gopls/internal/cache/package.go b/gopls/internal/cache/package.go index 95ab27e4e61..2e0ae6275e4 100644 --- a/gopls/internal/cache/package.go +++ b/gopls/internal/cache/package.go @@ -227,7 +227,7 @@ func (pkg *syntaxPackage) AsmFile(uri protocol.DocumentURI) (*asm.File, error) { for _, af := range pkg.asmFiles { if af.URI == uri { return af, nil - } + } } return nil, fmt.Errorf("no parsed file for %s in %v", uri, pkg.id) diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index 52728d69100..7760a5329d2 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -11,6 +11,7 @@ package xrefs import ( "go/ast" "go/types" + "slices" "sort" "golang.org/x/tools/go/types/objectpath" @@ -72,7 +73,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles // (e.g. local const/var/type). continue } - gobObj = &gobObject{Path: path, isAsm: false} + gobObj = &gobObject{Path: path} objects[obj] = gobObj } @@ -114,15 +115,13 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles } } + // For each asm file, record references to identifiers. for fileIndex, af := range asmFiles { for _, id := range af.Idents { _, name, ok := morestrings.CutLast(id.Name, ".") if !ok { continue } - if id.Kind != asm.Text { - continue - } obj := pkg.Scope().Lookup(name) if obj == nil { continue @@ -130,13 +129,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles objects := getObjects(pkg) gobObj, ok := objects[obj] if !ok { - path, err := objectpathFor(obj) - if err != nil { - // Capitalized but not exported - // (e.g. local const/var/type). - continue - } - gobObj = &gobObject{Path: path, isAsm: true} + gobObj = &gobObject{Path: objectpath.Path(obj.Name())} objects[obj] = gobObj } if rng, err := af.NodeRange(id); err == nil { @@ -183,10 +176,7 @@ func Lookup(mp *metadata.Package, data []byte, targets map[metadata.PackagePath] for _, gobObj := range gp.Objects { if _, ok := objectSet[gobObj.Path]; ok { for _, ref := range gobObj.Refs { - uri := mp.CompiledGoFiles[ref.FileIndex] - if gobObj.isAsm { - uri = mp.AsmFiles[ref.FileIndex] - } + uri := slices.Concat(mp.CompiledGoFiles, mp.AsmFiles)[ref.FileIndex] locs = append(locs, protocol.Location{ URI: uri, Range: ref.Range, @@ -223,9 +213,8 @@ type gobPackage struct { // A gobObject records all references to a particular symbol. type gobObject struct { - Path objectpath.Path // symbol name within package; "" => import of package itself - Refs []gobRef // locations of references within P, in lexical order - isAsm bool // true if this is an assembly object + Path objectpath.Path // symbol name within package; "" => import of package itself + Refs []gobRef // locations of references within P, in lexical order } type gobRef struct { diff --git a/gopls/internal/goasm/references.go b/gopls/internal/goasm/references.go index db72ba6512f..58f2bd8c269 100644 --- a/gopls/internal/goasm/references.go +++ b/gopls/internal/goasm/references.go @@ -20,29 +20,36 @@ import ( "golang.org/x/tools/internal/event" ) -func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position) ([]protocol.Location, error) { +// References returns a list of locations (file and position) where the symbol under the cursor in an assembly file is referenced, +// including both Go source files and assembly files within the same package. +func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, position protocol.Position, includeDeclaration bool) ([]protocol.Location, error) { ctx, done := event.Start(ctx, "goasm.References") defer done() - pkg, asmFile, err := GetPackageID(ctx, snapshot, fh.URI()) + mps, err := snapshot.MetadataForFile(ctx, fh.URI()) if err != nil { return nil, err } - - // Read the file. - content, err := fh.Content() + metadata.RemoveIntermediateTestVariants(&mps) + if len(mps) == 0 { + return nil, fmt.Errorf("no package metadata for file %s", fh.URI()) + } + mp := mps[0] + pkgs, err := snapshot.TypeCheck(ctx, mp.ID) if err != nil { return nil, err } - mapper := protocol.NewMapper(fh.URI(), content) - offset, err := mapper.PositionOffset(position) + pkg := pkgs[0] + asmFile, err := pkg.AsmFile(fh.URI()) + if err != nil { + return nil, err // "can't happen" + } + + offset, err := asmFile.Mapper.PositionOffset(position) if err != nil { return nil, err } - // // Parse the assembly. - // file := asm.Parse(fh.URI(), content) - // Figure out the selected symbol. // For now, just find the identifier around the cursor. var found *asm.Ident @@ -62,7 +69,10 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p if !ok { return nil, fmt.Errorf("not found") } - // return localReferences(pkg, targets, true, report) + + // TODO(grootguo): Currently, only references to the symbol within the package are found (i.e., only Idents in this package's Go files are searched). + // It is still necessary to implement cross-package reference lookup: that is, to find all references to this symbol in other packages that import the current package. + // Refer to the global search logic in golang.References, and add corresponding test cases for verification. for _, pgf := range pkg.CompiledGoFiles() { for curId := range pgf.Cursor.Preorder((*ast.Ident)(nil)) { id := curId.Node().(*ast.Ident) @@ -76,6 +86,11 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p } } + // If includeDeclaration is false, return only reference locations (exclude declarations). + if !includeDeclaration { + return locations, nil + } + for _, asmFile := range pkg.AsmFiles() { for _, id := range asmFile.Idents { if id.Name != sym { @@ -93,25 +108,3 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p return locations, nil } - -func GetPackageID(ctx context.Context, snapshot *cache.Snapshot, uri protocol.DocumentURI) (*cache.Package, *asm.File, error) { - mps, err := snapshot.MetadataForFile(ctx, uri) - if err != nil { - return nil, nil, err - } - metadata.RemoveIntermediateTestVariants(&mps) - if len(mps) == 0 { - return nil, nil, fmt.Errorf("no package metadata for file %s", uri) - } - mp := mps[0] - pkgs, err := snapshot.TypeCheck(ctx, mp.ID) - if err != nil { - return nil, nil, err - } - pkg := pkgs[0] - asmFile, err := pkg.AsmFile(uri) - if err != nil { - return nil, nil, err // "can't happen" - } - return pkg, asmFile, nil -} diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index 74ea0607c40..fbe06bd3eaa 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -33,7 +33,6 @@ import ( "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/asm" "golang.org/x/tools/gopls/internal/util/cursorutil" "golang.org/x/tools/gopls/internal/util/morestrings" "golang.org/x/tools/gopls/internal/util/safetoken" @@ -611,15 +610,13 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo } } + // Iterate over all assembly files and find all references to the target object. for _, pgf := range pkg.AsmFiles() { for _, id := range pgf.Idents { _, name, ok := morestrings.CutLast(id.Name, ".") if !ok { continue } - if id.Kind != asm.Text { - continue - } obj := pkg.Types().Scope().Lookup(name) if obj == nil { continue @@ -629,7 +626,7 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo URI: pgf.URI, Range: rng, } - report(asmLocation, true) + report(asmLocation, false) } } } diff --git a/gopls/internal/server/references.go b/gopls/internal/server/references.go index bb4981a5514..251a95b352c 100644 --- a/gopls/internal/server/references.go +++ b/gopls/internal/server/references.go @@ -37,7 +37,7 @@ func (s *server) References(ctx context.Context, params *protocol.ReferenceParam case file.Go: return golang.References(ctx, snapshot, fh, params.Position, params.Context.IncludeDeclaration) case file.Asm: - return goasm.References(ctx, snapshot, fh, params.Position) + return goasm.References(ctx, snapshot, fh, params.Position, params.Context.IncludeDeclaration) } return nil, nil // empty result } diff --git a/gopls/internal/test/marker/testdata/references/asm_ref.txt b/gopls/internal/test/marker/testdata/references/asm_ref.txt new file mode 100644 index 00000000000..564834a8d70 --- /dev/null +++ b/gopls/internal/test/marker/testdata/references/asm_ref.txt @@ -0,0 +1,25 @@ + +-- go.mod -- +module example.com +go 1.24 + +-- foo/foo.go -- +package foo + +func Add(a, b int) int //@ loc(use, "Add"), refs("Add", use, def) +func sub(a, b int) int //@ loc(useSub, "sub"), refs("sub", useSub, defSub, refSub) +var _ = sub //@loc(refSub, "sub"), refs("sub", useSub, defSub, refSub) + +-- foo/foo.s -- +// portable assembly +#include "textflag.h" + +TEXT ·Add(SB), NOSPLIT, $0-24 //@ loc(def, "Add"), refs("Add", def, use) + MOVQ a+0(FP), AX + ADDQ b+8(FP), AX + RET + +TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "sub"), refs("sub", defSub, useSub, refSub) + MOVQ a+0(FP), AX + SUBQ b+8(FP), AX + RET diff --git a/gopls/internal/util/asm/parse.go b/gopls/internal/util/asm/parse.go index 448a26bc6d2..16e7549059a 100644 --- a/gopls/internal/util/asm/parse.go +++ b/gopls/internal/util/asm/parse.go @@ -48,14 +48,14 @@ type File struct { URI protocol.DocumentURI Idents []Ident - Mapper *protocol.Mapper // may map fixed Src, not file content + Mapper *protocol.Mapper // TODO(adonovan): use token.File? This may be important in a // future in which analyzers can report diagnostics in .s files. } func (f *File) NodeRange(ident Ident) (protocol.Range, error) { - return f.Mapper.OffsetRange(ident.Offset, ident.End()) + return f.Mapper.OffsetRange(ident.Offset+2, ident.End()+1) } // Ident represents an identifier in an assembly file. From 8b193bf09b85fcf02d6d3bfc224811c352ab39c0 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Tue, 3 Jun 2025 22:00:07 +0800 Subject: [PATCH 07/14] gopls: Add TODO for handling cross-package references and refine local reference matching logic Updated the code in xrefs.go and references.go. Updates golang/go#71754 --- gopls/internal/cache/xrefs/xrefs.go | 2 ++ gopls/internal/golang/references.go | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index 7760a5329d2..6b1791b24a8 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -124,6 +124,8 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles } obj := pkg.Scope().Lookup(name) if obj == nil { + // TODO(grootguo): If the object is not found in the current package, + // consider handling cross-package references. continue } objects := getObjects(pkg) diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index fbe06bd3eaa..497d7614255 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -621,7 +621,10 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo if obj == nil { continue } - if rng, err := pgf.NodeRange(id); err == nil && matches(obj) { + if !matches(obj) { + continue + } + if rng, err := pgf.NodeRange(id); err == nil { asmLocation := protocol.Location{ URI: pgf.URI, Range: rng, From 25c8b569a0e1c52eca09099aa1b948214ab2bfde Mon Sep 17 00:00:00 2001 From: groot-guo Date: Wed, 4 Jun 2025 20:39:30 +0800 Subject: [PATCH 08/14] gopls: Refactor goasm.References function. Leveraged Defs and Uses mappings from types.Info to efficiently find references in Go files. Updates golang/go#71754 --- gopls/internal/goasm/references.go | 37 +++++++++++++------ .../test/marker/testdata/references/asm.txt | 36 ++++++++++++++++++ .../marker/testdata/references/asm_ref.txt | 25 ------------- gopls/internal/util/asm/parse.go | 5 +++ 4 files changed, 67 insertions(+), 36 deletions(-) create mode 100644 gopls/internal/test/marker/testdata/references/asm.txt delete mode 100644 gopls/internal/test/marker/testdata/references/asm_ref.txt diff --git a/gopls/internal/goasm/references.go b/gopls/internal/goasm/references.go index 58f2bd8c269..f7a09ca0fd3 100644 --- a/gopls/internal/goasm/references.go +++ b/gopls/internal/goasm/references.go @@ -10,6 +10,7 @@ import ( "context" "fmt" "go/ast" + "go/types" "golang.org/x/tools/gopls/internal/cache" "golang.org/x/tools/gopls/internal/cache/metadata" @@ -73,16 +74,34 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p // TODO(grootguo): Currently, only references to the symbol within the package are found (i.e., only Idents in this package's Go files are searched). // It is still necessary to implement cross-package reference lookup: that is, to find all references to this symbol in other packages that import the current package. // Refer to the global search logic in golang.References, and add corresponding test cases for verification. + obj := pkg.Types().Scope().Lookup(name) + matches := func(curObj types.Object) bool { + if curObj == nil { + return false + } + if curObj.Name() != obj.Name() { + return false + } + return true + } for _, pgf := range pkg.CompiledGoFiles() { for curId := range pgf.Cursor.Preorder((*ast.Ident)(nil)) { id := curId.Node().(*ast.Ident) - if id.Name == name { - loc, err := pgf.NodeLocation(id) - if err != nil { - return nil, err + curObj, ok := pkg.TypesInfo().Defs[id] + if !ok { + curObj, ok = pkg.TypesInfo().Uses[id] + if !ok { + continue } - locations = append(locations, loc) } + if !matches(curObj) { + continue + } + loc, err := pgf.NodeLocation(id) + if err != nil { + return nil, err + } + locations = append(locations, loc) } } @@ -96,12 +115,8 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p if id.Name != sym { continue } - if rng, err := asmFile.NodeRange(id); err == nil { - asmLocation := protocol.Location{ - URI: asmFile.URI, - Range: rng, - } - locations = append(locations, asmLocation) + if loc, err := asmFile.NodeLocation(id); err == nil { + locations = append(locations, loc) } } } diff --git a/gopls/internal/test/marker/testdata/references/asm.txt b/gopls/internal/test/marker/testdata/references/asm.txt new file mode 100644 index 00000000000..1bdfad56cb8 --- /dev/null +++ b/gopls/internal/test/marker/testdata/references/asm.txt @@ -0,0 +1,36 @@ +This test validates the References request functionality in Go assembly files. + +It ensures that references to both exported (`Add`) and unexported (`sub`) functions are correctly identified across Go and assembly files. The test covers: +- Locating the definition of functions in both Go and assembly files. +- Identifying all references to the functions (`Add` and `sub`) within the Go and assembly files. + +The test includes: +- `Add`: An exported function with references in both Go and assembly files. +- `sub`: An unexported function with references in both Go and assembly files, including a usage in Go code (`var _ = sub`). + +The assembly file demonstrates portable assembly syntax and verifies cross-file reference handling. + +-- go.mod -- +module example.com +go 1.24 + +-- foo/foo.go -- +package foo + +func Add(a, b int) int //@ loc(use, "Add"), refs("Add", use, def) +func sub(a, b int) int //@ loc(useSub, "sub"), refs("sub", useSub, defSub, refSub) +var _ = sub //@loc(refSub, "sub"), refs("sub", useSub, defSub, refSub) + +-- foo/foo.s -- +// portable assembly +#include "textflag.h" + +TEXT ·Add(SB), NOSPLIT, $0-24 //@ loc(def, "Add"), refs("Add", def, use) + MOVQ a+0(FP), AX + ADDQ b+8(FP), AX + RET + +TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "sub"), refs("sub", defSub, useSub, refSub) + MOVQ a+0(FP), AX + SUBQ b+8(FP), AX + RET diff --git a/gopls/internal/test/marker/testdata/references/asm_ref.txt b/gopls/internal/test/marker/testdata/references/asm_ref.txt deleted file mode 100644 index 564834a8d70..00000000000 --- a/gopls/internal/test/marker/testdata/references/asm_ref.txt +++ /dev/null @@ -1,25 +0,0 @@ - --- go.mod -- -module example.com -go 1.24 - --- foo/foo.go -- -package foo - -func Add(a, b int) int //@ loc(use, "Add"), refs("Add", use, def) -func sub(a, b int) int //@ loc(useSub, "sub"), refs("sub", useSub, defSub, refSub) -var _ = sub //@loc(refSub, "sub"), refs("sub", useSub, defSub, refSub) - --- foo/foo.s -- -// portable assembly -#include "textflag.h" - -TEXT ·Add(SB), NOSPLIT, $0-24 //@ loc(def, "Add"), refs("Add", def, use) - MOVQ a+0(FP), AX - ADDQ b+8(FP), AX - RET - -TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "sub"), refs("sub", defSub, useSub, refSub) - MOVQ a+0(FP), AX - SUBQ b+8(FP), AX - RET diff --git a/gopls/internal/util/asm/parse.go b/gopls/internal/util/asm/parse.go index 16e7549059a..dc43c5f783a 100644 --- a/gopls/internal/util/asm/parse.go +++ b/gopls/internal/util/asm/parse.go @@ -58,6 +58,11 @@ func (f *File) NodeRange(ident Ident) (protocol.Range, error) { return f.Mapper.OffsetRange(ident.Offset+2, ident.End()+1) } +// NodeLocation returns a protocol Location for the ast.Node interval in this file. +func (f *File) NodeLocation(ident Ident) (protocol.Location, error) { + return f.Mapper.OffsetLocation(ident.Offset+2, ident.End()+1) +} + // Ident represents an identifier in an assembly file. type Ident struct { Name string // symbol name (after correcting [·∕]); Name[0]='.' => current package From 334ea2a6d787841d2bd62f997bf27c323a514e27 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Wed, 25 Jun 2025 23:16:51 +0800 Subject: [PATCH 09/14] gopls: Refactor goasm.References function. Modify the file index logic for asmfile in xrefs.go. Adjust the overall code logic flow in references. When parsing asm files, add the original symbol name length, return the original position, and update the test files. Updates golang/go#71754 --- gopls/internal/cache/xrefs/xrefs.go | 20 +++++++-- gopls/internal/goasm/references.go | 42 ++++++------------- gopls/internal/golang/references.go | 4 ++ .../test/marker/testdata/references/asm.txt | 4 +- gopls/internal/util/asm/parse.go | 25 ++++++----- 5 files changed, 49 insertions(+), 46 deletions(-) diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index 6b1791b24a8..84d84b864d3 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -11,7 +11,6 @@ package xrefs import ( "go/ast" "go/types" - "slices" "sort" "golang.org/x/tools/go/types/objectpath" @@ -118,6 +117,9 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles // For each asm file, record references to identifiers. for fileIndex, af := range asmFiles { for _, id := range af.Idents { + if id.Kind != asm.Data && id.Kind != asm.Text { + continue + } _, name, ok := morestrings.CutLast(id.Name, ".") if !ok { continue @@ -131,6 +133,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles objects := getObjects(pkg) gobObj, ok := objects[obj] if !ok { + // obj is a package-level symbol, so its objectpath is just its name. gobObj = &gobObject{Path: objectpath.Path(obj.Name())} objects[obj] = gobObj } @@ -171,14 +174,25 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles // to any object in the target set. Each object is denoted by a pair // of (package path, object path). func Lookup(mp *metadata.Package, data []byte, targets map[metadata.PackagePath]map[objectpath.Path]struct{}) (locs []protocol.Location) { - var packages []*gobPackage + var ( + packages []*gobPackage + goFilesLen = len(mp.CompiledGoFiles) + goAsmFilesLen = len(mp.AsmFiles) + ) packageCodec.Decode(data, &packages) for _, gp := range packages { if objectSet, ok := targets[gp.PkgPath]; ok { for _, gobObj := range gp.Objects { if _, ok := objectSet[gobObj.Path]; ok { for _, ref := range gobObj.Refs { - uri := slices.Concat(mp.CompiledGoFiles, mp.AsmFiles)[ref.FileIndex] + var uri protocol.DocumentURI + if ref.FileIndex < goFilesLen { + uri = mp.CompiledGoFiles[ref.FileIndex] + } else if ref.FileIndex < goFilesLen+goAsmFilesLen { + uri = mp.AsmFiles[ref.FileIndex] + } else { + continue // out of bounds + } locs = append(locs, protocol.Location{ URI: uri, Range: ref.Range, diff --git a/gopls/internal/goasm/references.go b/gopls/internal/goasm/references.go index f7a09ca0fd3..22bd718fd2c 100644 --- a/gopls/internal/goasm/references.go +++ b/gopls/internal/goasm/references.go @@ -64,9 +64,8 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p return nil, fmt.Errorf("not an identifier") } - sym := found.Name var locations []protocol.Location - _, name, ok := morestrings.CutLast(sym, ".") + _, name, ok := morestrings.CutLast(found.Name, ".") if !ok { return nil, fmt.Errorf("not found") } @@ -76,25 +75,12 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p // Refer to the global search logic in golang.References, and add corresponding test cases for verification. obj := pkg.Types().Scope().Lookup(name) matches := func(curObj types.Object) bool { - if curObj == nil { - return false - } - if curObj.Name() != obj.Name() { - return false - } - return true + return curObj != nil && curObj.Name() == obj.Name() } for _, pgf := range pkg.CompiledGoFiles() { for curId := range pgf.Cursor.Preorder((*ast.Ident)(nil)) { id := curId.Node().(*ast.Ident) - curObj, ok := pkg.TypesInfo().Defs[id] - if !ok { - curObj, ok = pkg.TypesInfo().Uses[id] - if !ok { - continue - } - } - if !matches(curObj) { + if !matches(pkg.TypesInfo().ObjectOf(id)) { continue } loc, err := pgf.NodeLocation(id) @@ -105,21 +91,17 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p } } - // If includeDeclaration is false, return only reference locations (exclude declarations). - if !includeDeclaration { - return locations, nil - } - - for _, asmFile := range pkg.AsmFiles() { - for _, id := range asmFile.Idents { - if id.Name != sym { - continue - } - if loc, err := asmFile.NodeLocation(id); err == nil { - locations = append(locations, loc) + if includeDeclaration { + for _, asmFile := range pkg.AsmFiles() { + for _, id := range asmFile.Idents { + if id.Name == found.Name && + (id.Kind == asm.Text || id.Kind == asm.Global || id.Kind == asm.Label) { + if loc, err := asmFile.NodeLocation(id); err == nil { + locations = append(locations, loc) + } + } } } } - return locations, nil } diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index 497d7614255..d977b691360 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -33,6 +33,7 @@ import ( "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/asm" "golang.org/x/tools/gopls/internal/util/cursorutil" "golang.org/x/tools/gopls/internal/util/morestrings" "golang.org/x/tools/gopls/internal/util/safetoken" @@ -613,6 +614,9 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo // Iterate over all assembly files and find all references to the target object. for _, pgf := range pkg.AsmFiles() { for _, id := range pgf.Idents { + if id.Kind != asm.Data && id.Kind != asm.Text { + continue + } _, name, ok := morestrings.CutLast(id.Name, ".") if !ok { continue diff --git a/gopls/internal/test/marker/testdata/references/asm.txt b/gopls/internal/test/marker/testdata/references/asm.txt index 1bdfad56cb8..07cc4a46034 100644 --- a/gopls/internal/test/marker/testdata/references/asm.txt +++ b/gopls/internal/test/marker/testdata/references/asm.txt @@ -25,12 +25,12 @@ var _ = sub //@loc(refSub, "sub"), refs("sub", useSub, defSub, refSub) // portable assembly #include "textflag.h" -TEXT ·Add(SB), NOSPLIT, $0-24 //@ loc(def, "Add"), refs("Add", def, use) +TEXT ·Add(SB), NOSPLIT, $0-24 //@ loc(def, "·Add"), refs("Add", def, use) MOVQ a+0(FP), AX ADDQ b+8(FP), AX RET -TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "sub"), refs("sub", defSub, useSub, refSub) +TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "·sub"), refs("sub", defSub, useSub, refSub) MOVQ a+0(FP), AX SUBQ b+8(FP), AX RET diff --git a/gopls/internal/util/asm/parse.go b/gopls/internal/util/asm/parse.go index dc43c5f783a..5437c95ff0f 100644 --- a/gopls/internal/util/asm/parse.go +++ b/gopls/internal/util/asm/parse.go @@ -55,19 +55,20 @@ type File struct { } func (f *File) NodeRange(ident Ident) (protocol.Range, error) { - return f.Mapper.OffsetRange(ident.Offset+2, ident.End()+1) + return f.Mapper.OffsetRange(ident.Offset, ident.Offset+ident.OrigLen) } // NodeLocation returns a protocol Location for the ast.Node interval in this file. func (f *File) NodeLocation(ident Ident) (protocol.Location, error) { - return f.Mapper.OffsetLocation(ident.Offset+2, ident.End()+1) + return f.Mapper.OffsetLocation(ident.Offset, ident.Offset+ident.OrigLen) } // Ident represents an identifier in an assembly file. type Ident struct { - Name string // symbol name (after correcting [·∕]); Name[0]='.' => current package - Offset int // zero-based byte offset - Kind Kind + Name string // symbol name (after correcting [·∕]); Name[0]='.' => current package + Offset int // zero-based byte offset + OrigLen int // original length of the symbol name (before cleanup) + Kind Kind } // End returns the identifier's end offset. @@ -136,9 +137,10 @@ func Parse(uri protocol.DocumentURI, content []byte) *File { if isIdent(sym) { // (The Index call assumes sym is not itself "TEXT" etc.) idents = append(idents, Ident{ - Name: cleanup(sym), - Kind: kind, - Offset: offset + strings.Index(line, sym), + Name: cleanup(sym), + Kind: kind, + Offset: offset + strings.Index(line, sym), + OrigLen: len(sym), }) } continue @@ -196,9 +198,10 @@ func Parse(uri protocol.DocumentURI, content []byte) *File { sym = cutBefore(sym, "<") // "sym" =>> "sym" if isIdent(sym) { idents = append(idents, Ident{ - Name: cleanup(sym), - Kind: Ref, - Offset: offset + tokenPos, + Name: cleanup(sym), + Kind: Ref, + Offset: offset + tokenPos, + OrigLen: len(sym), }) } } From ab1c57db752e768090161603907fd5928db55fa0 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Fri, 27 Jun 2025 20:15:32 +0800 Subject: [PATCH 10/14] gopls: Refactor xrefs.go Lookup function. Modify the logic for distinguishing between asm files and Go files. Add test examples. Updates golang/go#71754 --- gopls/internal/cache/xrefs/xrefs.go | 12 ++++-------- .../internal/test/marker/testdata/references/asm.txt | 5 +++++ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index 84d84b864d3..1215ac3fc85 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -175,9 +175,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles // of (package path, object path). func Lookup(mp *metadata.Package, data []byte, targets map[metadata.PackagePath]map[objectpath.Path]struct{}) (locs []protocol.Location) { var ( - packages []*gobPackage - goFilesLen = len(mp.CompiledGoFiles) - goAsmFilesLen = len(mp.AsmFiles) + packages []*gobPackage ) packageCodec.Decode(data, &packages) for _, gp := range packages { @@ -186,12 +184,10 @@ func Lookup(mp *metadata.Package, data []byte, targets map[metadata.PackagePath] if _, ok := objectSet[gobObj.Path]; ok { for _, ref := range gobObj.Refs { var uri protocol.DocumentURI - if ref.FileIndex < goFilesLen { + if asmIndex := ref.FileIndex - len(mp.CompiledGoFiles); asmIndex < 0 { uri = mp.CompiledGoFiles[ref.FileIndex] - } else if ref.FileIndex < goFilesLen+goAsmFilesLen { - uri = mp.AsmFiles[ref.FileIndex] } else { - continue // out of bounds + uri = mp.AsmFiles[asmIndex] } locs = append(locs, protocol.Location{ URI: uri, @@ -234,6 +230,6 @@ type gobObject struct { } type gobRef struct { - FileIndex int // index of enclosing file within P's CompiledGoFiles + FileIndex int // index of enclosing file within P's CompiledGoFiles + AsmFiles Range protocol.Range // source range of reference } diff --git a/gopls/internal/test/marker/testdata/references/asm.txt b/gopls/internal/test/marker/testdata/references/asm.txt index 07cc4a46034..ef5e0216135 100644 --- a/gopls/internal/test/marker/testdata/references/asm.txt +++ b/gopls/internal/test/marker/testdata/references/asm.txt @@ -34,3 +34,8 @@ TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "·sub"), refs("sub", defSub, use MOVQ a+0(FP), AX SUBQ b+8(FP), AX RET + +TEXT ·AddAndSub(SB), NOSPLIT, $0-24 + CALL ·Add(SB) + CALL ·sub(SB) + RET From 5d35c968122fb3764a9548dcd0a765b180d3be4e Mon Sep 17 00:00:00 2001 From: groot-guo Date: Thu, 3 Jul 2025 23:27:34 +0800 Subject: [PATCH 11/14] gopls: Update the condition for assembly reference checks and add test cases. Update the condition for identifying assembly files to allow only ref and data types. Remove Go definition references from the goasm reference process. Add test files for different types of reference relationships between asm and Go. Updates golang/go#71754 --- gopls/internal/cache/xrefs/xrefs.go | 2 +- gopls/internal/goasm/references.go | 21 +++++--- gopls/internal/golang/references.go | 2 +- .../test/marker/testdata/references/asm.txt | 44 ++++++---------- .../marker/testdata/references/asm_go.txt | 51 +++++++++++++++++++ 5 files changed, 81 insertions(+), 39 deletions(-) create mode 100644 gopls/internal/test/marker/testdata/references/asm_go.txt diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index 1215ac3fc85..ae5de00f7cc 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -117,7 +117,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles // For each asm file, record references to identifiers. for fileIndex, af := range asmFiles { for _, id := range af.Idents { - if id.Kind != asm.Data && id.Kind != asm.Text { + if id.Kind != asm.Data && id.Kind != asm.Ref { continue } _, name, ok := morestrings.CutLast(id.Name, ".") diff --git a/gopls/internal/goasm/references.go b/gopls/internal/goasm/references.go index 22bd718fd2c..9a4c92f36f0 100644 --- a/gopls/internal/goasm/references.go +++ b/gopls/internal/goasm/references.go @@ -80,6 +80,9 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p for _, pgf := range pkg.CompiledGoFiles() { for curId := range pgf.Cursor.Preorder((*ast.Ident)(nil)) { id := curId.Node().(*ast.Ident) + if !includeDeclaration && pkg.TypesInfo().Defs[id] != nil { + continue + } if !matches(pkg.TypesInfo().ObjectOf(id)) { continue } @@ -91,17 +94,19 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p } } - if includeDeclaration { - for _, asmFile := range pkg.AsmFiles() { - for _, id := range asmFile.Idents { - if id.Name == found.Name && - (id.Kind == asm.Text || id.Kind == asm.Global || id.Kind == asm.Label) { - if loc, err := asmFile.NodeLocation(id); err == nil { - locations = append(locations, loc) - } + for _, asmFile := range pkg.AsmFiles() { + for _, id := range asmFile.Idents { + if id.Name == found.Name && + (id.Kind == asm.Data || id.Kind == asm.Ref) { + if id.Kind == asm.Data && !includeDeclaration { + continue + } + if loc, err := asmFile.NodeLocation(id); err == nil { + locations = append(locations, loc) } } } } + return locations, nil } diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index d977b691360..a1b117d66d9 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -614,7 +614,7 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo // Iterate over all assembly files and find all references to the target object. for _, pgf := range pkg.AsmFiles() { for _, id := range pgf.Idents { - if id.Kind != asm.Data && id.Kind != asm.Text { + if id.Kind != asm.Data && id.Kind != asm.Ref { continue } _, name, ok := morestrings.CutLast(id.Name, ".") diff --git a/gopls/internal/test/marker/testdata/references/asm.txt b/gopls/internal/test/marker/testdata/references/asm.txt index ef5e0216135..fef63ca3850 100644 --- a/gopls/internal/test/marker/testdata/references/asm.txt +++ b/gopls/internal/test/marker/testdata/references/asm.txt @@ -1,41 +1,27 @@ -This test validates the References request functionality in Go assembly files. -It ensures that references to both exported (`Add`) and unexported (`sub`) functions are correctly identified across Go and assembly files. The test covers: -- Locating the definition of functions in both Go and assembly files. -- Identifying all references to the functions (`Add` and `sub`) within the Go and assembly files. - -The test includes: -- `Add`: An exported function with references in both Go and assembly files. -- `sub`: An unexported function with references in both Go and assembly files, including a usage in Go code (`var _ = sub`). - -The assembly file demonstrates portable assembly syntax and verifies cross-file reference handling. -- go.mod -- module example.com go 1.24 --- foo/foo.go -- -package foo - -func Add(a, b int) int //@ loc(use, "Add"), refs("Add", use, def) -func sub(a, b int) int //@ loc(useSub, "sub"), refs("sub", useSub, defSub, refSub) -var _ = sub //@loc(refSub, "sub"), refs("sub", useSub, defSub, refSub) +-- example/label_example.s -- +TEXT ·JumpDemo(SB), $0-0 +label1: //@loc(defLabel, "label1") + JMP label1 //@loc(refLabel, "label1") --- foo/foo.s -- -// portable assembly -#include "textflag.h" - -TEXT ·Add(SB), NOSPLIT, $0-24 //@ loc(def, "·Add"), refs("Add", def, use) - MOVQ a+0(FP), AX - ADDQ b+8(FP), AX +-- example/in_package_ref.s -- +TEXT ·Foo(SB), $0-8 //@loc(defFooAsm, "·Foo") RET -TEXT ·sub(SB), NOSPLIT, $0-24 //@ loc(defSub, "·sub"), refs("sub", defSub, useSub, refSub) - MOVQ a+0(FP), AX - SUBQ b+8(FP), AX +TEXT ·Bar(SB), $0-8 + CALL ·Foo(SB) //@loc(callFooAsm, "·Foo") RET -TEXT ·AddAndSub(SB), NOSPLIT, $0-24 - CALL ·Add(SB) - CALL ·sub(SB) +-- example/example_data.s -- +DATA ·myGlobal+0(SB)/8, $42 +GLOBL ·myGlobal(SB), NOPTR, 8 + +TEXT ·UseGlobal(SB), $0-0 + MOVQ ·myGlobal(SB), AX //@loc(refMyGlobal, "·myGlobal") RET + diff --git a/gopls/internal/test/marker/testdata/references/asm_go.txt b/gopls/internal/test/marker/testdata/references/asm_go.txt new file mode 100644 index 00000000000..c8c4b38dd27 --- /dev/null +++ b/gopls/internal/test/marker/testdata/references/asm_go.txt @@ -0,0 +1,51 @@ +This test validates the References request functionality in Go assembly files. + +It ensures that references to both exported (`Add`) and unexported (`sub`) functions are correctly identified across Go and assembly files. The test covers: +- Locating the definition of functions in both Go and assembly files. +- Identifying all references to the functions (`Add` and `sub`) within the Go and assembly files. + +The test includes: +- `Add`: An exported function with references in both Go and assembly files. +- `sub`: An unexported function with references in both Go and assembly files, including a usage in Go code (`var _ = sub`). + +The assembly file demonstrates portable assembly syntax and verifies cross-file reference handling. + +-- go.mod -- +module example.com +go 1.24 + +-- go_call_asm/example_func.go -- +package go_call_asm + +func Add(a, b int) int //@loc(defAddGo, "Add"), refs("Add", defAddGo, callAddGo) +func UseAdd() int { + return Add(1, 2) //@loc(callAddGo, "Add") +} +var myGlobal int64 //@loc(defMyGlobalGo, "myGlobal"), refs("myGlobal", defMyGlobalGo, refMyGlobal, refMyGlobalGo) +var _ = myGlobal //@loc(refMyGlobalGo, "myGlobal") + +-- go_call_asm/example_asm.s -- +TEXT ·Add(SB), $0-24 //@loc(defAddAsm, "·Add"), refs("Add", defAddGo, callAddGo) + MOVQ a+0(FP), AX + ADDQ b+8(FP), AX + RET + +DATA ·myGlobal+0(SB)/8, $42 +GLOBL ·myGlobal(SB), NOPTR, 8 +TEXT ·UseGlobal(SB), $0-0 + MOVQ ·myGlobal(SB), AX //@loc(refMyGlobal, "·myGlobal") + RET + +-- asm_call_go/example_sub.go -- +package asm_call_go + +func Sub(a, b int) int { return a - b } //@loc(defSubGo, "Sub") + +-- asm_call_go/call_sub.s -- +TEXT ·CallSub(SB), $0-16 + MOVQ $10, AX + MOVQ $3, BX + MOVQ AX, a+0(FP) + MOVQ BX, b+8(FP) + CALL ·Sub(SB) //@loc(callSubAsm, "·Sub") + RET From bac05c3d62befa39379aa5c0402f30d204b9d827 Mon Sep 17 00:00:00 2001 From: groot-guo Date: Tue, 5 Aug 2025 23:08:42 +0800 Subject: [PATCH 12/14] gopls: Update the condition for assembly reference checks and add test cases. Update the condition for identifying assembly files to allow only ref and data types. Remove Go definition references from the goasm reference process. Add test files for different types of reference relationships between asm and Go. Updates golang/go#71754 --- gopls/internal/cache/xrefs/xrefs.go | 18 +++++++++++------- gopls/internal/goasm/references.go | 10 +++++----- gopls/internal/golang/references.go | 6 +++++- .../test/marker/testdata/references/asm.txt | 3 --- .../test/marker/testdata/references/asm_go.txt | 18 +++++++++++++++--- gopls/internal/util/asm/parse.go | 7 ++++--- 6 files changed, 40 insertions(+), 22 deletions(-) diff --git a/gopls/internal/cache/xrefs/xrefs.go b/gopls/internal/cache/xrefs/xrefs.go index ae5de00f7cc..94677ab40be 100644 --- a/gopls/internal/cache/xrefs/xrefs.go +++ b/gopls/internal/cache/xrefs/xrefs.go @@ -120,14 +120,20 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles if id.Kind != asm.Data && id.Kind != asm.Ref { continue } - _, name, ok := morestrings.CutLast(id.Name, ".") + pkgpath, name, ok := morestrings.CutLast(id.Name, ".") if !ok { continue } + if pkgpath != pkg.Path() { + // Reference to symbol in another package. + // TODO(grootguo): implement this case. See similar logic in + // golang.lookupDocLinkSymbol, which also does not yet handle this case. + // See also goasm.Definitions, which loads a package by path from the + // forward transitive closure. + continue + } obj := pkg.Scope().Lookup(name) if obj == nil { - // TODO(grootguo): If the object is not found in the current package, - // consider handling cross-package references. continue } objects := getObjects(pkg) @@ -137,7 +143,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles gobObj = &gobObject{Path: objectpath.Path(obj.Name())} objects[obj] = gobObj } - if rng, err := af.NodeRange(id); err == nil { + if rng, err := af.IdentRange(id); err == nil { gobObj.Refs = append(gobObj.Refs, gobRef{ FileIndex: fileIndex, Range: rng, @@ -174,9 +180,7 @@ func Index(files []*parsego.File, pkg *types.Package, info *types.Info, asmFiles // to any object in the target set. Each object is denoted by a pair // of (package path, object path). func Lookup(mp *metadata.Package, data []byte, targets map[metadata.PackagePath]map[objectpath.Path]struct{}) (locs []protocol.Location) { - var ( - packages []*gobPackage - ) + var packages []*gobPackage packageCodec.Decode(data, &packages) for _, gp := range packages { if objectSet, ok := targets[gp.PkgPath]; ok { diff --git a/gopls/internal/goasm/references.go b/gopls/internal/goasm/references.go index 9a4c92f36f0..b860cf77c18 100644 --- a/gopls/internal/goasm/references.go +++ b/gopls/internal/goasm/references.go @@ -27,7 +27,7 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p ctx, done := event.Start(ctx, "goasm.References") defer done() - mps, err := snapshot.MetadataForFile(ctx, fh.URI()) + mps, err := snapshot.MetadataForFile(ctx, fh.URI(), false) if err != nil { return nil, err } @@ -94,14 +94,14 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p } } + if !includeDeclaration { + return locations, nil + } for _, asmFile := range pkg.AsmFiles() { for _, id := range asmFile.Idents { if id.Name == found.Name && (id.Kind == asm.Data || id.Kind == asm.Ref) { - if id.Kind == asm.Data && !includeDeclaration { - continue - } - if loc, err := asmFile.NodeLocation(id); err == nil { + if loc, err := asmFile.IdentLocation(id); err == nil { locations = append(locations, loc) } } diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index a1b117d66d9..053f9e9027a 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -39,6 +39,7 @@ import ( "golang.org/x/tools/gopls/internal/util/safetoken" "golang.org/x/tools/internal/astutil" "golang.org/x/tools/internal/event" + "golang.org/x/tools/internal/typesinternal" ) // References returns a list of all references (sorted with @@ -625,10 +626,13 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo if obj == nil { continue } + if !typesinternal.IsPackageLevel(obj) { + continue + } if !matches(obj) { continue } - if rng, err := pgf.NodeRange(id); err == nil { + if rng, err := pgf.IdentRange(id); err == nil { asmLocation := protocol.Location{ URI: pgf.URI, Range: rng, diff --git a/gopls/internal/test/marker/testdata/references/asm.txt b/gopls/internal/test/marker/testdata/references/asm.txt index fef63ca3850..6e8cab38304 100644 --- a/gopls/internal/test/marker/testdata/references/asm.txt +++ b/gopls/internal/test/marker/testdata/references/asm.txt @@ -1,8 +1,5 @@ --- go.mod -- -module example.com -go 1.24 -- example/label_example.s -- TEXT ·JumpDemo(SB), $0-0 diff --git a/gopls/internal/test/marker/testdata/references/asm_go.txt b/gopls/internal/test/marker/testdata/references/asm_go.txt index c8c4b38dd27..426f290d193 100644 --- a/gopls/internal/test/marker/testdata/references/asm_go.txt +++ b/gopls/internal/test/marker/testdata/references/asm_go.txt @@ -25,13 +25,13 @@ var myGlobal int64 //@loc(defMyGlobalGo, "myGlobal"), refs("myGlobal", defMyGlob var _ = myGlobal //@loc(refMyGlobalGo, "myGlobal") -- go_call_asm/example_asm.s -- -TEXT ·Add(SB), $0-24 //@loc(defAddAsm, "·Add"), refs("Add", defAddGo, callAddGo) +TEXT ·Add(SB), $0-24 //@refs("Add", defAddGo, callAddGo) MOVQ a+0(FP), AX ADDQ b+8(FP), AX RET -DATA ·myGlobal+0(SB)/8, $42 -GLOBL ·myGlobal(SB), NOPTR, 8 +DATA ·myGlobal+0(SB)/8, $42 +GLOBL ·myGlobal(SB), NOPTR, 8 TEXT ·UseGlobal(SB), $0-0 MOVQ ·myGlobal(SB), AX //@loc(refMyGlobal, "·myGlobal") RET @@ -49,3 +49,15 @@ TEXT ·CallSub(SB), $0-16 MOVQ BX, b+8(FP) CALL ·Sub(SB) //@loc(callSubAsm, "·Sub") RET + +// When defined in another package, it will not be processed. +-- example/func.go -- +package example + +func Add(a, b int) int +func UseAdd() int { + return Add(1, 2) +} +var myGlobal int64 +var _ = myGlobal + diff --git a/gopls/internal/util/asm/parse.go b/gopls/internal/util/asm/parse.go index 5437c95ff0f..fa555e75a54 100644 --- a/gopls/internal/util/asm/parse.go +++ b/gopls/internal/util/asm/parse.go @@ -54,12 +54,13 @@ type File struct { // future in which analyzers can report diagnostics in .s files. } -func (f *File) NodeRange(ident Ident) (protocol.Range, error) { +// IdentRange returns the protocol.Range for the identifier in this file. +func (f *File) IdentRange(ident Ident) (protocol.Range, error) { return f.Mapper.OffsetRange(ident.Offset, ident.Offset+ident.OrigLen) } -// NodeLocation returns a protocol Location for the ast.Node interval in this file. -func (f *File) NodeLocation(ident Ident) (protocol.Location, error) { +// IdentLocation returns a protocol Location for the identifier in this file. +func (f *File) IdentLocation(ident Ident) (protocol.Location, error) { return f.Mapper.OffsetLocation(ident.Offset, ident.Offset+ident.OrigLen) } From 80dfc0b2fed8c2494bb249a6ebcd8ad575dbe76a Mon Sep 17 00:00:00 2001 From: groot-guo Date: Wed, 6 Aug 2025 20:34:07 +0800 Subject: [PATCH 13/14] gopls: Update test cases. Update test files for different types of reference relationships between asm and Go. Updates golang/go#71754 --- gopls/internal/test/marker/testdata/references/asm.txt | 4 +--- .../test/marker/testdata/references/asm_go.txt | 10 ---------- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/gopls/internal/test/marker/testdata/references/asm.txt b/gopls/internal/test/marker/testdata/references/asm.txt index 6e8cab38304..962257d315b 100644 --- a/gopls/internal/test/marker/testdata/references/asm.txt +++ b/gopls/internal/test/marker/testdata/references/asm.txt @@ -1,5 +1,4 @@ - - +Test of asm references. -- example/label_example.s -- TEXT ·JumpDemo(SB), $0-0 @@ -21,4 +20,3 @@ GLOBL ·myGlobal(SB), NOPTR, 8 TEXT ·UseGlobal(SB), $0-0 MOVQ ·myGlobal(SB), AX //@loc(refMyGlobal, "·myGlobal") RET - diff --git a/gopls/internal/test/marker/testdata/references/asm_go.txt b/gopls/internal/test/marker/testdata/references/asm_go.txt index 426f290d193..08914ab36fc 100644 --- a/gopls/internal/test/marker/testdata/references/asm_go.txt +++ b/gopls/internal/test/marker/testdata/references/asm_go.txt @@ -1,15 +1,5 @@ This test validates the References request functionality in Go assembly files. -It ensures that references to both exported (`Add`) and unexported (`sub`) functions are correctly identified across Go and assembly files. The test covers: -- Locating the definition of functions in both Go and assembly files. -- Identifying all references to the functions (`Add` and `sub`) within the Go and assembly files. - -The test includes: -- `Add`: An exported function with references in both Go and assembly files. -- `sub`: An unexported function with references in both Go and assembly files, including a usage in Go code (`var _ = sub`). - -The assembly file demonstrates portable assembly syntax and verifies cross-file reference handling. - -- go.mod -- module example.com go 1.24 From f2720708a0e073a7c8cf081f0cbb9fe88a69ea7a Mon Sep 17 00:00:00 2001 From: groot-guo Date: Tue, 12 Aug 2025 21:27:04 +0800 Subject: [PATCH 14/14] gopls: Update test cases. Update test files for different types of reference relationships between asm and Go. Updates golang/go#71754 --- gopls/internal/goasm/references.go | 12 +++++++----- gopls/internal/golang/references.go | 12 +++++++----- .../test/marker/testdata/references/asm_go.txt | 12 ------------ 3 files changed, 14 insertions(+), 22 deletions(-) diff --git a/gopls/internal/goasm/references.go b/gopls/internal/goasm/references.go index b860cf77c18..0207e6bc0a9 100644 --- a/gopls/internal/goasm/references.go +++ b/gopls/internal/goasm/references.go @@ -94,13 +94,15 @@ func References(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, p } } - if !includeDeclaration { - return locations, nil - } for _, asmFile := range pkg.AsmFiles() { for _, id := range asmFile.Idents { - if id.Name == found.Name && - (id.Kind == asm.Data || id.Kind == asm.Ref) { + if id.Name == found.Name { + if !includeDeclaration && id.Kind == asm.Ref { + continue + } + if id.Kind != asm.Data { + continue + } if loc, err := asmFile.IdentLocation(id); err == nil { locations = append(locations, loc) } diff --git a/gopls/internal/golang/references.go b/gopls/internal/golang/references.go index 053f9e9027a..f03bb04d38f 100644 --- a/gopls/internal/golang/references.go +++ b/gopls/internal/golang/references.go @@ -18,6 +18,7 @@ import ( "fmt" "go/ast" "go/types" + "log" "sort" "strings" "sync" @@ -39,7 +40,6 @@ import ( "golang.org/x/tools/gopls/internal/util/safetoken" "golang.org/x/tools/internal/astutil" "golang.org/x/tools/internal/event" - "golang.org/x/tools/internal/typesinternal" ) // References returns a list of all references (sorted with @@ -618,15 +618,17 @@ func localReferences(pkg *cache.Package, targets map[types.Object]bool, correspo if id.Kind != asm.Data && id.Kind != asm.Ref { continue } - _, name, ok := morestrings.CutLast(id.Name, ".") + pkgpath, name, ok := morestrings.CutLast(id.Name, ".") if !ok { continue } - obj := pkg.Types().Scope().Lookup(name) - if obj == nil { + log.Printf("ignoring asm identifier %q in package %q: not in package %q", id.Name, pkgpath, pkg.Types().Path()) + if pkgpath != pkg.Types().Path() { + log.Printf("ignoring asm identifier %q in package %q: not in package %q", id.Name, pkgpath, pkg.Types().Path()) continue } - if !typesinternal.IsPackageLevel(obj) { + obj := pkg.Types().Scope().Lookup(name) + if obj == nil { continue } if !matches(obj) { diff --git a/gopls/internal/test/marker/testdata/references/asm_go.txt b/gopls/internal/test/marker/testdata/references/asm_go.txt index 08914ab36fc..62ab1f27541 100644 --- a/gopls/internal/test/marker/testdata/references/asm_go.txt +++ b/gopls/internal/test/marker/testdata/references/asm_go.txt @@ -39,15 +39,3 @@ TEXT ·CallSub(SB), $0-16 MOVQ BX, b+8(FP) CALL ·Sub(SB) //@loc(callSubAsm, "·Sub") RET - -// When defined in another package, it will not be processed. --- example/func.go -- -package example - -func Add(a, b int) int -func UseAdd() int { - return Add(1, 2) -} -var myGlobal int64 -var _ = myGlobal -