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

go/types, types2: nil underlying for selectors on the current type decl #51509

Closed
findleyr opened this issue Mar 6, 2022 · 5 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented Mar 6, 2022

The following code crashes the type checker with a "nil underlying" panic, because we try to check the underlying when selection fails:

type X X.t

https://go.dev/play/p/H3Dv94uxQL9?v=gotip

Marking as a release blocker as this is a crash.

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 Mar 6, 2022
@findleyr findleyr added this to the Go1.18 milestone Mar 6, 2022
@griesemer griesemer self-assigned this Mar 6, 2022
@griesemer
Copy link
Contributor

It's notoriously difficult to prevent crashes in a compiler. We should fix this of course, but I think we'd be ok even if this made it into the release as this is pretty pathological.

@gopherbot
Copy link

Change https://go.dev/cl/390314 mentions this issue: types2: don't crash in selectors referring to the type being declared

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 7, 2022
@ianlancetaylor
Copy link
Contributor

Yes: a compiler crash should be fixed, of course, but in general a compiler crash on invalid code is not a release blocker. It's just a really really bad error message.

@findleyr
Copy link
Contributor Author

findleyr commented Mar 7, 2022

Agree that it may not be a release blocker, though it also crashes gopls in which case it is more than an error message.

One argument for the severity / commonality of this pattern is that it was introduced in https://go.dev/cl/378175, i.e. post beta, and seems to have been encountered pretty quickly inside of Google.

@gopherbot
Copy link

Change https://go.dev/cl/390423 mentions this issue: [release-branch.go1.18] go/types, types2: don't crash in selectors referring to the type being declared

gopherbot pushed a commit that referenced this issue Mar 8, 2022
…ferring to the type being declared

In Checker.typInternal, the SelectorExpr case was the only case that
didn't either set or pass along the incoming def *Named type.

Handle this by passing it along to Checker.selector and report a
cycle if one is detected.

Fixes #51509.

Change-Id: I6c2d46835f225aeb4cb25fe0ae55f6180cef038b
Reviewed-on: https://go-review.googlesource.com/c/go/+/390314
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
(cherry picked from commit 114d5de)
Reviewed-on: https://go-review.googlesource.com/c/go/+/390423
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants