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

cmd/compile: disallow not-in-heap types to be used as type arguments #54765

Closed
mdempsky opened this issue Aug 30, 2022 · 5 comments
Closed

cmd/compile: disallow not-in-heap types to be used as type arguments #54765

mdempsky opened this issue Aug 30, 2022 · 5 comments
Assignees
@mdempsky
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Go1.20

Comments

@mdempsky
Copy link
Member

I think we shouldn't allow not-in-heap types to be used as type arguments.

func f[T any]() { println(new(T)) }

is a valid generic function declaration, but instantiating it as f[nih] will (or should) fail. (We're inconsistent about whether we catch this today: https://go.dev/play/p/bRMtJouiaR5)

In theory, we could still allow instantiating f[nih] if f doesn't contain new(T), or allocate T on the stack, or any of the other problematic use cases; but this violates the generics principle that the type parameter constraints fully constrain what type arguments are valid.

I think the most likely case where this could come up today is with cgo and incomplete types; a user might want to write atomic.Pointer[C.some_incomplete_type], which would now fail to compile if we reject not-in-heap types as type arguments. However, internally atomic.Pointer is using an unsafe.Pointer to store the pointer, which may not be appropriate for a *C.some_incomplete_type value, so arguably this is already potentially broken.

/cc @ianlancetaylor @randall77
@mdempsky mdempsky added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Aug 30, 2022
@mdempsky mdempsky added this to the Go1.20 milestone Aug 30, 2022
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Aug 30, 2022
@randall77
Copy link
Contributor

I agree, we should probably disallow this. I don't see how it would be terribly useful, and I don't see how we could implement it safely for almost any uses of T.

We should allow instantiating with a pointer-to-nih type, though. We need to be at least a bit careful there, as a pointer-to-nih must be treated as a uintptr, not a pointer. Which means when shapifying we can't unify pointer-to-nih types with *byte (like we do for most pointer types). I think we already have that special case?

@mdempsky
Copy link
Member Author

We should allow instantiating with a pointer-to-nih type, though.

Ack, agreed. Pointer-to-nih types are not themselves nih, so they're not subject to the proposed restriction here.

I think we already have that special case?

Yeah, both nounified and unified have code to exempt pointer-to-nih from the current pointer shaping. I haven't spent too much time on it yet, but I'm pretty sure it's right.

@mknyszek
Copy link
Contributor

@mdempsky assigning to you for now, but feel free to unassign it if you don't plan to work on it. Sounds like we should probably have some thing here for 1.20 (does this need a backport?), but I'm not sure.

@gopherbot
Copy link

Change https://go.dev/cl/427235 mentions this issue: cmd/compile: reject not-in-heap types as type arguments

@janpfeifer
Copy link

janpfeifer commented Mar 4, 2023 •
edited

Just for future record, this created a nuanced incompatibility (code that worked in go 1.19 no longer works in 1.20), when using a C forward declared type as a type parameter, that is never instantiated (only pointers to it are used).

See example of breaking code in golang-nuts thread.

@ianlancetaylor suggests it's too subtle (hard?) to ensure that the type is only used in acceptable ways -- that is, as pointers I'm assuming. But there is also no way to impose that a type parameter to be of pointer type (which would be a way around).

Maybe in the future someone will think of something.

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

@mdempsky mdempsky

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Go1.20
Development

No branches or pull requests
5 participants
@mdempsky @mknyszek @randall77 @janpfeifer @gopherbot