internal/lsp/cache: invalidate analysis results on packages.Load
Most invalidation happens in snapshot.clone, but to be safe we were also
invalidating package data when new metadata is set via packages.Load. At
the time, I wasn't sure why this was necessary.
Now I understand: with experimentalUseInvalidMetadata it is possible
that we re-compute invalidated data using stale metadata in between the
initial clone and the subsequent reload. I noticed that it was also
possible to have stale analysis results after the Load results arrive.
Fix this by invalidating analysis results on Load, in addition to
packages. Factor out invalidation into a new helper method.
Since we believe this may fix analyzer panics, re-enable strict handling
of analysis panics during tests.
For golang/go#56035
For golang/go#54762
For golang/go#42857
Change-Id: I8c28e0700f8c16c58df4ecf60f6127b1c05d6dc5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/420538
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index d7e39b3..1587a5d 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -1856,26 +1856,7 @@
addRevDeps(id, invalidateMetadata)
}
- // Delete invalidated package type information.
- for id := range idsToInvalidate {
- for _, mode := range source.AllParseModes {
- key := packageKey{mode, id}
- result.packages.Delete(key)
- }
- }
-
- // Copy actions.
- // TODO(adonovan): opt: avoid iteration over s.actions.
- var actionsToDelete []actionKey
- s.actions.Range(func(k, _ interface{}) {
- key := k.(actionKey)
- if _, ok := idsToInvalidate[key.pkgid]; ok {
- actionsToDelete = append(actionsToDelete, key)
- }
- })
- for _, key := range actionsToDelete {
- result.actions.Delete(key)
- }
+ result.invalidatePackagesLocked(idsToInvalidate)
// If a file has been deleted, we must delete metadata for all packages
// containing that file.
@@ -2050,6 +2031,35 @@
return invalidated
}
+// invalidatePackagesLocked deletes data associated with the given package IDs.
+//
+// Note: all keys in the ids map are invalidated, regardless of the
+// corresponding value.
+//
+// s.mu must be held while calling this function.
+func (s *snapshot) invalidatePackagesLocked(ids map[PackageID]bool) {
+ // Delete invalidated package type information.
+ for id := range ids {
+ for _, mode := range source.AllParseModes {
+ key := packageKey{mode, id}
+ s.packages.Delete(key)
+ }
+ }
+
+ // Copy actions.
+ // TODO(adonovan): opt: avoid iteration over s.actions.
+ var actionsToDelete []actionKey
+ s.actions.Range(func(k, _ interface{}) {
+ key := k.(actionKey)
+ if _, ok := ids[key.pkgid]; ok {
+ actionsToDelete = append(actionsToDelete, key)
+ }
+ })
+ for _, key := range actionsToDelete {
+ s.actions.Delete(key)
+ }
+}
+
// fileWasSaved reports whether the FileHandle passed in has been saved. It
// accomplishes this by checking to see if the original and current FileHandles
// are both overlays, and if the current FileHandle is saved while the original
