CL 416874 changed the logic of populateProcessEnv and incorrectly
removed a write of ProcessEnv.ModFlag. We should explicitly set
-mod=readonly.
Change-Id: Ibacf3d4b4c0c978d65fde345741945d6136db159
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416877
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
This reverts CL 416879, which itself was a revert of CL 416874.
Reason for revert: failure is understood now, as described in
https://github.com/golang/go/issues/53796#issuecomment-1181157704 and fixed in CL 416881.
Change-Id: I1d6a4ae46fbb1bf78e2f23656de7885b439f43fb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416882
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
We were already using a token.File-agnostic position for object
positions in our references algorithm, but still relied on token.Pos for
identifying duplicate references. This breaks down when a file may have
multiple parsed representations in different packages.
While we should endeavor to eliminate duplicate parsing, our algorithms
should not rely on this for correctness. Update the reference
de-duplication to use the same position key as object search.
For golang/go#53796
Change-Id: Ic2e6c23380ea4e6b2747e4e5b45d7bfa6e656f0f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416881
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Previously, snapshots were born with a reference count of zero.
Although leases were created and returned by functions that
return a snapshot, and correctly disposed of by the caller, the
view did not hold a lease for its snapshot. The view's lease
on the snapshot was implicit, and for this reason the view
made explicit calls to Destroy when these implicit leases ended,
which would then wait for all explicit leases to end.
Now that the view holds an explicit lease to its snapshot, it is
safe (as a follow-up change) to move the destroy logic into the
call of release() that brings the refcount to zero.
Also:
- clarify that release functions should be called when
and only when err is nil.
- in View: remove unused View.cancel field.
- in tempModFile: clarify tricky return + defer + named-result-var statement.
- in DidModifyFiles: simplify type of releases from []func() to func().
- in Server.addFolders: fix reference leak (looks minor).
Change-Id: Ibf61f4ce109e91060e2ccd854c32214b119f2f0a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416875
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Whenever a view.importsState value is created, we use
snapshot.goCommandInvocation to extract information used to run the go
command in the imports.ProcessEnv. Before this CL, that process acquired
the current snapshot (whatever it was), and held onto it for the purpose
of sharing a workspace directory. As a consequence all the memory in
that snapshot was pinned for the lifecycle of the importsState, which
can be the entire editing session. This results in a memory leak as
information in the session is invalidated.
Fix this by creating a copy of the workspace directory to be owned by
the importsState.
Also:
- Add some TODOs
- Clean up some stale comments
Fixesgolang/go#53780
Change-Id: I2c55cc26b2d46c9320c6c7cd86b3e24971cd5073
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416874
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
...now that I have understood why they are not
in fact inessential.
Change-Id: I1ab881a7d24cd71ee183bc8c6a1a058dbda641e2
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416077
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
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>
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>
If metadata is refreshed during the execution of buildPackageHandle, we
should not store the resulting package handle in the snapshot, as it
breaks the invariant that computed packages match the currently loaded
metadata.
This strictness revealed another bug: because of our fine-grained
locking in snapshot.load, it is possible that we set valid metadata
multiple times, leading to unnecessary invalidation and potential
further races to package handles. Fix this by building all metadata when
processing the go/packages result, and only filtering updates after
grabbing the lock.
For golang/go#53733
Change-Id: Ib63ae4fbd97d0d25d45fe04f9bcd835996db41da
Reviewed-on: https://go-review.googlesource.com/c/tools/+/416224
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This change replaces the 5 remaining calls to Bind (generational
lifetime) with GetHandle (reference counting). The handles
are now stored in persistent.Maps, which simplifies the
invalidation logic.
All 5 have span.URIs as keys:
symbolizeHandles
parse{Mod,Work}Handles
mod{Tidy,Why}Handles
Also, factor the functions that use these maps to have a common form:
- a fooImpl function that returns an R result and an error;
- a foo wrapper that decorates it with caching.
- a local fooResult type, defined struct{R; error} that is the cache entry.
The functions for getting/setting map entries are all inlined.
The fooHandle types are all replaced by *memoize.Handle, now that
their use is local.
No behavior change is intended.
The other uses of Bind are deleted in these CLs:
https://go-review.googlesource.com/c/tools/+/415975 (astCacheData)
https://go-review.googlesource.com/c/tools/+/415504 (actions)
Change-Id: I77cc4e828936fe171152ca13a12f7a639299e9e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415976
Auto-Submit: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Now that we have a token.File (albeit throwaway)
for a parsed go.mod file, we can combine the .go and go.mod
logic for turning it into a protocol.DocumentLink.
Change-Id: Id1783644cbd450f0e8dc807beb8ba625675d8540
Reviewed-on: https://go-review.googlesource.com/c/tools/+/410136
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
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>
This change uses a persistent.Map for actions, just like packages.
Actions are now reference counted rather than generational.
Also:
- note optimization opportunities.
- minor cleanups.
Change-Id: Ibbac8848a3beb3fe19056a7b160d2185155e7021
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415504
gopls-CI: kokoro <noreply+kokoro@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
span.NewRange now accepts a *token.File and two token.Pos.
It is the caller's responsibility to look up the File
in the FileSet, if necessary (it usually isn't), and to
ensure the Pos values are valid. Ditto NewMappedRange.
This reduces the creep of Snapshot into functions that
have no need to know about it. Also the bug.Report call
in NewRange has been pushed up into the caller and
logically eliminated in all but one case.
I think we should aim for the invariant that functions that
operate on a single file should accept a *token.File, not
a FileSet; only functions that operate on sets of files
(e.g. type checking, analysis) should use a FileSet.
This is not always possible: some public functions
accept a FileSet only to re-lookup a single file already
known to the caller; if necessary we could provide token.File
variants of these.
This may ultimately allow us to create a new FileSet per
call to the parser, so that Files and FileSets are in
1:1 correspondance and there is no global FileSet.
(It currently grows without bound, on the order of kilobytes
per keystroke.) FileSets containing multiple files,
needed when we interact with the type checker and analysis,
may be temporarily synthesized on demand from a set of Files,
analogous to mmap'ing a few files into a blank address space.
Also:
- replace File.Position(pos).Line by File.Line(pos)
- replace pos == token.NoPos by pos.IsValid()
- avoid fishy token.Pos conversions in link.go
- other minor simplifications
Change-Id: Ia3119e0ac7e193801fbafa81c8f48acfa14e9ae4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/409935
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This speeds up workspace initialization time in DegradeClosed memory
mode from 3m to 1m by avoiding unnecessary recomputation of results.
Change-Id: Ie5df82952d50ab42125defd148136329f0d50a48
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413658
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: David Chase <drchase@google.com>
This on average reduces latency from 12ms to 4ms on internal codebase.
Updates golang/go#45686
Change-Id: Id376fcd97ce375210f2ad8b88e42f6ca283d29d3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413657
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
...so that each client doesn't have to.
Change-Id: I039c493031c5c90c4479741cf6f7572dad480808
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415502
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Before, buildPackageHandle and buildKey were mutually recursive.
Together they performed a sequential recursion over Metadata.Deps,
calling GetFile and parseGoHandle for every file, and then
finally (in postorder) binding a Handle for the type checking step.
This change inlines buildKey to make the recursion more obvious,
performs the recursion over dependencies first, followed by
the reading of Go source files for this package, in parallel.
(The IWL benchmark reports improvement but its variance is so
high I'm not sure I trust it.) Other opportunities for parallelism
are pointed out in new comments.
The Bind operation for typechecking calls dep.check for each
dependency in a separate goroutine. It no longer waits for
each one since it is only prefetching the information
that will be required during import processing, which will
block until the information becomes available.
Before, both reading and parsing appear to occur twice:
once in buildPackageHandle and once in doTypeCheck.
(Perhaps the second was a cache hits, but there's no need
to rely on a cache.)
Now, only file reading (GetFile) occurs in buildPackageHandle,
since that's all that's needed for the packageKey.
And parsing only occurs in doTypeCheck. The source.FileHandles
are plumbed through as parameters.
Also:
- move parseGoHandles to a local function, since it exists only
for buildPackageKey. It no longer parses, it only reads.
- lots of TODO comments for possible optimizations,
and typical measured times of various operations.
- remove obsolete comment re: Bind and finalizers.
Change-Id: Iad049884607b73eaa6701bdf7771f96b042142d5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/411913
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Before, NewMapper accounts for 2.1% of bytes allocated in the
WorkspaceSymbols benchmark. This change causes the newline index
table to be allocated once instead of by appending.
The function now accounts for 0.55%.
Change-Id: I9172dd34ee2be9e7175e311d4a6518f1e6660a5f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415501
Auto-Submit: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
dynamicSymbolMatch is an allocation hotspot (9% of all bytes),
because it allocates a 3-element []string that quickly becomes
garbage. This change passes in an empty slice with spare capacity
allowing the same array to be reused throughout the matchFile loop.
BenchmarkSymbols on k8s shows -72% bytes, -88% allocs, -9% wall time.
Change-Id: Id20c7cd649874a212e4d4c5f1aa095277b044a5b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415500
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
We can use a property that split does a dfs search for the key before
doing an actual work. This allows us to do a low-cost early return if
there is no key to delete.
Change-Id: I6ed8068945f9f2dacc356d72b18afce04ec89a3c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413659
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
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>
This change causes (*snapshot).getWorkspaceDir to create a temporary
directory directly, rather than via the Store/Generation/Handle
mechanism. The work is done at most once per snapshot, and the
directory is deleted in Snapshot.Destroy.
This removes the last remaining use of Handle's cleanup mechanism,
which will be deleted in a follow-up.
Change-Id: I32f09a67846d9b5577cb8849b226427f86443303
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414499
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
In Go 1.18 types.AssignableTo() started reporting that an invalid type
is assignable to any interface. *types.PkgName (i.e. an import at the
top of the file) has an invalid type for its Type(), so we started
thinking all in scope imports were great candidates when the expected
type was an interface.
Fix by wrapping the AssignableTo (and AssertableTo) to explicitly
return false if either operand is invalid.
Updates golang/go#53595
Change-Id: Ie5a84b7f410ff5c73c6b7870e052bafaf3e21e99
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415595
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
This adds a snippet that applies to variables of type chan.
When used, it replaces `channel.range!` with the following snippet:
```
for e := range channel {
|
}
```
Where `|` indicates the location of the cursor.
Change-Id: I8b2f889b22b9f2c292041e5ca5f63c5d0ca98f11
GitHub-Last-Rev: 9cb894be80d0c5243a5e42779c3e96ba79aa66b5
GitHub-Pull-Request: golang/tools#386
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414194
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
This allows reducing critical section of `g.store.mu` as the vast
majority of entries do not rely on generation-based GC.
Change-Id: I985af0b38504ddedb22649290deac91797577b75
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413656
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
This on average reduces latency from 25ms to 12ms on internal codebase.
Updates golang/go#45686
Change-Id: I49c8f09f8e54b7b486d7ff7eb8f4ba9f0d90b278
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413655
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
This function was actually left behind, after making suggested changes
to CL 413683.
Change-Id: I6933e870ded9da5af06724c28839c37d58fb4cdc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414856
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Selectively upgrade only staticcheck, to pick up fixes for support to
generic code.
Change-Id: Ia625c4d46780139aa6e70447eebe1b6d476d4722
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415495
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Include a suggested fix with the diagnostic for unkeyed composite
literals. This suggested fix will add the name of each of the
fields.
For golang/go#53062
Change-Id: I0c33191ff3cf66c95a9a055848274cc2b0c38224
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414674
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Add text edits that a user can accept to make the unkeyed composite
literals keyed from the inlay hints. The text edits modify all of
the unkeyed fields in a composite literal, since a mixture of keyed
and unkeyed fields are not allowed.
Change-Id: I0683fbaa5e22bc004b91c98fc09e495e797826ee
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414855
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Our symbol query only searches Go files, so there is no point to
building (broken) symbol information for non-Go files. Doing so
introduces a lot more symbol handles that need to be tracked and walked.
Change-Id: I96dd62766d079805fcb1d16eb361adfc0c31eea1
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415199
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This change should have been included in
https://go-review.googlesource.com/c/tools/+/415057
but I hastily submitted it without a CI run
thinking "how can a doc only change break something?".
Well now I know. Sorry. :(
Change-Id: Ib0fd25fddd7f9580961b44dcad032d4851684f63
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415058
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
...and don't claim that the most compact field order is optimal.
The exception is relatively obscure, but the fact that it exists
is important because it means it is not safe to apply the code
transformation unconditionally.
Change-Id: I391fbc1872b578d5340dd7c8fded48be30b820e0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415057
Reviewed-by: Robert Findley <rfindley@google.com>
This is an optimization to avoid calculating inlayhints that are
not in the requested range.
Change-Id: I311f297d2998ae7d0db822eac540b1c12cae6e23
Reviewed-on: https://go-review.googlesource.com/c/tools/+/412455
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Add some additional logging to help debug golang/go#53586
For golang/go#53586
Change-Id: I0574fb01be47b265cd6e412855794bc2cb836dff
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414854
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
Add regtests for inlay hints to verify we can turn on
and off different inlay hints.
Change-Id: Id88450c40c048b6c2544d22a0d3eadb57b70a723
Reviewed-on: https://go-review.googlesource.com/c/tools/+/411911
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Suzy Mueller <suzmue@golang.org>
Find needs to invoke the go command to find the package export data.
It cannot rely on GOPATH-based file location heuristics.
This has the nice side effect of automatically compiling the code,
removing the possibility of stale export data.
Ideally Find would use go/packages, but go/packages imports
this package for the export data parser (not for Find itself),
so we have to make do with an explicit go command invocation.
Marked both Find and NewImporter deprecated: using go/packages
will be faster for nearly all use cases, because it can gather info
about multiple packages in a single go command invocation.
They are essentially unused anyway.
Removed the file name print from the example because
the file may be in the cache, in which case it will not be
named "fmt.a".
Change-Id: I7940c90e230b22df9dcbfc8103a69a2d18df3bb0
Reviewed-on: https://go-review.googlesource.com/c/tools/+/310515
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
This change causes the satisfy constraint pass to correctly handle
type parameters. In nearly all cases this means calling coreType(T)
instead of T.Underlying(). This, and the addition of cases for
C[T] and C[X, Y], should make the code robust to generic syntax.
However, it is still not clear what the semantics of constraints
are for the renaming tool. That work is left to a follow-up.
Also, add a test suite that exercises all the basic operators,
using generics in each case.
Fixesgolang/go#52940
Change-Id: Ic1261eb551c99b582c35fadaa148b979532588df
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413690
Reviewed-by: Robert Findley <rfindley@google.com>
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>
Change-Id: I086edda41c57b603afa660afb9396e17ba6c1a36
Reviewed-on: https://go-review.googlesource.com/c/tools/+/414074
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
When setting new metadata for a package ID, we invalidate the
corresponding type-checked packages. Whenever we invalidate a package,
we must also invalidate its reverse transitive closure. Otherwise we may
end up in a scenario where the go/types import graph does not match
gopls' view of the import graph.
Change-Id: I8db6ff3e4a722656e6dde7907e7c0470375c4847
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413683
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>
If gopls believes it has valid metadata for a package, don't set new
metadata. This is consistent with previous behavior that was changed in
CL 340851.
In principle this shouldn't matter, but in practice there are places
where gopls doesn't yet want to invalidate packages, *even though* their
metadata may have changed (such as while editing a go.mod file before
saving). In the future we should eliminate these places, but for now we
should let snapshot.clone control this invalidation.
This also reduces the number of type-checked packages we invalidate on
load.
Change-Id: I0cc9bd4186245bec401332198de0047ff37e7ec7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413681
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>
Previously, gopls would fall back on a gopath resolver when running
goimports from a directory containing go.work (but not go.mod). Fix this
by update the code to recognize that GOWORK also puts goimports into
module mode.
All the work to _support_ go.work had already been done, but the tests
were only passing because they were setting GO111MODULE=on explicitly
(and therefore GOMOD=/dev/null was satisfying the pre-existing check).
Also add a test for the regression in gopls.
Fixesgolang/go#52784
Change-Id: I31df6f71a949a5668e8dc001b3ee25ad26f2f927
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413689
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
To properly format these field names in the vscode config ui
these fields should be camel case.
Change-Id: I3b8b8fb6371172ecb464710f7d91b9fc67e0ed42
Reviewed-on: https://go-review.googlesource.com/c/tools/+/413684
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Suzy Mueller <suzmue@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Robert Findley
Alan Donovan
Ruslan Nigmatullin
Muir Manders
Davide Masserut
Suzy Mueller
Russ Cox
Ben Sarah Golightly
Jamal Carvalho