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

Jump to bottom

go/types: panic in new logic in recordCommaOkTypes #59371

Closed
findleyr opened this issue Apr 1, 2023 · 4 comments
Closed

go/types: panic in new logic in recordCommaOkTypes #59371

findleyr opened this issue Apr 1, 2023 · 4 comments
Assignees
@griesemer
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone
Go1.21

Comments

@findleyr
Copy link
Contributor

findleyr commented Apr 1, 2023

Just hit this panic in gopls, using go version devel go1.21-a7fd2fab0e Fri Mar 31 16:05:14 2023 +0000 linux/amd64

panic: /home/rfindley/src/go/src/go/types/check.go:490: assertion failed
        panic: /home/rfindley/src/go/src/go/types/check.go:490: assertion failed [recovered]
        panic: /home/rfindley/src/go/src/go/types/check.go:490: assertion failed

goroutine 756640 [running]:
go/types.(*Checker).handleBailout(0xc000875680, 0xc009a51668)
        /home/rfindley/src/go/src/go/types/check.go:299 +0x88
panic({0xd26e00?, 0xc0123e4eb0?})
        /home/rfindley/src/go/src/runtime/panic.go:913 +0x21f
go/types.assert(0x0?)
        /home/rfindley/src/go/src/go/types/errors.go:28 +0x54
go/types.(*Checker).recordCommaOkTypes(0xc000875680, {0x10fa300, 0xc00eec3350}, {0xc0123e4e80, 0x2, 0xc006c60210?})
        /home/rfindley/src/go/src/go/types/check.go:490 +0x158
go/types.(*Checker).initVars(0xc000875680?, {0xc0123e4e50?, 0x2, 0x2}, {0xc005c16500?, 0x1, 0x1}, {0x0, 0x0})
        /home/rfindley/src/go/src/go/types/assignments.go:391 +0x2a5
go/types.(*Checker).shortVarDecl(0xc000875680, {0x10f5a60, 0xc0121d9aa0}, {0xc007b02f20, 0x2, 0xc007b02da0?}, {0xc005c16500?, 0x1, 0x1})
        /home/rfindley/src/go/src/go/types/assignments.go:528 +0x9d9
go/types.(*Checker).stmt(0xc000875680, 0x0, {0x10f9dc0?, 0xc00474a3c0?})
        /home/rfindley/src/go/src/go/types/stmt.go:476 +0xb05
go/types.(*Checker).stmtList(0x130335f?, 0x0, {0xc00eb15a80?, 0x16631c?, 0x16631c?})
        /home/rfindley/src/go/src/go/types/stmt.go:124 +0x78
go/types.(*Checker).funcBody(0xc000875680, 0xc00aff3f20, {0xe95c6a?, 0xc00007eaa0?}, 0xc0076d9d40, 0xc00eec3410, {0x0, 0x0})
        /home/rfindley/src/go/src/go/types/stmt.go:44 +0x345
go/types.(*Checker).exprInternal.func1()
        /home/rfindley/src/go/src/go/types/expr.go:1362 +0x3b
go/types.(*Checker).processDelayed(0xc000875680, 0xd0b)
        /home/rfindley/src/go/src/go/types/check.go:387 +0x15c
panic({0xd26e00?, 0xc0123e4ca0?})
        /home/rfindley/src/go/src/runtime/panic.go:919 +0x270
go/types.assert(0x0?)
        /home/rfindley/src/go/src/go/types/errors.go:28 +0x54
go/types.(*Checker).recordCommaOkTypes(0xc000875680, {0x10fa300, 0xc00eec3350}, {0xc0123e4c60, 0x2, 0xc0066a27ac?})
        /home/rfindley/src/go/src/go/types/check.go:490 +0x158
go/types.(*Checker).initVars(0xc000875680?, {0xc0123e4c30?, 0x2, 0x2}, {0xc005c16500?, 0x1, 0x1}, {0x0, 0x0})
        /home/rfindley/src/go/src/go/types/assignments.go:391 +0x2a5
go/types.(*Checker).shortVarDecl(0xc000875680, {0x10f5a60, 0xc0121d9a40}, {0xc007b02f20, 0x2, 0xc007b02da0?}, {0xc005c16500?, 0x1, 0x1})
        /home/rfindley/src/go/src/go/types/assignments.go:528 +0x9d9
