Skip to content

gopls: add CompiledAsmFiles in cache.Package #572

New issue

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

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

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions gopls/internal/cache/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 != "" {
Expand All @@ -1503,6 +1507,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,
Expand Down Expand Up @@ -1555,6 +1560,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])
Expand Down Expand Up @@ -1611,6 +1620,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" {
Expand Down
7 changes: 7 additions & 0 deletions gopls/internal/cache/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,13 @@ func buildMetadata(updates map[PackageID]*metadata.Package, loadDir string, stan
*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))
}
copyURIs(&mp.CompiledGoFiles, pkg.CompiledGoFiles)
copyURIs(&mp.GoFiles, pkg.GoFiles)
copyURIs(&mp.IgnoredFiles, pkg.IgnoredFiles)
Expand Down
3 changes: 3 additions & 0 deletions gopls/internal/cache/metadata/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ type Package struct {
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
Expand Down
4 changes: 3 additions & 1 deletion gopls/internal/cache/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

// Convenient aliases for very heavily used types.
Expand Down Expand Up @@ -49,6 +50,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
Expand All @@ -69,7 +71,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
}
Expand Down
19 changes: 19 additions & 0 deletions gopls/internal/cache/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -43,3 +44,21 @@ 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 parses the assembly files whose contents are provided by fhs.
func parseAsmFiles(ctx context.Context, fhs ...file.Handle) ([]*asm.File, error) {
pafs := 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()
}
pafs[i] = asm.Parse(fh.URI(), content)
}
return pafs, nil
}
45 changes: 41 additions & 4 deletions gopls/internal/cache/xrefs/xrefs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/goasm/definition.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
13 changes: 11 additions & 2 deletions gopls/internal/util/asm/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion gopls/internal/util/asm/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down