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

Allow limited transmuting between types involving type parameters #86281

Closed

Conversation

illicitonion
Copy link

Before this change, functions could never perform transmutes between types which involve type parameters.

This opens up slightly less limited analysis whereby certain types involving Sized type parameters can be considered relative to each other, even if the exact size of that type parameter isn't yet known because monomorphization hasn't happened yet. Specifically it allows:

  1. repr(transparent) types to be considered to have the same size as the Sized type they wrap.
  2. Arrays of type parameters (or repr(transaprent) wrappers thereof) to be considered to have the same size as other arrays of the same type parameters (or repr(transparent) wrappers thereof), as long as their element count is the same.

It only supports very limited comparisons, but it opens up more safe and legal transmutes than were previously allowed.

In particular, it allows for transmutes like:

fn make_array<T, const N: usize>() -> [T; N] {
    let mut values: [MaybeUninit<T>; N] = unsafe { MaybeUninit::<[MaybeUninit<T>; N]>::uninit().assume_init() };
    // [... Init values]
    std::mem::transmute(values)
}

which are currently not allowed because [T; N] isn't considered to have the same size as [MaybeUninit<T>; N]

Fixes #61956.

This allows code like:
```rust
fn my_array_transmute<T>(t1: [T; 10]) -> [T; 10] {
    unsafe {
        std::mem::transmute(t1)
    }
}
```
This allows code like:
```rust
struct T1<T>([T; 10]);

fn my_transmute<T>(t1: T1<T>) -> [T; 10] {
    unsafe {
        std::mem::transmute(t1)
    }
}
```
@rust-highfive
Copy link
Collaborator

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2021
@bstrie
Copy link
Contributor

bstrie commented Jun 29, 2021

  1. repr(transparent) types to be considered to have the same size as the Sized type they wrap.
  2. Arrays of type parameters (or repr(transaprent) wrappers thereof) to be considered to have the same size as other arrays of the same type parameters (or repr(transparent) wrappers thereof), as long as their element count is the same.

@RalfJung , you seemed to be in favor of allowing such transmutes in #61956 , are you still of the opinion that it should be sound to expand transmutation with the rules described above?

@bstrie bstrie added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jun 29, 2021
@RalfJung
Copy link
Member

Seems reasonable to me but I'd recommend t-lang FCP for changes like this.

@Lokathor
Copy link
Contributor

Note: Strictly speaking transmute_copy can already also do this.

@varkor
Copy link
Member

varkor commented Jun 30, 2021

Reassigning to r? @RalfJung, who is more familiar with the nuances here.

@rust-highfive rust-highfive assigned RalfJung and unassigned varkor Jun 30, 2021
@RalfJung
Copy link
Member

RalfJung commented Jul 1, 2021

@varkor I might be familiar with the nuances of at the bigh level here, but I am completely unfamiliar with the nuances of the code that is being changed here.^^ I don't think I can review that, I am afraid.

But I'll nominate this for lang team discussion. Cc @rust-lang/lang

/// * one's size is a const generic parameter
/// * the other's is an const value which happens to be equal to that const generic parameter
/// will not be considered equal.
UnresolvedConstant {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like RepeatedTy or so would make more sense? The current name doesn't really convey a lot of meaning.

@scottmcm
Copy link
Member

scottmcm commented Jul 6, 2021

Personally I'm torn here. On one hand, I think this would be quite useful. On the other hand, I worry that this might make it harder to make transmute non-magic.

Maybe someone from the const WG has an idea about whether this would make it harder to encode the "sizes are the same" check as a "real" type system predicate instead of intrinsic magic?

For the specific example in the OP, one way forward would be to just stabilize https://doc.rust-lang.org/nightly/std/mem/union.MaybeUninit.html#method.array_assume_init (or something like it).

@RalfJung
Copy link
Member

From the implementation side, maybe @nikic or @nagisa could review this?

@nagisa
Copy link
Member

nagisa commented Jul 10, 2021

r? @nagisa yeah, I can review the implementation but before I spend time on this please make a decision at T-lang level that this is a desirable change in the first place.

@rust-highfive rust-highfive assigned nagisa and unassigned RalfJung Jul 10, 2021
@nagisa nagisa added the S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). label Jul 10, 2021
@joshtriplett
Copy link
Member

Dropping I-nominated; @scottmcm to write up a consensus from lang-team discussion.

@crlf0710 crlf0710 added S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). and removed S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). labels Aug 14, 2021
@scottmcm
Copy link
Member

We discussed this in the 07-20 lang team meeting, and the consensus was to not move forward with this.

The core of the concern here is that it felt like it was overlapping with the safe transmute work. Not a 100% overlap, of course, but that there are primitive features which that project will be adding that could be leveraged here in a way that's less ad-hoc. And this change brings up a bunch of the same visibility/coherence questions that that project is tackling, like how changing a private field from u32 to NonZeroU32 could become a compilation-breaking change if people are transmuting values of the type (as opposed to a "it was always UB, just possibly not caught previously" runtime problem).

For now we'd like to see #61956 fixed without encouraging transmute, probably by a method like in #80908.

(Spitballing without my lang hat on: safe transmute will know that [T; N] -> [MaybeUninit<T>; N] is fine, so maybe there's space for a still-not-safe but less-yolo transmute for the other direction.)

@scottmcm scottmcm closed this Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Const generics: Generic array transmutes do not work
10 participants