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: NewMethodSet doesn't terminate for recursively embedded generics [1.18 backport] #52804

Closed
gopherbot opened this issue May 9, 2022 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link
Contributor

@findleyr requested issue #52715 to be considered for backport to the next 1.18 minor release.

@gopherbot please backport to 1.18. This is an infinite recursion via valid usage of the go/types API.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label May 9, 2022
@gopherbot gopherbot added this to the Go1.18.2 milestone May 9, 2022
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/405117 mentions this issue: [release-branch.go1.18] go/types, types2: use a type lookup by identity in method lookup

@heschi
Copy link
Contributor

heschi commented May 9, 2022

Unavoidable bug in functionality new in 1.18 seems like a reasonable cherrypick to me. Approved.

@heschi heschi added the CherryPickApproved Used during the release process for point releases label May 9, 2022
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label May 9, 2022
@gopherbot
Copy link
Contributor Author

Closed by merging 020fc47 to release-branch.go1.18.

gopherbot pushed a commit that referenced this issue May 10, 2022
…ty in method lookup

Named type identity is no longer canonical. For correctness, named types
need to be compared with types.Identical. Our method set algorithm was
not doing this: it was using a map to de-duplicate named types, relying
on their pointer identity. As a result it was possible to get incorrect
results or even infinite recursion, as encountered in #52715.

To fix this, look up types by identity in NewMethodSet and
LookupFieldOrMethod. This does a linear search among types with equal
origin. Alternatively we could use a *Context to do a hash lookup, but
in practice we will be considering a small number of types, and so
performance is not a concern and a linear lookup is simpler. This also
means we don't have to rely on our type hash being perfect, which we
don't depend on elsewhere.

Also add more tests for NewMethodSet and LookupFieldOrMethod involving
generics.

Fixes #52804

Change-Id: I04dfeff54347bc3544d95a30224c640ef448e9b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/404099
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
(cherry picked from commit f088f49)
Reviewed-on: https://go-review.googlesource.com/c/go/+/405117
Reviewed-by: Alan Donovan <adonovan@google.com>
@golang golang locked and limited conversation to collaborators May 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

2 participants