x/tools/gopls: Hover: invalid nil entry in types.Defs map #69362
Comments
adonovan
added
NeedsInvestigation
|
I can't explain this one. The only three calls to
It can't be (1) because the Hover logic handled the package clause specially at L186. It can't be (2) because this would cause referencedObject to return a non-nil third result (selectedType), causing an early return. And it can't be (3) because that would cause referencedObject not to return a non-nil Object, Ident pair, leading to an early return. Clearly there is a mistake in my logic. But where? |
|
Agreed. I think the best we'll be able to do here is downgrade the panic into two or more bug reports that refine the panic. This is not the first "can't happen" bug related to go/types... |
|
Change https://go.dev/cl/627015 mentions this issue: |
This CL splits the various fallible checks that may have been responsible for the crash report in golang/go#69362 onto separate lines so that we can confirm whether a nil map entry is indeed the cause. (I am almost certain that it is, but I still can't explain it.) Updates golang/go#69362 Change-Id: I0469e285bda65c21e80a348af04ea0e69f6a31c0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/627015 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Alan Donovan <[email protected]> Reviewed-by: Robert Findley <[email protected]>
This CL splits the various fallible checks that may have been responsible for the crash report in golang/go#69362 onto separate lines so that we can confirm whether a nil map entry is indeed the cause. (I am almost certain that it is, but I still can't explain it.) Updates golang/go#69362 Change-Id: I0469e285bda65c21e80a348af04ea0e69f6a31c0 Reviewed-on: https://go-review.googlesource.com/c/tools/+/627015 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Alan Donovan <[email protected]> Reviewed-by: Robert Findley <[email protected]>
|
This stack
|
|
Well, the new stack is clearly a lie! Bumping this to 0.18.1, when we can hopefully have accurate stacks. |
|
I have a theory: of the three cases listed above, the type-switch one is the most plausible. In order to reach the assertion, we must have gotten the triple (ident != nil, obj != nil, selectedType == nil) from referencedObject. Of the three return statements in referencedObject, this implicates only the one after the call to typeSwitchImplicits, and means that typeSwitchImplicits must have returned (empty slice, nil Type). Looking at the return statements within typeSwitchImplicits, this implicates only the final one (the other two are unreachable due to preconditions of this call). So, we have a type switch with an empty Body.List, and there is missing type information for the operand of the type switch: The question then becomes: what syntax could y be to cause the type checker not to record a type for it? This would likely be a type checker bug. I looked for recent "no type recorded for expr" bugs but didn't find anything promising. Still, I think this implicates the behavior of the type checker on a certain input with at least two compilation errors. |
|
Ah, blaming the "may be nil" comment on the TypeOf call brought me to #69092 (comment), which records that we decided types may always be nil and that we should be defensive. So perhaps the right thing to do here is to return types.Invalid if the TypeOf call fails; then hover will take the early return and not reach the Defs lookup. |
|
@adonovan FWIW I tried to follow your lead, and it may be a dead end: So we can't have implicits with no RHS type. |
adonovan
changed the title
|
The most recent stack shows definitively that the types.Defs map contains a nil value. |
|
I repro'ed! Test+fix incoming. |
|
Change https://go.dev/cl/652015 mentions this issue: |
This stack
zUGLQAwas reported by telemetry:Looks like
Defs[ident]=nilis an actual map entry. This is confirmed by #69362 (comment).crash/crashruntime.gopanic:+69runtime.panicmem:=262runtime.sigpanic:+19golang.org/x/tools/gopls/internal/golang.hover:+170golang.org/x/tools/gopls/internal/golang.Hover:+4golang.org/x/tools/gopls/internal/server.(*server).Hover:+30golang.org/x/tools/gopls/internal/protocol.serverDispatch:+335golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3:+5golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4:+52golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1:+2golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2:+3runtime.goexit:+0Issue created by golang.org/x/tools/gopls/internal/telemetry/cmd/stacks.
Dups: ylB3Iw x2v5eg v95MAw
The text was updated successfully, but these errors were encountered: