gopls/internal/cache: use AddExistingFiles in the parse cache
Use the new tokeninternal.AddExistingFiles API to simplify the parse
cache and type-checking pass. Since we can now add more parsed files
even after starting importing, we no longer need the multi-phased
type-checking pass, and can evaluate promises in any order.
This broke things on 32-bit systems, so add a temporary fall-back on
those systems that doesn't cache parsed files.
For golang/go#57987
Change-Id: I465763acd8baf275675890de64975bdbc045f375
Reviewed-on: https://go-review.googlesource.com/c/tools/+/473165
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/gopls/internal/lsp/cache/check.go b/gopls/internal/lsp/cache/check.go
index be27518..529940a 100644
--- a/gopls/internal/lsp/cache/check.go
+++ b/gopls/internal/lsp/cache/check.go
@@ -45,10 +45,10 @@
type typeCheckBatch struct {
meta *metadataGraph
- cpulimit chan struct{} // concurrency limiter for CPU-bound operations
- needSyntax map[PackageID]bool // packages that need type-checked syntax
- parsedFiles map[span.URI]*source.ParsedGoFile // parsed files necessary for type-checking
- fset *token.FileSet // FileSet describing all parsed files
+ fset *token.FileSet // FileSet describing all parsed files
+ parseCache *parseCache // shared parsing cache
+ cpulimit chan struct{} // concurrency limiter for CPU-bound operations
+ needSyntax map[PackageID]bool // packages that need type-checked syntax
// Promises holds promises to either read export data for the package, or
// parse and type-check its syntax.
@@ -57,11 +57,15 @@
// awaited, they must write an entry into the imports map.
promises map[PackageID]*memoize.Promise
- mu sync.Mutex
- needFiles map[span.URI]source.FileHandle // de-duplicated file handles required for type-checking
- imports map[PackageID]pkgOrErr // types.Packages to use for importing
- exportData map[PackageID][]byte
- packages map[PackageID]*Package
+ mu sync.Mutex
+ // TODO(rfindley): parsedFiles, which holds every file needed for
+ // type-checking, may not be necessary given the parseCache.
+ //
+ // In fact, parsedFiles may be counter-productive due to pinning all files in
+ // memory during large operations.
+ parsedFiles map[span.URI]*source.ParsedGoFile // parsed files necessary for type-checking
+ imports map[PackageID]pkgOrErr // types.Packages to use for importing
+ packages map[PackageID]*Package
}
type pkgOrErr struct {
@@ -79,18 +83,17 @@
// indicates context cancellation or otherwise significant failure to perform
// the type-checking operation.
func (s *snapshot) TypeCheck(ctx context.Context, ids ...PackageID) ([]source.Package, error) {
- // Build up shared state for efficient type-checking.
+ // Shared state for efficient type-checking.
b := &typeCheckBatch{
- cpulimit: make(chan struct{}, runtime.GOMAXPROCS(0)),
- needSyntax: make(map[PackageID]bool),
- parsedFiles: make(map[span.URI]*source.ParsedGoFile),
- // fset is built during the parsing pass.
+ fset: fileSetWithBase(reservedForParsing),
+ parseCache: s.parseCache,
+ cpulimit: make(chan struct{}, runtime.GOMAXPROCS(0)),
+ needSyntax: make(map[PackageID]bool),
- needFiles: make(map[span.URI]source.FileHandle),
- promises: make(map[PackageID]*memoize.Promise),
- imports: make(map[PackageID]pkgOrErr),
- exportData: make(map[PackageID][]byte),
- packages: make(map[PackageID]*Package),
+ parsedFiles: make(map[span.URI]*source.ParsedGoFile),
+ promises: make(map[PackageID]*memoize.Promise),
+ imports: make(map[PackageID]pkgOrErr),
+ packages: make(map[PackageID]*Package),
}
// Check for existing active packages.
@@ -120,12 +123,8 @@
b.meta = s.meta
s.mu.Unlock()
- // -- Step 1: assemble the promises graph --
- var (
- needExportData = make(map[PackageID]packageHandleKey)
- packageHandles = make(map[PackageID]*packageHandle)
- )
-
+ // -- assemble the promises graph --
+ //
// collectPromises collects promises to load packages from export data or
// type-check.
var collectPromises func(PackageID) error
@@ -152,21 +151,11 @@
if err != nil {
return err
}
- packageHandles[id] = ph
-
- if b.needSyntax[id] {
- // We will need to parse and type-check this package.
- //
- // We may also need to parse and type-check if export data is missing,
- // but that is handled after fetching export data below.
- b.addNeededFiles(ph)
- } else if id != "unsafe" { // we can't load export data for unsafe
- needExportData[id] = ph.key
- }
debugName := fmt.Sprintf("check(%s)", id)
b.promises[id] = memoize.NewPromise(debugName, func(ctx context.Context, _ interface{}) interface{} {
pkg, err := b.processPackage(ctx, ph)
+
b.mu.Lock()
b.imports[m.ID] = pkgOrErr{pkg, err}
b.mu.Unlock()
@@ -178,57 +167,7 @@
collectPromises(id)
}
- // -- Step 2: collect export data --
- //
- // This must be done before parsing in order to determine which files must be
- // parsed.
- {
- var g errgroup.Group
- for id, key := range needExportData {
- id := id
- key := key
- g.Go(func() error {
- data, err := filecache.Get(exportDataKind, key)
- if err != nil {
- if err == filecache.ErrNotFound {
- ph := packageHandles[id]
- b.addNeededFiles(ph) // we will need to parse and type check
- return nil // ok: we will type check later
- }
- return err
- }
- b.mu.Lock()
- b.exportData[id] = data
- b.mu.Unlock()
- return nil
- })
- }
- if err := g.Wait(); err != nil {
- return pkgs, err
- }
- }
-
- // -- Step 3: parse files required for type checking. --
- //
- // Parse all necessary files in parallel. Unfortunately we can't start
- // parsing each package's file as soon as we discover that it is a syntax
- // package, because the parseCache cannot add files to an existing FileSet.
- {
- var fhs []source.FileHandle
- for _, fh := range b.needFiles {
- fhs = append(fhs, fh)
- }
- pgfs, fset, err := s.parseCache.parseFiles(ctx, source.ParseFull, fhs...)
- if err != nil {
- return pkgs, err
- }
- for _, pgf := range pgfs {
- b.parsedFiles[pgf.URI] = pgf
- }
- b.fset = fset
- }
-
- // -- Step 4: await results --
+ // -- await type-checking. --
//
// Start a single goroutine for each promise.
{
@@ -277,20 +216,41 @@
return pkgs, firstErr
}
-// addNeededFiles records the files necessary for type-checking ph, for later
-// parsing.
-func (b *typeCheckBatch) addNeededFiles(ph *packageHandle) {
+// parseFiles gets pre-existing parsed files for fhs from b.parsedFiles, or
+// parses as needed.
+func (b *typeCheckBatch) parseFiles(ctx context.Context, fhs []source.FileHandle) ([]*source.ParsedGoFile, error) {
+ var needFiles []source.FileHandle // files that need to be parsed
+ var needIndex []int // indexes in fhs of the entries in needFiles
+
+ pgfs := make([]*source.ParsedGoFile, len(fhs))
+ b.mu.Lock()
+ for i, fh := range fhs {
+ if pgf, ok := b.parsedFiles[fh.URI()]; ok {
+ pgfs[i] = pgf
+ } else {
+ needFiles = append(needFiles, fh)
+ needIndex = append(needIndex, i)
+ }
+ }
+ b.mu.Unlock()
+
+ parsed, err := b.parseCache.parseFiles(ctx, b.fset, source.ParseFull, needFiles...)
+ if err != nil {
+ return nil, err
+ }
+
b.mu.Lock()
defer b.mu.Unlock()
-
- // Technically for export-only packages we only need compiledGoFiles, but
- // these slices are usually redundant.
- for _, fh := range ph.inputs.goFiles {
- b.needFiles[fh.URI()] = fh
+ for i, pgf := range parsed {
+ idx := needIndex[i]
+ if existing, ok := b.parsedFiles[pgf.URI]; ok { // lost a race
+ pgfs[idx] = existing
+ } else {
+ b.parsedFiles[pgf.URI] = pgf
+ pgfs[idx] = pgf
+ }
}
- for _, fh := range ph.inputs.compiledGoFiles {
- b.needFiles[fh.URI()] = fh
- }
+ return pgfs, nil
}
// processPackage processes the package handle for the type checking batch,
@@ -315,31 +275,32 @@
}()
}
- b.mu.Lock()
- data, ok := b.exportData[ph.m.ID]
- b.mu.Unlock()
+ if b.needSyntax[ph.m.ID] {
+ // We need a syntax package.
+ syntaxPkg, err := b.checkPackage(ctx, ph)
+ if err != nil {
+ return nil, err
+ }
- if ok {
- // We need export data, and have it.
- return b.importPackage(ctx, ph.m, data)
+ b.mu.Lock()
+ b.packages[ph.m.ID] = syntaxPkg
+ b.mu.Unlock()
+ return syntaxPkg.pkg.types, nil
}
- if !b.needSyntax[ph.m.ID] {
- // We need only a types.Package, but don't have export data.
- // Type-check as fast as possible (skipping function bodies).
+ if ph.m.ID == "unsafe" {
+ return types.Unsafe, nil
+ }
+
+ data, err := filecache.Get(exportDataKind, ph.key)
+ if err == filecache.ErrNotFound {
+ // No cached export data: type-check as fast as possible.
return b.checkPackageForImport(ctx, ph)
}
-
- // We need a syntax package.
- syntaxPkg, err := b.checkPackage(ctx, ph)
if err != nil {
- return nil, err
+ return nil, fmt.Errorf("failed to read cache data for %s: %v", ph.m.ID, err)
}
-
- b.mu.Lock()
- b.packages[ph.m.ID] = syntaxPkg
- b.mu.Unlock()
- return syntaxPkg.pkg.types, nil
+ return b.importPackage(ctx, ph.m, data)
}
// importPackage loads the given package from its export data in p.exportData
@@ -364,26 +325,23 @@
// checkPackageForImport type checks, but skips function bodies and does not
// record syntax information.
func (b *typeCheckBatch) checkPackageForImport(ctx context.Context, ph *packageHandle) (*types.Package, error) {
- if ph.m.ID == "unsafe" {
- return types.Unsafe, nil
- }
impMap, errMap := b.importMap(ph.inputs.id)
onError := func(e error) {
// Ignore errors for exporting.
}
cfg := b.typesConfig(ph.inputs, onError, impMap, errMap)
- var files []*ast.File
- for _, fh := range ph.inputs.compiledGoFiles {
- pgf := b.parsedFiles[fh.URI()]
- if pgf == nil {
- return nil, fmt.Errorf("compiled go file %q failed to parse", fh.URI().Filename())
- }
- files = append(files, pgf.File)
+ pgfs, err := b.parseFiles(ctx, ph.inputs.compiledGoFiles)
+ if err != nil {
+ return nil, err
}
cfg.IgnoreFuncBodies = true
pkg := types.NewPackage(string(ph.inputs.pkgPath), string(ph.inputs.name))
check := types.NewChecker(cfg, b.fset, pkg, nil)
+ files := make([]*ast.File, len(pgfs))
+ for i, pgf := range pgfs {
+ files[i] = pgf.File
+ }
_ = check.Files(files) // ignore errors
// If the context was cancelled, we may have returned a ton of transient
@@ -923,24 +881,19 @@
// Collect parsed files from the type check pass, capturing parse errors from
// compiled files.
- for _, fh := range inputs.goFiles {
- pgf := b.parsedFiles[fh.URI()]
- if pgf == nil {
- // If go/packages told us that a file is in a package, it should be
- // parseable (after all, it was parsed by go list).
- return nil, bug.Errorf("go file %q failed to parse", fh.URI().Filename())
- }
- pkg.goFiles = append(pkg.goFiles, pgf)
+ var err error
+ pkg.goFiles, err = b.parseFiles(ctx, inputs.goFiles)
+ if err != nil {
+ return nil, err
}
- for _, fh := range inputs.compiledGoFiles {
- pgf := b.parsedFiles[fh.URI()]
- if pgf == nil {
- return nil, fmt.Errorf("compiled go file %q failed to parse", fh.URI().Filename())
- }
+ pkg.compiledGoFiles, err = b.parseFiles(ctx, inputs.compiledGoFiles)
+ if err != nil {
+ return nil, err
+ }
+ for _, pgf := range pkg.compiledGoFiles {
if pgf.ParseErr != nil {
pkg.parseErrors = append(pkg.parseErrors, pgf.ParseErr)
}
- pkg.compiledGoFiles = append(pkg.compiledGoFiles, pgf)
}
// Use the default type information for the unsafe package.
diff --git a/gopls/internal/lsp/cache/mod_tidy.go b/gopls/internal/lsp/cache/mod_tidy.go
index 8818a28..00e0e5c 100644
--- a/gopls/internal/lsp/cache/mod_tidy.go
+++ b/gopls/internal/lsp/cache/mod_tidy.go
@@ -8,6 +8,7 @@
"context"
"fmt"
"go/ast"
+ "go/token"
"io/ioutil"
"os"
"path/filepath"
@@ -460,7 +461,7 @@
//
// TODO(rfindley): this should key off source.ImportPath.
func parseImports(ctx context.Context, s *snapshot, files []source.FileHandle) (map[string]bool, error) {
- pgfs, _, err := s.parseCache.parseFiles(ctx, source.ParseHeader, files...)
+ pgfs, err := s.parseCache.parseFiles(ctx, token.NewFileSet(), source.ParseHeader, files...)
if err != nil { // e.g. context cancellation
return nil, err
}
diff --git a/gopls/internal/lsp/cache/parse.go b/gopls/internal/lsp/cache/parse.go
index 55b1510..59420be 100644
--- a/gopls/internal/lsp/cache/parse.go
+++ b/gopls/internal/lsp/cache/parse.go
@@ -27,7 +27,7 @@
// ParseGo parses the file whose contents are provided by fh, using a cache.
// The resulting tree may have beeen fixed up.
func (s *snapshot) ParseGo(ctx context.Context, fh source.FileHandle, mode parser.Mode) (*source.ParsedGoFile, error) {
- pgfs, _, err := s.parseCache.parseFiles(ctx, mode, fh)
+ pgfs, err := s.parseCache.parseFiles(ctx, token.NewFileSet(), mode, fh)
if err != nil {
return nil, err
}
diff --git a/gopls/internal/lsp/cache/parse_cache.go b/gopls/internal/lsp/cache/parse_cache.go
index 8bd64a5..3511b75 100644
--- a/gopls/internal/lsp/cache/parse_cache.go
+++ b/gopls/internal/lsp/cache/parse_cache.go
@@ -9,15 +9,50 @@
"context"
"go/parser"
"go/token"
+ "math/bits"
"runtime"
- "sort"
"sync"
"golang.org/x/sync/errgroup"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/internal/memoize"
+ "golang.org/x/tools/internal/tokeninternal"
)
+// reservedForParsing defines the room in the token.Pos space reserved for
+// cached parsed files.
+//
+// Files parsed through the parseCache are guaranteed not to have overlapping
+// spans: the parseCache tracks a monotonic base for newly parsed files.
+//
+// By offsetting the initial base of a FileSet, we can allow other operations
+// accepting the FileSet (such as the gcimporter) to add new files using the
+// normal FileSet APIs without overlapping with cached parsed files.
+//
+// Note that 1<<60 represents an exabyte of parsed data, more than any gopls
+// process can ever parse.
+//
+// On 32-bit systems we don't cache parse results (see parseFiles).
+const reservedForParsing = 1 << (bits.UintSize - 4)
+
+// fileSetWithBase returns a new token.FileSet with Base() equal to the
+// requested base, or 1 if base < 1.
+func fileSetWithBase(base int) *token.FileSet {
+ fset := token.NewFileSet()
+ if base > 1 {
+ // Add a dummy file to set the base of fset. We won't ever use the
+ // resulting FileSet, so it doesn't matter how we achieve this.
+ //
+ // FileSets leave a 1-byte padding between files, so we set the base by
+ // adding a zero-length file at base-1.
+ fset.AddFile("", base-1, 0)
+ }
+ if base >= 1 && fset.Base() != base {
+ panic("unexpected FileSet.Base")
+ }
+ return fset
+}
+
// This file contains an implementation of a bounded-size parse cache, that
// offsets the base token.Pos value of each cached file so that they may be
// later described by a single dedicated FileSet.
@@ -34,9 +69,8 @@
// cache hits for low-latency operations.
const parseCacheMaxFiles = 200
-// parsePadding is additional padding allocated between entries in the parse
-// cache to allow for increases in length (such as appending missing braces)
-// caused by fixAST.
+// parsePadding is additional padding allocated to allow for increases in
+// length (such as appending missing braces) caused by fixAST.
//
// This is used to mitigate a chicken and egg problem: we must know the base
// offset of the file we're about to parse, before we start parsing, and yet
@@ -57,11 +91,11 @@
// caching) multiple files. This is necessary for type-checking, where files
// must be parsed in a common fileset.
type parseCache struct {
- mu sync.Mutex
- m map[parseKey]*parseCacheEntry
- lru queue // min-atime priority queue of *parseCacheEntry
- clock uint64 // clock time, incremented when the cache is updated
- nextOffset token.Pos // token.Pos offset for the next parsed file
+ mu sync.Mutex
+ m map[parseKey]*parseCacheEntry
+ lru queue // min-atime priority queue of *parseCacheEntry
+ clock uint64 // clock time, incremented when the cache is updated
+ nextBase int // base offset for the next parsed file
}
// parseKey uniquely identifies a parsed Go file.
@@ -77,10 +111,8 @@
lruIndex int
}
-// startParse prepares a parsing pass, using the following steps:
-// - search for cache hits
-// - create new promises for cache misses
-// - store as many new promises in the cache as space will allow
+// startParse prepares a parsing pass, creating new promises in the cache for
+// any cache misses.
//
// The resulting slice has an entry for every given file handle, though some
// entries may be nil if there was an error reading the file (in which case the
@@ -124,16 +156,32 @@
continue
}
- // ...otherwise, create a new promise to parse with a non-overlapping offset
- fset := token.NewFileSet()
- if c.nextOffset > 0 {
- // Add a dummy file so that this parsed file does not overlap with others.
- fset.AddFile("", 1, int(c.nextOffset))
- }
- c.nextOffset += token.Pos(len(content) + parsePadding + 1) // leave room for src fixes
- fh := fh
+ // ...otherwise, create a new promise to parse with a non-overlapping
+ // offset. We allocate 2*len(content)+parsePadding to allow for re-parsing
+ // once inside of parseGoSrc without exceeding the allocated space.
+ base, nextBase, fset := c.allocateSpaceLocked(2*len(content) + parsePadding)
+ uri := fh.URI()
promise := memoize.NewPromise(string(fh.URI()), func(ctx context.Context, _ interface{}) interface{} {
- return parseGoSrc(ctx, fset, fh.URI(), content, mode)
+ pgf := parseGoSrc(ctx, fset, uri, content, mode)
+ file := pgf.Tok
+ if file.Base()+file.Size()+1 > nextBase {
+ // The parsed file exceeds its allocated space, likely due to multiple
+ // passes of src fixing. In this case, we have no choice but to re-do
+ // the operation with the correct size.
+ //
+ // Even though the final successful parse requires only file.Size()
+ // bytes of Pos space, we need to accommodate all the missteps to get
+ // there, as parseGoSrc will repeat them.
+ actual := file.Base() + file.Size() - base // actual size consumed, after re-parsing
+ c.mu.Lock()
+ _, next, fset := c.allocateSpaceLocked(actual)
+ c.mu.Unlock()
+ pgf = parseGoSrc(ctx, fset, uri, content, mode)
+ if pgf.Tok.Base()+pgf.Tok.Size() != next-1 {
+ panic("internal error: non-deterministic parsing result")
+ }
+ }
+ return pgf
})
promises[i] = promise
@@ -163,24 +211,60 @@
return promises, firstReadError
}
-// parseFiles returns a ParsedGoFile for the given file handles in the
+// allocateSpaceLocked reserves the next n bytes of token.Pos space in the
+// cache.
+//
+// It returns the resulting file base, next base, and an offset FileSet to use
+// for parsing.
+func (c *parseCache) allocateSpaceLocked(size int) (int, int, *token.FileSet) {
+ base := c.nextBase
+ c.nextBase += size + 1
+ return base, c.nextBase, fileSetWithBase(base)
+}
+
+// The parse cache is not supported on 32-bit systems, where reservedForParsing
+// is too small to be viable.
+func parseCacheSupported() bool {
+ return bits.UintSize != 32
+}
+
+// parseFiles returns a ParsedGoFile for each file handle in fhs, in the
// requested parse mode.
//
-// If parseFiles returns an error, it still returns a slice,
-// but with a nil entry for each file that could not be parsed.
-//
-// The second result is a FileSet describing all resulting parsed files.
-//
// For parsed files that already exists in the cache, access time will be
// updated. For others, parseFiles will parse and store as many results in the
// cache as space allows.
-func (c *parseCache) parseFiles(ctx context.Context, mode parser.Mode, fhs ...source.FileHandle) ([]*source.ParsedGoFile, *token.FileSet, error) {
- promises, firstReadError := c.startParse(mode, fhs...)
+//
+// The token.File for each resulting parsed file will be added to the provided
+// FileSet, using the tokeninternal.AddExistingFiles API. Consequently, the
+// given fset should only be used in other APIs if its base is >=
+// reservedForParsing.
+//
+// If parseFiles returns an error, it still returns a slice,
+// but with a nil entry for each file that could not be parsed.
+func (c *parseCache) parseFiles(ctx context.Context, fset *token.FileSet, mode parser.Mode, fhs ...source.FileHandle) ([]*source.ParsedGoFile, error) {
+ pgfs := make([]*source.ParsedGoFile, len(fhs))
+
+ // Temporary fall-back for 32-bit systems, where reservedForParsing is too
+ // small to be viable. We don't actually support 32-bit systems, so this
+ // workaround is only for tests and can be removed when we stop running
+ // 32-bit TryBots for gopls.
+ if bits.UintSize == 32 {
+ for i, fh := range fhs {
+ var err error
+ pgfs[i], err = parseGoImpl(ctx, fset, fh, mode)
+ if err != nil {
+ return pgfs, err
+ }
+ }
+ return pgfs, nil
+ }
+
+ promises, firstErr := c.startParse(mode, fhs...)
// Await all parsing.
var g errgroup.Group
g.SetLimit(runtime.GOMAXPROCS(-1)) // parsing is CPU-bound.
- pgfs := make([]*source.ParsedGoFile, len(fhs))
for i, promise := range promises {
if promise == nil {
continue
@@ -196,77 +280,22 @@
return nil
})
}
- if err := g.Wait(); err != nil {
- return nil, nil, err
+
+ if err := g.Wait(); err != nil && firstErr == nil {
+ firstErr = err
}
- // Construct a token.FileSet mapping all parsed files, and update their
- // Tok to the corresponding file in the new fileset.
- //
- // In the unlikely event that a parsed file no longer fits in its allocated
- // space in the FileSet range, it will need to be re-parsed.
-
+ // Augment the FileSet to map all parsed files.
var tokenFiles []*token.File
- fileIndex := make(map[*token.File]int) // to look up original indexes after sorting
- for i, pgf := range pgfs {
+ for _, pgf := range pgfs {
if pgf == nil {
continue
}
- fileIndex[pgf.Tok] = i
tokenFiles = append(tokenFiles, pgf.Tok)
}
+ tokeninternal.AddExistingFiles(fset, tokenFiles)
- sort.Slice(tokenFiles, func(i, j int) bool {
- return tokenFiles[i].Base() < tokenFiles[j].Base()
- })
-
- var needReparse []int // files requiring reparsing
- out := tokenFiles[:0]
- for i, f := range tokenFiles {
- if i < len(tokenFiles)-1 && f.Base()+f.Size() >= tokenFiles[i+1].Base() {
- if f != tokenFiles[i+1] { // no need to re-parse duplicates
- needReparse = append(needReparse, fileIndex[f])
- }
- } else {
- out = append(out, f)
- }
- }
- fset := source.FileSetFor(out...)
-
- // Re-parse any remaining files using the stitched fileSet.
- for _, i := range needReparse {
- // Start from scratch, rather than using ParsedGoFile.Src, so that source
- // fixing operates exactly the same (note that fixing stops after a limited
- // number of tries).
- fh := fhs[i]
- content, err := fh.Content()
- if err != nil {
- if firstReadError == nil {
- firstReadError = err
- }
- continue
- }
- pgfs[i] = parseGoSrc(ctx, fset, fh.URI(), content, mode)
- }
-
- // Ensure each PGF refers to a token.File from the new FileSet.
- for i, pgf := range pgfs {
- if pgf == nil {
- continue
- }
- newTok := fset.File(token.Pos(pgf.Tok.Base()))
- if newTok == nil {
- panic("internal error: missing tok for " + pgf.URI)
- }
- if newTok.Base() != pgf.Tok.Base() || newTok.Size() != pgf.Tok.Size() {
- panic("internal error: mismatching token.File in synthetic FileSet")
- }
- pgf2 := *pgf
- pgf2.Tok = newTok
- pgfs[i] = &pgf2
- }
-
- return pgfs, fset, firstReadError
+ return pgfs, firstErr
}
// -- priority queue boilerplate --
diff --git a/gopls/internal/lsp/cache/parse_cache_test.go b/gopls/internal/lsp/cache/parse_cache_test.go
index cc28a6b..817c466 100644
--- a/gopls/internal/lsp/cache/parse_cache_test.go
+++ b/gopls/internal/lsp/cache/parse_cache_test.go
@@ -8,43 +8,50 @@
"context"
"fmt"
"go/token"
+ "math/bits"
"testing"
"golang.org/x/tools/gopls/internal/lsp/source"
"golang.org/x/tools/gopls/internal/span"
)
+func skipIfNoParseCache(t *testing.T) {
+ if bits.UintSize == 32 {
+ t.Skip("the parse cache is not supported on 32-bit systems")
+ }
+}
+
func TestParseCache(t *testing.T) {
+ skipIfNoParseCache(t)
+
ctx := context.Background()
uri := span.URI("file:///myfile")
fh := makeFakeFileHandle(uri, []byte("package p\n\nconst _ = \"foo\""))
+ fset := token.NewFileSet()
var cache parseCache
- pgfs1, _, err := cache.parseFiles(ctx, source.ParseFull, fh)
+ pgfs1, err := cache.parseFiles(ctx, fset, source.ParseFull, fh)
if err != nil {
t.Fatal(err)
}
pgf1 := pgfs1[0]
- pgfs2, _, err := cache.parseFiles(ctx, source.ParseFull, fh)
+ pgfs2, err := cache.parseFiles(ctx, fset, source.ParseFull, fh)
pgf2 := pgfs2[0]
if err != nil {
t.Fatal(err)
}
- if pgf1.File != pgf2.File {
+ if pgf1 != pgf2 {
t.Errorf("parseFiles(%q): unexpected cache miss on repeated call", uri)
}
// Fill up the cache with other files, but don't evict the file above.
files := []source.FileHandle{fh}
files = append(files, dummyFileHandles(parseCacheMaxFiles-1)...)
- pgfs3, fset, err := cache.parseFiles(ctx, source.ParseFull, files...)
+ pgfs3, err := cache.parseFiles(ctx, fset, source.ParseFull, files...)
pgf3 := pgfs3[0]
- if pgf3.File != pgf1.File {
+ if pgf3 != pgf1 {
t.Errorf("parseFiles(%q, ...): unexpected cache miss", uri)
}
- if pgf3.Tok == pgf1.Tok {
- t.Errorf("parseFiles(%q, ...): unexpectedly matching token file", uri)
- }
if pgf3.Tok.Base() != pgf1.Tok.Base() || pgf3.Tok.Size() != pgf1.Tok.Size() {
t.Errorf("parseFiles(%q, ...): result.Tok has base: %d, size: %d, want (%d, %d)", uri, pgf3.Tok.Base(), pgf3.Tok.Size(), pgf1.Tok.Base(), pgf1.Tok.Size())
}
@@ -54,20 +61,22 @@
// Now overwrite the cache, after which we should get new results.
files = dummyFileHandles(parseCacheMaxFiles)
- _, _, err = cache.parseFiles(ctx, source.ParseFull, files...)
+ _, err = cache.parseFiles(ctx, fset, source.ParseFull, files...)
if err != nil {
t.Fatal(err)
}
- pgfs4, _, err := cache.parseFiles(ctx, source.ParseFull, fh)
+ pgfs4, err := cache.parseFiles(ctx, fset, source.ParseFull, fh)
if err != nil {
t.Fatal(err)
}
- if pgfs4[0].File == pgf1.File {
+ if pgfs4[0] == pgf1 {
t.Errorf("parseFiles(%q): unexpected cache hit after overwriting cache", uri)
}
}
func TestParseCache_Reparsing(t *testing.T) {
+ skipIfNoParseCache(t)
+
defer func(padding int) {
parsePadding = padding
}(parsePadding)
@@ -80,23 +89,25 @@
// Parsing should succeed even though we overflow the padding.
var cache parseCache
- _, _, err := cache.parseFiles(context.Background(), source.ParseFull, files...)
+ _, err := cache.parseFiles(context.Background(), token.NewFileSet(), source.ParseFull, files...)
if err != nil {
t.Fatal(err)
}
}
func TestParseCache_Duplicates(t *testing.T) {
+ skipIfNoParseCache(t)
+
ctx := context.Background()
uri := span.URI("file:///myfile")
fh := makeFakeFileHandle(uri, []byte("package p\n\nconst _ = \"foo\""))
var cache parseCache
- pgfs, _, err := cache.parseFiles(ctx, source.ParseFull, fh, fh)
+ pgfs, err := cache.parseFiles(ctx, token.NewFileSet(), source.ParseFull, fh, fh)
if err != nil {
t.Fatal(err)
}
- if pgfs[0].File != pgfs[1].File {
+ if pgfs[0] != pgfs[1] {
t.Errorf("parseFiles(fh, fh): = [%p, %p], want duplicate files", pgfs[0].File, pgfs[1].File)
}
}
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index e0b9a23..406a428 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -2059,9 +2059,10 @@
return false, false, false
}
+ fset := token.NewFileSet()
// Parse headers to compare package names and imports.
- oldHeads, _, oldErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseHeader, oldFH)
- newHeads, _, newErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseHeader, newFH)
+ oldHeads, oldErr := lockedSnapshot.parseCache.parseFiles(ctx, fset, source.ParseHeader, oldFH)
+ newHeads, newErr := lockedSnapshot.parseCache.parseFiles(ctx, fset, source.ParseHeader, newFH)
if oldErr != nil || newErr != nil {
// TODO(rfindley): we can get here if newFH does not exist. There is
@@ -2112,8 +2113,8 @@
// Note: if this affects performance we can probably avoid parsing in the
// common case by first scanning the source for potential comments.
if !invalidate {
- origFulls, _, oldErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseFull, oldFH)
- newFulls, _, newErr := lockedSnapshot.parseCache.parseFiles(ctx, source.ParseFull, newFH)
+ origFulls, oldErr := lockedSnapshot.parseCache.parseFiles(ctx, fset, source.ParseFull, oldFH)
+ newFulls, newErr := lockedSnapshot.parseCache.parseFiles(ctx, fset, source.ParseFull, newFH)
if oldErr == nil && newErr == nil {
invalidate = magicCommentsChanged(origFulls[0].File, newFulls[0].File)
} else {
diff --git a/gopls/internal/lsp/cache/symbols.go b/gopls/internal/lsp/cache/symbols.go
index 1611334..a254a0c 100644
--- a/gopls/internal/lsp/cache/symbols.go
+++ b/gopls/internal/lsp/cache/symbols.go
@@ -62,7 +62,7 @@
// It may use a parsed file already present in the cache but
// otherwise does not populate the cache.
func symbolizeImpl(ctx context.Context, snapshot *snapshot, fh source.FileHandle) ([]source.Symbol, error) {
- pgfs, _, err := snapshot.parseCache.parseFiles(ctx, source.ParseFull, fh)
+ pgfs, err := snapshot.parseCache.parseFiles(ctx, token.NewFileSet(), source.ParseFull, fh)
if err != nil {
return nil, err
}