go/types.(*Checker).stmt(0xc000875680, 0x0, {0x10f9dc0?, 0xc00474a3c0?})
        /home/rfindley/src/go/src/go/types/stmt.go:476 +0xb05
go/types.(*Checker).stmtList(0x6ae32b?, 0x0, {0xc00eb15a80?, 0x64d42c?, 0xc0076d9b00?})
        /home/rfindley/src/go/src/go/types/stmt.go:124 +0x78
go/types.(*Checker).funcBody(0xc000875680, 0xc00aff3f20, {0xe95c6a?, 0xc00007eaa0?}, 0xc0076d9d40, 0xc00eec3410, {0x0, 0x0})
        /home/rfindley/src/go/src/go/types/stmt.go:44 +0x345
go/types.(*Checker).exprInternal.func1()
        /home/rfindley/src/go/src/go/types/expr.go:1362 +0x3b
go/types.(*Checker).processDelayed(0xc000875680, 0xd0b)
        /home/rfindley/src/go/src/go/types/check.go:387 +0x15c
go/types.(*Checker).shortVarDecl(0xc000875680, {0x10f5a60, 0xc0121d99e0}, {0xc005c16320, 0x1, 0xc006f57500?}, {0xc005c16630?, 0x1, 0x1})
        /home/rfindley/src/go/src/go/types/assignments.go:531 +0x9ee
go/types.(*Checker).stmt(0xc000875680, 0x0, {0x10f9dc0?, 0xc00474a6c0?})
        /home/rfindley/src/go/src/go/types/stmt.go:476 +0xb05
go/types.(*Checker).stmtList(0xc009a51468?, 0x0, {0xc007a96c00?, 0x0?, 0x6ae32b?})
        /home/rfindley/src/go/src/go/types/stmt.go:124 +0x78
go/types.(*Checker).funcBody(0xc000875680, 0xc00aff3f20, {0xc0073a82e8?, 0xc007f706c0?}, 0xc00da22d80, 0xc00eec3680, {0x0, 0x0})
        /home/rfindley/src/go/src/go/types/stmt.go:44 +0x345
go/types.(*Checker).funcDecl.func1()
        /home/rfindley/src/go/src/go/types/decl.go:831 +0x39
go/types.(*Checker).processDelayed(0xc000875680, 0x0)
        /home/rfindley/src/go/src/go/types/check.go:387 +0x15c
go/types.(*Checker).checkFiles(0xc000875680, {0xc0069b7300, 0x1c, 0x20})
        /home/rfindley/src/go/src/go/types/check.go:332 +0x16e
go/types.(*Checker).Files(...)

Given that this logic changed in https://go.dev/cl/478218 (and surrounding CLs), it seems likely that this is a new panic in go/types. (though I will caveat that a lot of logic in gopls has changed recently with respect to type checking).

Unfortunately, I was not able to reproduce by replaying my editing session (likely due batching of keystrokes in the replay), so we'll have to stare at this. For starters, we should separate those assertions onto separate lines.

CC @griesemer
@findleyr findleyr added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Apr 1, 2023
@findleyr findleyr added this to the Go1.21 milestone Apr 1, 2023
@griesemer griesemer self-assigned this Apr 12, 2023
@findleyr
Copy link
Contributor Author

Minimal repro: just ensure that 'ok' has type Typ[Invalid], and we won't get a typed RHS operand.

func _() {
  var ok undef
  var m map[int]int
  _, ok := m[0]
}

@griesemer
Copy link
Contributor

Even simpler repro:

func _() {
	var m map[int]int
	_, ok = m[0]
}

@griesemer
Copy link
Contributor

griesemer commented Apr 18, 2023 •
edited

Analysis: This code was broken with CL 478218. Before that CL, Checker.assignVar used to return the assigned type, or nil, in case of failure. Checker.recordCommaOkTypes used to take two types (not two operands), and if one of those types was nil, it would simply not record. With the changes in CL 478218, we lost that (nil) signal.

@gopherbot
Copy link

Change https://go.dev/cl/486135 mentions this issue: go/types, types2: don't panic for invalid assignments of comma-ok expressions

@adonovan adonovan mentioned this issue Apr 26, 2023
Closed
@golang golang locked and limited conversation to collaborators Apr 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@griesemer griesemer

Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
3 participants
@griesemer @gopherbot @findleyr