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.

Sign up for GitHub

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/internal/gcimporter: importer needs to skip types with implicit type parameters #55110

Closed
mdempsky opened this issue Sep 16, 2022 · 3 comments
Closed

go/internal/gcimporter: importer needs to skip types with implicit type parameters #55110

mdempsky opened this issue Sep 16, 2022 · 3 comments
Assignees
@mdempsky
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.20

Comments

@mdempsky
Copy link
Member

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

$ go vet .
# m
panic: unexpected object with 1 implicit type parameter(s) [recovered]
	panic: unexpected object with 1 implicit type parameter(s)

The issue is the local type X is effectively promoted to a global type, but then given additional implicit type parameters. This type definition is never actually needed by the go/internal/gcimporter (because it doesn't read function bodies). However, the go/types API doesn't support lazy object importing, so go/internal/gcimporter eagerly imports everything.

We just need to skip over local type definitions. These can be distinguished because they have a ·N vargen suffix in their name.
@mdempsky mdempsky added NeedsFix The path to resolution is known, but the work has not been done. release-blocker labels Sep 16, 2022
@mdempsky mdempsky added this to the Go1.20 milestone Sep 16, 2022
@mdempsky mdempsky self-assigned this Sep 16, 2022
@gopherbot
Copy link

Change https://go.dev/cl/431495 mentions this issue: go/internal/gcimporter: fix ureader.go handling of local defined types

@timothy-king
Copy link
Contributor

I am not really sure what "skip over local type definitions" means.

FWIW we also need to handle X[int;string] becoming global by flowing into an interface: https://go.dev/play/p/N3pcJCgqU7F?v=gotip .

@mdempsky
Copy link
Member Author

mdempsky commented Sep 19, 2022 •
edited

I am not really sure what "skip over local type definitions" means.

go/types.Importer implementations are responsible for creating the go/types.Package and populating its top-level Scope.

As an implementation detail, cmd/compile handles local type definitions (e.g., func F() any { type T int; return T(0) }) by renaming the variable and hoisting to global scope (effectively rewriting into type T·1 int; func F() any { return T·1(0) }).

However, these types shouldn't be added to Package.Scope, because they weren't actually declared there.

FWIW we also need to handle X[int;string] becoming global by flowing into an interface: https://go.dev/play/p/N3pcJCgqU7F?v=gotip .

Ack, but I think that detail is only visible to users that parse sub.go and type check it with go/types? If package sub's export data is imported instead, then the implementation details of F are opaque.

@golang golang locked and limited conversation to collaborators Sep 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@mdempsky mdempsky

Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
3 participants
@mdempsky @timothy-king @gopherbot