Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: add logging to detect instances of "cannot use A (value of type B) as B value" #38403

Closed
myitcv opened this issue Apr 13, 2020 · 22 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@myitcv
Copy link
Member

myitcv commented Apr 13, 2020

What version of Go are you using (go version)?

$ go version
go version devel +801cd7c84d Thu Apr 2 09:00:44 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200408132156-9ee5ef7a2c0d => github.com/myitcvforks/tools v0.0.0-20200408225201-7e808beafd9f
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20200408132156-9ee5ef7a2c0d => github.com/myitcvforks/tools/gopls v0.0.0-20200408225201-7e808beafd9f

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/.vim/plugged/govim/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build285741329=/tmp/go-build -gno-record-gcc-switches"

What did you do?

This is a follow up to #38267 (comment).

I'm occasionally seeing instances of "cannot use A (value of type B) as B value" with -remote.

The most recent instance was after I change branches, (with all editor instances closed) at which point I saw:

...cannot use X (value of type context.Context) as context.Context...

What did you expect to see?

No such errors

What did you see instead?

The above errors.

I don't have any repro steps as yet, nor any details that are worth uploading to this issue. So instead opening this as a placeholder in case others start seeing issues and can provide more context.


cc @stamblerre @findleyr

FYI @leitzler

@myitcv myitcv added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. gopls Issues related to the Go language server, gopls. labels Apr 13, 2020
@gopherbot gopherbot added this to the Unreleased milestone Apr 13, 2020
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Apr 13, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Apr 13, 2020
@stamblerre
Copy link
Contributor

stamblerre commented Apr 13, 2020

Thanks for the report. I could see this happening with -remote since we have multiple sessions sharing the cache - please let me know if you see this without -remote.

It's a cache invalidation issue where we end up type-checking a package with 2 different versions of one dependency (so somehow there are 2 "contexts" in this case). This is more likely to happen when you have a workspace structured like this:

package a imports package b and package c
package b imports package c

Please do add more detail and a log if you see it again. The sequence of file changes will help us understand what steps are needed to repro.

@myitcv
Copy link
Member Author

myitcv commented Apr 14, 2020

please let me know if you see this without -remote.

As I mention above, I don't have any repro steps as yet. When I do I'll try with and without remote.

@myitcv
Copy link
Member Author

myitcv commented Apr 17, 2020

Quick update: I'm seeing this semi-regularly now, although still not able to 100% repro.

The setup is roughly this:

  • I'm working on a stack of changes in a branch, let's name them 1-10
  • I start work (i.e. no gopls currently running) via git rebase -i to edit 8
  • I do some work, add the changes and git rebase --continue
  • I get a conflict during the rebase of 10, fix those changes, add and git rebase --continue
  • I then go back to doing some work to edit 8

Now I'm in the bad state.

@findleyr
Copy link
Contributor

Thanks very much for the details. Couple questions:

When you say 'no gopls currently running', you mean no session (i.e. forwarder), right? The daemon is running through all this?

How do you get out of the bad state? Do you restart your session? Do you have to kill the daemon?

@findleyr findleyr self-assigned this Apr 17, 2020
@myitcv
Copy link
Member Author

myitcv commented Apr 17, 2020

When you say 'no gopls currently running', you mean no session (i.e. forwarder), right?

No, I mean no remote. This was simply to indicate I'm starting from cold/fresh.

How do you get out of the bad state?

At the moment, quit all editor sessions and then killall gopls 😄

@atombender
Copy link

