diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go index 9329d4bd0d1..0e3b03233f8 100644 --- a/gopls/internal/lsp/cache/check.go +++ b/gopls/internal/lsp/cache/check.go @@ -884,15 +884,15 @@ func (b *packageHandleBuilder) getTransitiveRefsLocked(id PackageID) map[string] for name := range ph.refs { if token.IsExported(name) { pkgs := b.s.pkgIndex.NewSet() - for _, node := range ph.refs[name] { + for _, sym := range ph.refs[name] { // TODO: opt: avoid int -> PackageID -> int conversions here. - id := node.PackageID(b.s.pkgIndex) + id := b.s.pkgIndex.DeclaringPackage(sym) pkgs.Add(id) otherRefs := b.getTransitiveRefsLocked(id) if otherRefs == nil { return nil // a predecessor failed: exit early } - if otherSet, ok := otherRefs[node.Name]; ok { + if otherSet, ok := otherRefs[sym.Name]; ok { pkgs.Union(otherSet) } } diff --git a/gopls/internal/lsp/source/typerefs/packageset.go b/gopls/internal/lsp/source/typerefs/packageset.go index 2f1937dbcb9..afa31244b37 100644 --- a/gopls/internal/lsp/source/typerefs/packageset.go +++ b/gopls/internal/lsp/source/typerefs/packageset.go @@ -34,25 +34,25 @@ func NewPackageIndex() *PackageIndex { // idx returns the packageIdx referencing id, creating one if id is not yet // tracked by the receiver. -func (r *PackageIndex) idx(id source.PackageID) int { - r.mu.Lock() - defer r.mu.Unlock() - if i, ok := r.m[id]; ok { +func (index *PackageIndex) idx(id source.PackageID) int { + index.mu.Lock() + defer index.mu.Unlock() + if i, ok := index.m[id]; ok { return i } - i := len(r.ids) - r.m[id] = i - r.ids = append(r.ids, id) + i := len(index.ids) + index.m[id] = i + index.ids = append(index.ids, id) return i } // id returns the PackageID for idx. // // idx must have been created by this PackageIndex instance. -func (r *PackageIndex) id(idx int) source.PackageID { - r.mu.Lock() - defer r.mu.Unlock() - return r.ids[idx] +func (index *PackageIndex) id(idx int) source.PackageID { + index.mu.Lock() + defer index.mu.Unlock() + return index.ids[idx] } // A PackageSet is a set of source.PackageIDs, optimized for inuse memory @@ -70,18 +70,27 @@ const blockSize = bits.UintSize // // PackageSets may only be combined with other PackageSets from the same // instance. -func (s *PackageIndex) NewSet() *PackageSet { +func (index *PackageIndex) NewSet() *PackageSet { return &PackageSet{ - parent: s, + parent: index, sparse: make(map[int]blockType), } } +// DeclaringPackage returns the ID of the symbol's declaring package. +// The package index must be the one used during decoding. +func (index *PackageIndex) DeclaringPackage(sym Symbol) source.PackageID { + return index.id(sym.pkgIdx) +} + // Add records a new element in the package set. func (s *PackageSet) Add(id source.PackageID) { s.add(s.parent.idx(id)) } +// AddDeclaringPackage adds sym's declaring package to the set. +func (s *PackageSet) AddDeclaringPackage(sym Symbol) { s.add(sym.pkgIdx) } + func (s *PackageSet) add(idx int) { i := int(idx) s.sparse[i/blockSize] |= 1 << (i % blockSize) diff --git a/gopls/internal/lsp/source/typerefs/pkgrefs.go b/gopls/internal/lsp/source/typerefs/pkggraph_test.go similarity index 79% rename from gopls/internal/lsp/source/typerefs/pkgrefs.go rename to gopls/internal/lsp/source/typerefs/pkggraph_test.go index b2c94de2015..514bf02ecad 100644 --- a/gopls/internal/lsp/source/typerefs/pkgrefs.go +++ b/gopls/internal/lsp/source/typerefs/pkggraph_test.go @@ -2,15 +2,23 @@ // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. -package typerefs +package typerefs_test + +// This file is logically part of the test in pkgrefs_test.go: that +// file defines the test assertion logic; this file provides a +// reference implementation of a client of the typerefs package. import ( + "bytes" "context" + "fmt" + "os" "runtime" "sync" "golang.org/x/sync/errgroup" "golang.org/x/tools/gopls/internal/lsp/source" + "golang.org/x/tools/gopls/internal/lsp/source/typerefs" "golang.org/x/tools/gopls/internal/span" ) @@ -19,9 +27,6 @@ const ( // // Warning: produces a lot of output! Best to run with small package queries. trace = false - - // debug enables additional assertions. - debug = false ) // A Package holds reference information for a single package. @@ -32,18 +37,18 @@ type Package struct { // transitiveRefs records, for each exported declaration in the package, the // transitive set of packages within the containing graph that are // transitively reachable through references, starting with the given decl. - transitiveRefs map[string]*PackageSet + transitiveRefs map[string]*typerefs.PackageSet // ReachesViaDeps records the set of packages in the containing graph whose // syntax may affect the current package's types. See the package // documentation for more details of what this means. - ReachesByDeps *PackageSet + ReachesByDeps *typerefs.PackageSet } // A PackageGraph represents a fully analyzed graph of packages and their // dependencies. type PackageGraph struct { - pkgIndex *PackageIndex + pkgIndex *typerefs.PackageIndex meta source.MetadataSource parse func(context.Context, span.URI) (*source.ParsedGoFile, error) @@ -63,7 +68,7 @@ type PackageGraph struct { // algorithm. func BuildPackageGraph(ctx context.Context, meta source.MetadataSource, ids []source.PackageID, parse func(context.Context, span.URI) (*source.ParsedGoFile, error)) (*PackageGraph, error) { g := &PackageGraph{ - pkgIndex: NewPackageIndex(), + pkgIndex: typerefs.NewPackageIndex(), meta: meta, parse: parse, packages: make(map[source.PackageID]*futurePackage), @@ -120,7 +125,7 @@ func (g *PackageGraph) Package(ctx context.Context, id source.PackageID) (*Packa func (g *PackageGraph) buildPackage(ctx context.Context, id source.PackageID) (*Package, error) { p := &Package{ metadata: g.meta.Metadata(id), - transitiveRefs: make(map[string]*PackageSet), + transitiveRefs: make(map[string]*typerefs.PackageSet), } var files []*source.ParsedGoFile for _, filename := range p.metadata.CompiledGoFiles { @@ -138,9 +143,10 @@ func (g *PackageGraph) buildPackage(ctx context.Context, id source.PackageID) (* } // Compute the symbol-level dependencies through this package. - // TODO(adonovan): opt: persist this in the filecache, keyed + data := typerefs.Encode(files, id, imports) + + // data can be persisted in a filecache, keyed // by hash(id, CompiledGoFiles, imports). - data := Encode(files, id, imports) // This point separates the local preprocessing // -- of a single package (above) from the global -- @@ -150,9 +156,33 @@ func (g *PackageGraph) buildPackage(ctx context.Context, id source.PackageID) (* // package and declarations in this package or another // package. See the package documentation for a detailed // description of what these edges do (and do not) represent. - classes := Decode(g.pkgIndex, id, data) - - idx := g.pkgIndex.idx(id) + classes := typerefs.Decode(g.pkgIndex, id, data) + + // Debug + if trace && len(classes) > 0 { + var buf bytes.Buffer + fmt.Fprintf(&buf, "%s\n", id) + for _, class := range classes { + for i, name := range class.Decls { + if i == 0 { + fmt.Fprintf(&buf, "\t") + } + fmt.Fprintf(&buf, " .%s", name) + } + // Group symbols by package. + var prevID PackageID + for _, sym := range class.Refs { + id := g.pkgIndex.DeclaringPackage(sym) + if id != prevID { + prevID = id + fmt.Fprintf(&buf, "\n\t\t-> %s:", id) + } + fmt.Fprintf(&buf, " .%s", sym.Name) + } + fmt.Fprintln(&buf) + } + os.Stderr.Write(buf.Bytes()) + } // Now compute the transitive closure of packages reachable // from any exported symbol of this package. @@ -164,8 +194,10 @@ func (g *PackageGraph) buildPackage(ctx context.Context, id source.PackageID) (* // when the package id changes. depP := p for _, sym := range class.Refs { - assert(sym.pkgIdx != idx, "intra-package edge") - symPkgID := g.pkgIndex.id(sym.pkgIdx) + symPkgID := g.pkgIndex.DeclaringPackage(sym) + if symPkgID == id { + panic("intra-package edge") + } if depP.metadata.ID != symPkgID { // package changed var err error @@ -174,7 +206,7 @@ func (g *PackageGraph) buildPackage(ctx context.Context, id source.PackageID) (* return nil, err } } - set.add(sym.pkgIdx) + set.AddDeclaringPackage(sym) set.Union(depP.transitiveRefs[sym.Name]) } for _, name := range class.Decls { @@ -195,7 +227,7 @@ func (g *PackageGraph) buildPackage(ctx context.Context, id source.PackageID) (* // reachesByDeps computes the set of packages that are reachable through // dependencies of the package m. -func (g *PackageGraph) reachesByDeps(ctx context.Context, m *source.Metadata) (*PackageSet, error) { +func (g *PackageGraph) reachesByDeps(ctx context.Context, m *source.Metadata) (*typerefs.PackageSet, error) { transitive := g.pkgIndex.NewSet() for _, depID := range m.DepsByPkgPath { dep, err := g.Package(ctx, depID) diff --git a/gopls/internal/lsp/source/typerefs/pkgrefs_test.go b/gopls/internal/lsp/source/typerefs/pkgrefs_test.go index b6e1ac242a4..39200a515dc 100644 --- a/gopls/internal/lsp/source/typerefs/pkgrefs_test.go +++ b/gopls/internal/lsp/source/typerefs/pkgrefs_test.go @@ -23,7 +23,6 @@ import ( "golang.org/x/tools/gopls/internal/astutil" "golang.org/x/tools/gopls/internal/lsp/cache" "golang.org/x/tools/gopls/internal/lsp/source" - "golang.org/x/tools/gopls/internal/lsp/source/typerefs" "golang.org/x/tools/gopls/internal/span" "golang.org/x/tools/internal/packagesinternal" "golang.org/x/tools/internal/testenv" @@ -87,7 +86,7 @@ func TestBuildPackageGraph(t *testing.T) { }) t0 = time.Now() - g, err := typerefs.BuildPackageGraph(ctx, meta, ids, newParser().parse) + g, err := BuildPackageGraph(ctx, meta, ids, newParser().parse) if err != nil { t.Fatal(err) } @@ -263,7 +262,7 @@ func BenchmarkBuildPackageGraph(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - _, err := typerefs.BuildPackageGraph(ctx, meta, ids, newParser().parse) + _, err := BuildPackageGraph(ctx, meta, ids, newParser().parse) if err != nil { b.Fatal(err) } diff --git a/gopls/internal/lsp/source/typerefs/refs.go b/gopls/internal/lsp/source/typerefs/refs.go index 27486c0111c..516afc6d185 100644 --- a/gopls/internal/lsp/source/typerefs/refs.go +++ b/gopls/internal/lsp/source/typerefs/refs.go @@ -11,7 +11,6 @@ import ( "go/ast" "go/token" "log" - "os" "sort" "strings" @@ -65,12 +64,6 @@ type Symbol struct { Name string } -// PackageID returns the symbol's package identifier. -// The package index must be the one used during decoding. -func (sym Symbol) PackageID(index *PackageIndex) source.PackageID { - return index.id(sym.pkgIdx) -} - // -- internals -- // A symbolSet is a set of symbols used internally during index construction. @@ -726,14 +719,16 @@ func (decl *declNode) find() *declNode { return rep } +const debugSCC = false // enable assertions in strong-component algorithm + func checkCanonical(x *declNode) { - if debug { + if debugSCC { assert(x == x.find(), "not canonical") } } func assert(cond bool, msg string) { - if debug && !cond { + if debugSCC && !cond { panic(msg) } } @@ -828,31 +823,6 @@ func decode(pkgIndex *PackageIndex, id source.PackageID, data []byte) []Class { return classes[i].Decls[0] < classes[j].Decls[0] }) - // Debug - if trace && len(classes) > 0 { - var buf bytes.Buffer - fmt.Fprintf(&buf, "%s\n", id) - for _, class := range classes { - for i, name := range class.Decls { - if i == 0 { - fmt.Fprintf(&buf, "\t") - } - fmt.Fprintf(&buf, " .%s", name) - } - // Group symbols by package. - prevID := -1 - for _, sym := range class.Refs { - if sym.pkgIdx != prevID { - prevID = sym.pkgIdx - fmt.Fprintf(&buf, "\n\t\t-> %s:", sym.PackageID(pkgIndex)) - } - fmt.Fprintf(&buf, " .%s", sym.Name) - } - fmt.Fprintln(&buf) - } - os.Stderr.Write(buf.Bytes()) - } - return classes } diff --git a/gopls/internal/lsp/source/typerefs/refs_test.go b/gopls/internal/lsp/source/typerefs/refs_test.go index adf79bd8f7e..b83c7812407 100644 --- a/gopls/internal/lsp/source/typerefs/refs_test.go +++ b/gopls/internal/lsp/source/typerefs/refs_test.go @@ -543,7 +543,7 @@ type Z map[ext.A]ext.B for _, name := range class.Decls { var syms []string for _, sym := range class.Refs { - syms = append(syms, fmt.Sprintf("%s.%s", sym.PackageID(index), sym.Name)) + syms = append(syms, fmt.Sprintf("%s.%s", index.DeclaringPackage(sym), sym.Name)) } sort.Strings(syms) got[name] = syms