aboutsummaryrefslogtreecommitdiff
path: root/internal/lsp/cache/parse.go
AgeCommit message (Collapse)Author
2022-09-07gopls: migrate internal/lsp to gopls/internal/lspRobert Findley
This CL was created using the following commands: ./gopls/internal/migrate.sh git add . git codereview gofmt For golang/go#54509 Change-Id: Iceeec602748a5e6f609c3ceda8d19157e5c94009 Reviewed-on: https://go-review.googlesource.com/c/tools/+/426796 gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Peter Weinberger <pjw@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-14internal/memoize: rename Handle to PromiseAlan Donovan
Also: - add test of NewHandle - update package doc and other doc comments - factor Store.Handle with NewHandle - declare Handle before Store Change-Id: I4bcea2c9debf1e77f973ef7ea9dbe2fd7a373996 Reviewed-on: https://go-review.googlesource.com/c/tools/+/417417 Auto-Submit: Alan Donovan <adonovan@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Robert Findley <rfindley@google.com> Run-TryBot: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-07-13internal/lsp/cache: don't trim unexported struct fieldsAlan Donovan
The trimming optimization deletes parts of the syntax tree that don't affect the type checking of package-level declarations. It used to remove unexported struct fields, but this had observable consequences: it would affect the offset of later fields, and the size and aligment of structs, causing the 'fieldalignment' analyzer to report incorrect findings. Also, it required a complex workaround in the UI element for hovering over a type to account for the missing parts. This change restores unexported fields. The logic of recordFieldsUses has been inlined and specialized for each case (params+results, struct fields, interface methods) as they are more different than alike. BenchmarkMemStats on k8s shows +4% HeapAlloc: a lot, but a small part of the 32% saving of the trimming optimization as a whole. Also: - trimAST: delete func bodies without visiting them. - minor clarifications. Updates golang/go#51016 Change-Id: Ifae15564a8fb86af3ea186af351a2a92eb9deb22 Reviewed-on: https://go-review.googlesource.com/c/tools/+/415503 gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Alan Donovan <adonovan@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2022-07-13internal/lsp/cache: move PosTo{Decl,Field} out of cacheAlan Donovan
Before, these methods of the Source interface used to use a cache of buildASTCache, which built a Pos-keyed map for a whole file, but the necessary algorithm is essentially a binary search which is plenty fast enough to avoid the need for cache. This change implements that algorithm and moves both methods out of the interface into a single function, source.FindDeclAndField. -- I measured the duration of all calls to astCacheData (before) and FindDeclAndField (after) occurring within this command: $ go test -bench=TestBenchmarkConfiguredCompletion -v ./gopls/internal/regtest/bench -completion_workdir=$HOME/w/kubernetes -completion_file=../kubernetes/pkg/generated/openapi/zz_generated.openapi.go -completion_regexp=Get (The numbers reported by this benchmark are problematic, which is why I measured call times directly; see https://github.com/golang/go/issues/53798.) Results: before (n=4727) max = 21ms, 90% = 4.4ms, median = 19us after (n=6282) max = 2.3ms, 90% = 25us, median = 14us The increased number of calls to the function after the change is due to a longstanding bug in the benchmark: each iteration of the b.N loop doesn't do a fixed amount of work, it does as much as it can in 10s. Thus making the code faster simply causes the benchmark to spend the same amount of time on other parts of the program--such as the loop that calls FindDeclAndField. See https://go-review.googlesource.com/c/tools/+/221021 for background on the previous implementation. Change-Id: I745ecc4e65378fbe97f456228cafba84105b7e49 Reviewed-on: https://go-review.googlesource.com/c/tools/+/416880 Auto-Submit: Alan Donovan <adonovan@google.com> Run-TryBot: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
2022-07-08internal/memoize: delete Generation and BindAlan Donovan
Now that the lifetime of all handles in the store is determined by reference counting, we no longer need the generation feature. The Arg interface, renamed RefCounted, is now optional, and causes the lifetime of the argument to be extended for the duration of the Function call. This is important when the Get(ctx) context is cancelled, causing the function call to outlive Get: if Get's reference to the argument was borrowed, it needs to increase the refcount to prevent premature destruction. Also: - add missing snapshot.release() call in importsState.populateProcessEnv. - remove the --memoize_panic_on_destroyed flag. Change-Id: I0b3d37c16f8b3f550bb10120c066b628c3db244b Reviewed-on: https://go-review.googlesource.com/c/tools/+/416076 Run-TryBot: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Alan Donovan <adonovan@google.com> Reviewed-by: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-08internal/lsp/cache: simplify ParseGoAlan Donovan
This change simplifes the ParseGo interface to make it consistent with the other handle+map operations: - ParseGoImpl is the basic parser. - The 'fixed' bool result is a field of ParsedGoFile. - ParseGo is the caching wrapper. The map accessors have been inlined into it. - goFiles (renamed parsedGoFiles) is now just a bare persistent.Map. - parseGoHandle is replaced by *memoize.Handle - the operations of "make a handle" and "wait for it" are no longer separate (since clients never want one without the other). - cachedPGF and peekOrParse have been combined into peekParseGoLocked. Change-Id: If01a6aaa7e6a8d78cb89c305e5279738e8e7bb55 Reviewed-on: https://go-review.googlesource.com/c/tools/+/416223 TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Alan Donovan <adonovan@google.com>
2022-07-07internal/lsp/cache: use GetHandle not Bind in astCacheDataAlan Donovan
This change replaces Bind (generational lifetime) with GetHandle (reference counting) for the cache of buildASTCache calls, as Bind is deprecated. Also: - add missing commentary, particularly on the question of why this cache is needed at all. - remove unused field astCacheData.err - simplify SignatureHelp to avoid unnecessary use of Declaration. - minor simplifications to logic surrounding FindPackageFromPos and PosTo{Decl,Field}. Change-Id: I2b7a798b84f23856037797fa6e9ccc5595422e7c Reviewed-on: https://go-review.googlesource.com/c/tools/+/415975 Reviewed-by: Robert Findley <rfindley@google.com> Run-TryBot: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Alan Donovan <adonovan@google.com> gopls-CI: kokoro <noreply+kokoro@google.com>
2022-07-01internal/memoize: delete Bind(cleanup) hookAlan Donovan
Now that the workspace directory uses Snapshot.Destroy to clean up (see https://go-review.googlesource.com/c/tools/+/414499) there is no need for this feature. Change-Id: Id5782273ce5030b4fb8f3b66a8d16a45a831ed91 Reviewed-on: https://go-review.googlesource.com/c/tools/+/414500 Reviewed-by: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Auto-Submit: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Alan Donovan <adonovan@google.com>
2022-06-27internal/memoize: don't destroy reference counted handlesRobert Findley
Unlike generational handles, when reference counted handles are evicted from the Store we don't know that they are also no longer in use by active goroutines. Destroying them causes goroutine leaks. Also fix a data race because Handle.mu was not acquired in the release func returned by GetHandle. Change-Id: Ida7bb6961a035dd24ef8566c7e4faa6890296b5b Reviewed-on: https://go-review.googlesource.com/c/tools/+/414455 Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-06-22internal/lsp/cache: use persistent map for storing gofiles in the snapshotRuslan Nigmatullin
Use treap (https://en.wikipedia.org/wiki/Treap) as a persistent map to avoid copying s.goFiles across generations. Maintain an additional s.parseKeysByURIMap to avoid scanning s.goFiles on individual file's content invalidation. This on average reduces didChange latency on internal codebase from 160ms to 150ms. In a followup the same approach can be used to avoid copying s.files, s.packages, and s.knownSubdirs. Updates golang/go#45686 Change-Id: Ic4a9b3c8fb2b66256f224adf9896ddcaaa6865b1 GitHub-Last-Rev: 0abd2570ae9b20ea7126ff31bee69aa0dc3f40aa GitHub-Pull-Request: golang/tools#382 Reviewed-on: https://go-review.googlesource.com/c/tools/+/411554 Reviewed-by: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> Run-TryBot: Robert Findley <rfindley@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-06-17internal/lsp/debug: reduce critical sections in traceAlan Donovan
This change reduces the sizes of the critical section in traces.ProcessEvent, in particular moving allocations ahead of Lock. This reduces the contention according to the trace profiler. See https://go-review.googlesource.com/c/go/+/411909 for another reduction in contention. The largest remaining contention is Handle.Get, which thousands of goroutines wait for because we initiate typechecking top down. (Second attempt at https://go-review.googlesource.com/c/tools/+/411910, reverted in https://go-review.googlesource.com/c/tools/+/412694. The changes to Generation.Bind have been dropped.) Change-Id: Ia9050c97bd12d2d75055f8d1dfcda3ef1f5ad334 Reviewed-on: https://go-review.googlesource.com/c/tools/+/412820 Run-TryBot: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-06-16Revert "internal/lsp/cache: reduce critical sections"Alan Donovan
This reverts commit 654a14b5274602698564a5e9710c0778be664c7a. Reason for revert: my flawed understanding of the concurrency Change-Id: I31a35267323bb1ff4dff1d9244d3ce69c36cdda4 Reviewed-on: https://go-review.googlesource.com/c/tools/+/412694 Run-TryBot: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com> gopls-CI: kokoro <noreply+kokoro@google.com>
2022-06-15internal/lsp/cache: reduce critical sectionsAlan Donovan
This change reduces the sizes of the critical sections in traces.ProcessEvent and Generation.Bind, in particular moving allocations ahead of Lock. This reduces the contention according to the trace profiler. See https://go-review.googlesource.com/c/go/+/411909 for another reduction in contention. The largest remaining contention is Handle.Get, which thousands of goroutines wait for because we initiate typechecking top down. Also, add a couple of possible optimization TODO comments, and delete a stale comment re: Bind. Change-Id: I995a0bb46e8c9bf0c23492fb62b56f4539bc32f8 Reviewed-on: https://go-review.googlesource.com/c/tools/+/411910 Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
2022-06-01internal/span: eliminate TokenConverterRobert Findley
The TokenConverter has been trimmed down to a thin wrapper around token.File, and can now be removed. Change-Id: I9985492274c88e6a13e6d62dadab5595c75c7840 Reviewed-on: https://go-review.googlesource.com/c/tools/+/406134 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Alan Donovan <adonovan@google.com>
2022-05-23internal/lsp/cache: invalidate metadata when parsing issues resolveRobert Findley
Package loading (at least via go list) fails when the import header doesn't parse, so we need to invalidate metadata upon a state change from non parsing->parsing. Refactor metadata invalidation to implement this feature, add additional documentation, and avoid unnecessary work. This change revealed a latent bug (via TestDeleteDirectory): when statting a deleted directory failed, we could fail to invalidate any package IDs, even those we already knew about. This bug was masked by the somewhat complicated semantics of pkgNameChanged. The semantics of pkgNameChanged are simplified to encapsulate any change to a package->file association. Also refactor the parsing API somewhat, and add a bug report if we don't get a ParseGoFile while inspecting for metadata changes. Update most regtests to panic upon receiving bug reports. Fixes golang/go#52981 Change-Id: I1838963ecc9c01e316f887aa9d8f1260662281ab Reviewed-on: https://go-review.googlesource.com/c/tools/+/407501 Reviewed-by: Alan Donovan <adonovan@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Findley <rfindley@google.com>
2022-05-17internal/span: eliminate Converter and FileConverterRobert Findley
The only real implementation of position conversion was via a *token.File, so refactor the converter logic to eliminate the Converter interface, and just use a single converter implementation that uses a *token.File to convert between offsets and positions. This change is meant to be a zero-impact refactoring for non-test code. As such, I abstained from panicking in several places where it would make sense. In later CLs, once the bug reporting API lands, we can insert bug reports in these places. Change-Id: Id2e503acd80d089bc5d73e983215784015471f04 Reviewed-on: https://go-review.googlesource.com/c/tools/+/405546 gopls-CI: kokoro <noreply+kokoro@google.com> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
2022-05-09internal/lsp: factor out go/token wrapper into a safetoken packageRobert Findley
This avoids an import cycle that prevented these wrappers from being used in the lsppos package. Change-Id: I9eedd256db983dfcf962edba39e3d4f3a1aabdeb Reviewed-on: https://go-review.googlesource.com/c/tools/+/403680 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Robert Findley <rfindley@google.com> Reviewed-by: Alan Donovan <adonovan@google.com> gopls-CI: kokoro <noreply+kokoro@google.com>
2022-04-20gopls: remove usage of golang.org/x/xerrorsRobert Findley
As of golang/go#50827, gopls no longer supports building at 1.12, and so usage of golang.org/x/xerrors can be replaced with the native support for error wrapping introduced in Go 1.13. Remove this usage as a step toward eliminating the xerrors dependency from x/tools. For golang/go#52442 Change-Id: Ibf459cc72402a30a6c2735dc620f76ed8a5e2525 Reviewed-on: https://go-review.googlesource.com/c/tools/+/401097 Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-04-12gopls, internal/lsp: gofmtRuss Cox
Gofmt to update doc comments to the new formatting. (There are so many files in x/tools I am breaking up the gofmt'ing into multiple CLs.) For golang/go#51082. Change-Id: Ife11502fe1e59a04d53dba9edccd3043e57f9ae8 Reviewed-on: https://go-review.googlesource.com/c/tools/+/399358 Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gopher Robot <gobot@golang.org> Auto-Submit: Russ Cox <rsc@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com>
2022-01-13internal/template: identify template files by the templateExtensions optionpjw
Make the language id (sent from the client) 'gotmpl' equivalent to 'tmpl' Wherever a view is known, use its options to determine which files are template files. Whenever the client sends an explicit languageID, use that. Partially fixes golang/vscode-go#1957 Change-Id: I04cd630d6c6c80e0a78c2fafb6ddc1166ce86829 Reviewed-on: https://go-review.googlesource.com/c/tools/+/376854 Trust: Peter Weinberger <pjw@google.com> Reviewed-by: Robert Findley <rfindley@google.com>
2021-11-05internal/lsp/cache: don't offset invalid positionsRobert Findley
The zero value of token.Pos is special: it determines IsValid(), which has semantic significance in many places. offsetPositions, which adjusts positions after AST mangling, was adjusting all positions -- including invalid positions. As a result it was making invalid positions valid, leaving a nonsensical AST. Fix it to skip invalid positions. Change-Id: I844f243f9a4a3f98ace32fe30e934410d11142dd Reviewed-on: https://go-review.googlesource.com/c/tools/+/361614 Trust: Robert Findley <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Suzy Mueller <suzmue@golang.org>
2021-10-12internal/lsp: use source.Offset instead of tok.OffsetRebecca Stambler
This isn't strictly necessary for some of the cases, but it's better to use it in all cases. Also added a test to ensure that we avoid (*token.File).Offset in all of gopls--test was probably overkill, but it was quick to write. Change-Id: I6dd0126e2211796d5de4e7a389386d7aa81014f0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/353890 Run-TryBot: Rebecca Stambler <rstambler@golang.org> Trust: Rebecca Stambler <rstambler@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2021-10-04internal/lsp: handle nil pointer in fixInitStmtRebecca Stambler
I'm going to make a follow-up CL to create a source.Offset function that will always check inRange, and we should use that everywhere instead of token.Offset, which is pretty prone to panicking. Fixes golang/go#48763 Change-Id: Ia81309400d15a28c133f4b3d41c6239231c2532d Reviewed-on: https://go-review.googlesource.com/c/tools/+/353889 Trust: Rebecca Stambler <rstambler@golang.org> Run-TryBot: Rebecca Stambler <rstambler@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org>
2021-08-15internal/lsp: export and move some objects related to metadataRob Findley
In preparation for moving metadata related functionality to a separate package, move around some types and export some symbols. This is purely to reduce diffs in subsequent CLs, and contains no functional changes. Change-Id: I24d4fbd71df78e4d7a84f6598cdf820b41d542a2 Reviewed-on: https://go-review.googlesource.com/c/tools/+/340729 Trust: Robert Findley <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-08-02internal/lsp: handle invalid positions in semantic token debug logicRebecca Stambler
Check that a position is in range before using it. Fixes golang/vscode-go#1656 Change-Id: I1598ebab76a1775afd8f63b9849049b31fb74a8b Reviewed-on: https://go-review.googlesource.com/c/tools/+/339169 Trust: Rebecca Stambler <rstambler@golang.org> Run-TryBot: Rebecca Stambler <rstambler@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Peter Weinberger <pjw@google.com>
2021-07-21internal/lsp: handle panic in fix ASTRebecca Stambler
I'm not sure how this can happen, but it seems possible that a bad expression might somehow have an invalid position. Fixes golang/go#47231 Change-Id: I0794bdfb66f668fc375e9fe561c9f239c8b92492 Reviewed-on: https://go-review.googlesource.com/c/tools/+/334892 Trust: Rebecca Stambler <rstambler@golang.org> Run-TryBot: Rebecca Stambler <rstambler@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2021-07-08internal/lsp/cache: be consistent about using snapshot.FileSetRob Findley
Ideally we could at some point break the snapshot->view->session->cache reverse traversal, but for now at least don't copy this pattern around everywhere. Change-Id: Ib144e6d322016f5b9563f21c56a0691c1a8ec97d Reviewed-on: https://go-review.googlesource.com/c/tools/+/309270 Trust: Robert Findley <rfindley@google.com> Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-06-03internal/lsp: address some staticcheck warningRebecca Stambler
Change-Id: I5eea4d35ef6ad4159ca96ba2765477c4603a1ca6 Reviewed-on: https://go-review.googlesource.com/c/tools/+/324396 Trust: Rebecca Stambler <rstambler@golang.org> Run-TryBot: Rebecca Stambler <rstambler@golang.org> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2021-05-11internal/lsp/cache: trim more stuff in ParseExported modeHeschi Kreinick
Despite the name, ParseExported only hollowed out declarations -- it didn't actually drop any from the AST. This leaves a fair amount of unexported crud behind. Unfortunately, there are a *lot* of ways to expose an unexported declaration from an exported one, and it can be done across files. Because of that, discarding unexported declarations requires a lot of work. This CL implements a decent attempt at pruning as much as possible from the AST in ParseExported mode. First, we analyze the AST of all the files in the package for exported uses of unexported identifiers, iterating to a fixed point. Then, we type check those ASTs. If there are missing identifiers (probably due to a bug in the dependency analysis) we use those errors to re-parse. After that we give up and fall back to the older, less effective trimming. The pkg type changes slightly to accomodate the new control flow. We have to analyze all the files at once because an unexported type might be exposed in another file. Unfortunately, that means we can't parse a single file at a time any more -- the result of parsing a file depends on the result of parsing its siblings. To avoid cache corruption, we have to do the parsing directly in type checking, uncached. This, in turn, required changes to the PosTo* functions. Previously, they operated just on files, but a file name is no longer sufficient to get a ParseExported AST. Change them to work on Packages instead. I squeezed in a bit of refactoring while I was touching them. Change-Id: I61249144ffa43ad645ed38d79e873e3998b0f38d Reviewed-on: https://go-review.googlesource.com/c/tools/+/312471 Trust: Heschi Kreinick <heschi@google.com> Run-TryBot: Heschi Kreinick <heschi@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2021-03-02internal/lsp/cache: refactor Go file parsingHeschi Kreinick
Hopefully improve some of the details around parsing that have always confused me. - parser.ParseFile will never return an error other than scanner.ErrorList. Encode that in ParsedGoFile. - parser.ParseFile will never return a nil file. Eliminate the code path that handled that. - Explain why we might fail to find a token.File. - Trying to fix errors appears quite expensive even if there aren't any to fix. Don't waste the time. Change-Id: I87e082ed52c98a438bc7fd3b29e1a486f32fb347 Reviewed-on: https://go-review.googlesource.com/c/tools/+/297069 Trust: Heschi Kreinick <heschi@google.com> Run-TryBot: Heschi Kreinick <heschi@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2021-02-05internal/lsp/cache: allow fixing multiple syntax errorsMuir Manders
Allow fixSrc to run multiple times on a file. For example: func main() { var s S if s.<> } type S struct { i int } To properly complete at <>, we need to perform two fixes. We must insert a phantom underscore after "s." to deal with the dangling selector, and then we must insert phantom "{}" for the "if" statement to deal with the incomplete "if" block. Previously we stopped at one fix, but now we allow for up to 10. I added a limit because I am afraid there are cases where we could get stuck trying to apply the same fix again and again. Also, drive-by set isIncomplete=true when we return early in lsp/completion.go. I have found my editor incorrectly caches this zero result in certain cases because it thinks results are "complete". Fixes golang/go#43471. Change-Id: Idd34cc164d579fa12a27920dc3afb372799abf26 Reviewed-on: https://go-review.googlesource.com/c/tools/+/289271 Run-TryBot: Muir Manders <muir@mnd.rs> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Hyang-Ah Hana Kim <hyangah@gmail.com> Trust: Rebecca Stambler <rstambler@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-10-30internal/memoize: add a final argument to Bind for cleaning upRob Findley
With its memoization and refcounting, the cache is well suited to the sharing of other expensive resources, specifically those that interact with the file system. However, it provides no means to clean up those resources when they are no longer needed. Add an additional argument to Bind to clean up any values produced by the bound function when they are no longer referenced. For golang/go#41836 Change-Id: Icb2b12949de06f2ec7daf868f12a9c699540fa5b Reviewed-on: https://go-review.googlesource.com/c/tools/+/263937 Run-TryBot: Robert Findley <rfindley@google.com> gopls-CI: kokoro <noreply+kokoro@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com> Trust: Robert Findley <rfindley@google.com>
2020-08-12internal/lsp/cache: trim ellipsis array literalsHeschi Kreinick
While looking at Kubernetes I noticed that golang.org/x/text packages were some of the largest. The problem is the large code-generated tables, which use ellipsis array literals. Teach gopls to trim the cases that matter there. While silly, this trims ~60MB off the live heap, so I think it might be worth it. Change-Id: I0cfd80bd5fbc8703ac628312982af9c6ed871758 Reviewed-on: https://go-review.googlesource.com/c/tools/+/248180 Run-TryBot: Heschi Kreinick <heschi@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-10internal/memoize: switch from GC-driven to explicit deletionHeschi Kreinick
The GC-based cache has given us a number of problems. First, memory leaks driven by reference cycles: the Go runtime cannot collect cycles involving finalizers, which prevents us from writing natural code in Bind callbacks. If we screw it up, we get a mysterious leak that takes a long time to track down. Second, the behavior is generally mysterious; it's hard to predict how long a value lasts, and harder to tell if a value being live is a bug. Third, we think that it may be interacting poorly with the GC, resulting in unnecessary memory usage. The structure of the values we put in the cache is not actually that complicated -- there are only 5 significant types: parse, typecheck, analyze, parse mod, and analyze mod. Managing them manually should not be conceptually difficult, and in fact we already do most of the work in (*snapshot).clone. In this CL the cache adds the concept of "generations", which function as reference counts on cache entries. Entries are still global and shared across generations, but will be explicitly deleted once no generations refer to them. The idea is that each snapshot is a new generation, and can inherit entries from the previous snapshot or leave them behind to be deleted. One obvious risk of this scheme is that we'll leave dangling references to values without actually inheriting them across generations. To prevent that, getting a value requires passing in the generation at which it's being read, and an error will be returned if that generation is dead. Change-Id: I4b30891efd7be4e10f2b84f4c067b0dee43dcf9c Reviewed-on: https://go-review.googlesource.com/c/tools/+/242838 Run-TryBot: Heschi Kreinick <heschi@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-04internal/lsp/cache: store parseGoHandlesHeschi Kreinick
parseGoHandles have lifetimes separate from the packages they belong to. For example, a package may be invalidated by a change to one of its files, but we still want to retain the parse results for all the rest. Track them explicitly. Change-Id: I03a4ffe283bf2b252d2d838bdb2cf332cd981075 Reviewed-on: https://go-review.googlesource.com/c/tools/+/245059 Run-TryBot: Heschi Kreinick <heschi@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-08-03internal/lsp: separate LSP files from FS filesHeschi Kreinick
FileHandle currently includes LSP-level information about Version and Session. That's dangerous, because the cache operates in terms of URIs and content only -- we explicitly want to share results across sessions and versions if they happen to be the same. Split the LSP information into separate types, VersionedFileHandle and VersionedFileIdentity. Change-Id: I158646b783375b58245468599301e2a29c657e71 Reviewed-on: https://go-review.googlesource.com/c/tools/+/245058 Run-TryBot: Heschi Kreinick <heschi@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2020-08-03internal/lsp: move builtin package to SnapshotHeschi Kreinick
The builtin package was the one special case where we parsed Go outside the context of a Snapshot. Move it up. Change-Id: I1f4bb536adb40019e0ea9c5c89f38b15737abb8c Reviewed-on: https://go-review.googlesource.com/c/tools/+/245057 Run-TryBot: Heschi Kreinick <heschi@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28internal/lsp: replace ParseGoHandle with concrete dataHeschi Kreinick
ParseGoHandles serve two purposes: they pin cache entries so that redundant calculations are cached, and they allow users to obtain the actual parsed AST. The former is an implementation detail, and the latter turns out to just be an annoyance. Parsed Go files are obtained from two places. By far the most common is from a type checked package. But a type checked package must by definition have already parsed all the files it contains, so the PGH is already computed and cannot have failed. Type checked packages can simply return the parsed file without requiring a separate Check operation. We do want to pin the cache entries in this case, which I've done by holding on to the PGH in cache.pkg. There are some cases where we directly parse a file, such as for the FoldingRange LSP call, which doesn't need type information. Those parses can actually fail, so we do need an error check. But we don't need the PGH; in all cases we are immediately using and discarding it. So it turns out we don't actually need the PGH type at all, at least not in the public API. Instead, we can pass around a concrete struct that has the various pieces of data directly available. This uncovered a bug in typeCheck: it should fail if it encounters any real errors. Change-Id: I203bf2dd79d5d65c01392d69c2cf4f7744fde7fc Reviewed-on: https://go-review.googlesource.com/c/tools/+/244021 Run-TryBot: Heschi Kreinick <heschi@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28internal/lsp/cache: fix parseKeyHeschi Kreinick
The FileIdentity struct mixes information about the file itself (filename, hash) with information about the LSP references to that file (session ID, version). When we create a cache key using it, we only want the former, as returned by the String method. Otherwise we split the cache whenever those irrelevant fields are different. We also use FileIdentity as an element of diagnosticsKey, but I believe that use is appropriate. Change-Id: I094e00d2700e05778da635effbb69d0ebcb6726e Reviewed-on: https://go-review.googlesource.com/c/tools/+/244020 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28internal/lsp: pass snapshot/view to memoize.FunctionsHeschi Kreinick
Due to the runtime's inability to collect cycles involving finalizers, we can't close over handles in memoize.Functions without causing memory leaks. Up until now we've dealt with that by closing over all the bits of the snapshot that we want, but it distorts the design of all the code used in the Functions. We can solve the problem another way: instead of closing over the snapshot/view, we can force the caller to pass it in. This is somewhat scary: there is no requirement that the argument matches the data that we're working with. But the reality is that this is not a new problem: the Function used to calculate a cache value is not necessarily the one that the caller expects. As long as the cache key fully identifies all the inputs to the Function, the output should be correct. And since the caller used the snapshot/view to calculate that cache key, it should always be safe to pass in that snapshot/view. If it's not, then we already had a bug. The Arg type in memoize is clumsy, but I thought it would be nice to have at least a little bit of type safety. I'm open to suggestions. Change-Id: I23f546638b0c66a4698620a986949087211f4762 Reviewed-on: https://go-review.googlesource.com/c/tools/+/244019 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-28internal/lsp: minimize PackageHandle interfaceHeschi Kreinick
The PackageHandle interface is fairly redundant with the Package interface. As much as convenient, move users to Package and weaken/remove methods from PackageHandle. I would like to get rid of CompiledGoFiles too but NarrowestPackageHandle is a little annoying. I think this is unambiguously a step forward so I figured we can get it in and keep iterating. Change-Id: I6c5a3f462b1f19cbca6a267fedc36ce54613b6fc Reviewed-on: https://go-review.googlesource.com/c/tools/+/244018 Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-07-24internal/lsp: add parse errors as diagnostics even when parsing failsRebecca Stambler
When a package consists of files that only fail to parse, we fail to associate parse errors with the package, and therefore return no diagnostics. To address this, we need to associate the errors with the package. This involves adding a *token.File to the parseGoData so that error messages and positions can be properly extracted, even when there is no associated AST. Fixes golang/go#39763 Change-Id: I5620821b9bcbeb499c498f9061dcf487d159bed5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/243579 Run-TryBot: Rebecca Stambler <rstambler@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-07-01internal/lsp/source: speed up completion candidate formattingMuir Manders
Completion could be slow due to calls to astutil.PathEnclosingInterval for every candidate during formatting. There were two reasons we called PEI: 1. To properly render type alias names, we must refer to the AST because the alias name is not available in the typed world. Previously we would call PEI to find the *type.Var's corresponding *ast.Field, but now we have a PosToField cache that lets us jump straight from the types.Object's token.Pos to the corresponding *ast.Field. 2. To display an object's documentation we must refer to the AST. We need the object's declaring node and any containing ast.Decl. We now maintain a special PosToDecl cache so we can avoid the PEI call in this case as well. We can't use a single cache for both because the *ast.Field's position is present in both caches (but points to different nodes). The caches are memoized to defer generation until they are needed and to save work creating them if the *ast.Files haven't changed. These changes speed up completing the fields of github.com/aws/aws-sdk-go/service/ec2 from 18.5s to 45ms on my laptop. Fixes golang/go#37450. Change-Id: I25cc5ea39551db728a2348f346342ebebeddd049 Reviewed-on: https://go-review.googlesource.com/c/tools/+/221021 Run-TryBot: Muir Manders <muir@mnd.rs> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-06-24internal/memoize: add an error return to (*handle).GetRebecca Stambler
Fixes golang/go#36004 Change-Id: I8da7c21eaa9cf6ffac12aabdd6803d06781cef32 Reviewed-on: https://go-review.googlesource.com/c/tools/+/239564 Run-TryBot: Rebecca Stambler <rstambler@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-06-11internal/lsp: read files eagerlyHeschi Kreinick
We use file identities pervasively throughout gopls. Prior to this change, the identity is the modification date of an unopened file, or the hash of an opened file. That means that opening a file changes its identity, which causes unnecessary churn in the cache. Unfortunately, there isn't an easy way to fix this. Changing the cache key to something else, such as the modification time, means that we won't unify cache entries if a change is made and then undone. The approach here is to read files eagerly in GetFile, so that we know their hashes immediately. That resolves the churn, but means that we do a ton of file IO at startup. Incidental changes: Remove the FileSystem interface; there was only one implementation and it added a fair amount of cruft. We have many other places that assume os.Stat and such work. Add direct accessors to FileHandle for URI, Kind, and Version. Most uses of (FileHandle).Identity were for stuff that we derive solely from the URI, and this helped me disentangle them. It is a *ton* of churn, though. I can revert it if you want. Change-Id: Ia2133bc527f71daf81c9d674951726a232ca5bc9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/237037 Run-TryBot: Heschi Kreinick <heschi@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-05-07internal/lsp/cache: avoid string(int) conversionsmasher164
LoadMode and ParseMode are currently hashed with a string(int) conversion, but this conversion is now discouraged (see the vet check 'stringintconv' for more information). Since the hash uses the code point, this change replaces these instances with string(rune(int)). Updates golang/go#32479. Change-Id: I0d8e91d073fc34ac9faafe75a0d86cf71a5327d4 Reviewed-on: https://go-review.googlesource.com/c/tools/+/232679 Reviewed-by: Rebecca Stambler <rstambler@golang.org> Run-TryBot: Rebecca Stambler <rstambler@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
2020-04-23internal/event: renaming the main event API functionsIan Cottrell
event.Log removed event.Print -> event.Log event.Record -> event.Metric event.StartSpan -> event.Start In order to support this core now exposes the MakeEvent and Export functions. Change-Id: Ic7550d88dbf400e32c419adbb61d1546c471841e Reviewed-on: https://go-review.googlesource.com/c/tools/+/229238 Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-23internal/telemetry: renaming to internal/eventIan Cottrell
internal/telemetry/event was renamed to internal/event/core Some things were partly moved from internal/telemetry/event straight to internal/event to minimize churn in the following restructuring. Change-Id: I8511241c68d2d05f64c52dbe04748086dd325158 Reviewed-on: https://go-review.googlesource.com/c/tools/+/229237 Run-TryBot: Ian Cottrell <iancottrell@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Findley <rfindley@google.com>
2020-04-10internal/lsp/cache: hide type errors if we fix up the ASTRebecca Stambler
I was curious about why were logging errors during type-checking in tests, and the answer turned out to be a bit more sinister than I expected. We were getting type error messages without filepaths, so I tried to reproduce it in the playground and wasn't able to. I realized that these errors were coming from were coming from the "fixed" version of the AST that we pass to the type checker. Adding fake positions to our fake Cond statements trivially fixes the logging issue, but it does nothing to handle the fact that the error makes no sense to the user - because it applies to something that's not in the source code. I figured we have two options: (1) skip type errors for all packages with "fixed" ASTs, or (2) add something to the error messages to indicate that the source code may not match. Starting with (1) here, and if it becomes a problem, we can move to 2. All ASTs that we fix have *ast.BadExpr in them, meaning that, by definition they have parse errors which we will preferentially show those errors to users in diagnostics (so I'm not sure how to test this). Change-Id: I17733968aa15f989cdd3e4e7261c4f4fe9b97495 Reviewed-on: https://go-review.googlesource.com/c/tools/+/227557 Run-TryBot: Rebecca Stambler <rstambler@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Heschi Kreinick <heschi@google.com>
2020-03-27internal/lsp/source: remove unused parameters from functionsRohan Challa
This change uses the new unusedparams analyzer to remove any unused parameters from functions inside of internal/lsp/source :) Change-Id: I220100e832971b07cd80a701cd8b293fe708af3d Reviewed-on: https://go-review.googlesource.com/c/tools/+/225997 Run-TryBot: Rohan Challa <rohan@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rebecca Stambler <rstambler@golang.org>
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-11 21:03:55 +0000