@stamblerre This smells like the same bug? I was happily editing some tests when, suddenly, another file (which is fine and wasn't being edited) in the same test package started showing as erroring. The error was on an import line:

	gogo_proto "github.com/gogo/protobuf/proto"
	^
	"github.com/gogo/protobuf/proto" imported but not used as gogo_proto

and also on this later line in the file:

func assertMutationFailure(h *harness.Harness, namespace string, muts []*proto.Mutation, errorType gogo_proto.Message) {
^
Message not declared by package proto

Log: gopls.log.gz. (Note: Had to scrub a lot of didOpen log messages as it logs the whole file contents, and this is a private repo.)

@atombender
Copy link

I can actually reproduce this error consistently, even across restarts. The file keeps getting the same error when editing other files. Let me know if there's anything I can do to get more diagnostics.

@findleyr
Copy link
Contributor

findleyr commented May 4, 2020

@atombender, thank you for the report, and for attaching your logs. I'm not positive that your error is the same bug. How are you invoking gopls - which editor are you using and are which flags are you passing?

@stamblerre and I discussed, and think this is probably unrelated to using -remote based on reports of this bug surfacing without that flag. Unassigning for triage, though this is still a high priority bug.

@findleyr findleyr removed their assignment May 4, 2020
@gopherbot
Copy link

Change https://golang.org/cl/232197 mentions this issue: internal/lsp/cache: add logic to detect golang/go#38403 and panic early

@atombender
Copy link

How are you invoking gopls - which editor are you using and are which flags are you passing?

VSCode. I just have -rpc.trace.

@stamblerre
Copy link
Contributor

stamblerre commented May 5, 2020

I have a reasonable guess about what the cause of the bug is, so hopefully the CL above will be able to address it.

@atombender: I do not think you're seeing the same issue, but unfortunately, I cannot make good sense of your log. Based on the changes you were making, it doesn't seem like gopls would've been recomputing the imports for your package. If you're still able to reproduce it - are there specific repro steps? What happens when you go into the test file - do the errors remain, or do they disappear? If you're able to produce a small repro case, that would be ideal for investigation.

@atombender
Copy link

@stamblerre Unfortunately, that was 5 days ago, and I'm not able to reproduce it right now, even if I reset my work back to that point in time. I will let you know if it happens again.

@myitcv
Copy link
Member Author

myitcv commented May 5, 2020

@stamblerre I have a stack of CLs to rebase, but at that point I will try out this debugging CL and report back.

@atombender
Copy link

Had one of these happen again. As before, I am using GOFLAGS=-tags=integration, and the files here are not excluded.

Screen Shot 2020-05-07 at 13 13 03

I wasn't editing that file when the error popped up. It showed up as I was editing a different file, I believe harness.go in the same package.

gopls.log.gz

@atombender
Copy link

I have been able to isolate it in a repo that I'll share in a moment.

@atombender
Copy link

@gopherbot
Copy link

Change https://golang.org/cl/232817 mentions this issue: internal/lsp: fix caching issue with duplicate handles

@stamblerre
Copy link
Contributor

Thank you very much for the repro. I think this is a different issue so I filed it separately: #38927.

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.1 May 13, 2020
@stamblerre stamblerre changed the title x/tools/gopls: instances of "cannot use A (value of type B) as B value" using -remote x/tools/gopls: instances of "cannot use A (value of type B) as B value" Jun 10, 2020
@stamblerre
Copy link
Contributor

@niaow
Copy link

niaow commented Jun 26, 2020

Currently I have this occurring in 2 packages.

One of the instances is a direct assignment of a concrete stdlib type on these 2 lines:
https://github.com/pilosa/pilosa/blob/9dc1775b93464f78acc8573cfad2f405b1175fb5/server/tlsconfig.go#L142-L143

cannot use certPool (variable of type *x509.CertPool) as *x509.CertPool value in assignment
cannot use certPool (variable of type *x509.CertPool) as *x509.CertPool value in assignment

In another (not public) package, I have an issue with *grpc.ClientConn being passed into a function accepting grpc.ClientConnInterface (which appears to be caused by a type check error on an interface method argument):

cannot use conn (variable of type *grpc.ClientConn) as grpc.ClientConnInterface value in argument to pb.NewPilosaClient: wrong type for method Invoke

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v1.0.0 Jul 22, 2020
@stamblerre
Copy link
Contributor

This issue has popped up now and again, but it seems to be less prevalent these days.
For v1.0, I would like to resurrect and merge https://go-review.googlesource.com/c/tools/+/232197.

@stamblerre stamblerre changed the title x/tools/gopls: instances of "cannot use A (value of type B) as B value" x/tools/gopls: add logging to detect instances of "cannot use A (value of type B) as B value" Sep 25, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default Nov 10, 2020
@stamblerre stamblerre moved this from Needs Triage to Non-critical in vscode-go: gopls by default Nov 10, 2020
@stamblerre
Copy link
Contributor

@heschik's rewrite of the cache has likely resolved this issue, especially since no one has seen it in months. Closing.

vscode-go: gopls by default automation moved this from Non-critical to Done Nov 12, 2020
@stamblerre stamblerre removed this from the gopls/vscode-go milestone Nov 12, 2020
@golang golang locked and limited conversation to collaborators Nov 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants