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

Layout of single-field structs #34

Closed
nikomatsakis opened this issue Oct 11, 2018 · 14 comments · Fixed by #164
Closed

Layout of single-field structs #34

nikomatsakis opened this issue Oct 11, 2018 · 14 comments · Fixed by #164
Labels
A-layout Topic: Related to data structure layout (`#[repr]`) C-open-question Category: An open question that we should revisit S-writeup-needed Status: Ready for a writeup and no one is assigned

Comments

@nikomatsakis
Copy link
Contributor

PR #31 identified this as an area for continued discussion. Specifically, if you have a struct with single field (struct Foo { x: T }), should we guarantee that the memory layout of Foo is identical to the memory layout of T (note that ABI details around function calls may still draw a distinction, which is why #[repr(transparent)] is needed). What about zero-sized types like PhantomData?

@rkruppe wrote:

Long ago I proposed that we might want to guarantee (some subset of) newtype unpacking for repr(Rust) structs. @nikomatsakis carried this over into #11 as discussion point but it received no further discussion. I like to think that means it's uncontroversial 😄 I've also never heard of any reason why one might not want that to be true.

To make a specific proposal, let's restrict it to structs [1] that contain a single field having the same memory layout as the type of the sole field. So struct Foo<T>(T); and struct Foo<T> { x: T } would be laid out like T in memory, though possibly still passed differently in function calls.

[1] The same guarantee for (T,) are already covered by the special case of homogeneous tuples being laid out like arrays that is already in this PR.

@nikomatsakis nikomatsakis added active discussion topic A-layout Topic: Related to data structure layout (`#[repr]`) labels Oct 11, 2018
@nikomatsakis
Copy link
Contributor Author

nikomatsakis commented Oct 11, 2018

I believe we should make this guarantee. I would further like to say that it applies to any struct with a single field of non-zero size (as determined b the other layout rules). I'm not sure if that is something we can always do, though. I'm thinking of complications around unsizing, for example.. do they potentially apply?

(In particular, might that force a larger alignment or some such thing?)

cc @eddyb @gankro

@the8472
Copy link
Member

the8472 commented Oct 11, 2018

This would automatically fall out of #36, correct?

@Gankra
Copy link

Gankra commented Oct 11, 2018

I am a big +1 on making newtyping a layout (but not ABI) no-op

Also I believe we should have a rule that layout operates as if types with (size: 0, align: 1) have been removed. (So PhantomData newtypes are also nops)

@RalfJung
Copy link
Member

If we adopt what @gankro said, then this is indeed a subset of #36: After removing (size: 0, align: 1)-types, a "single-field struct" (according to the definition, having a single non-ZST field where I guess we mean ZST with alignment 1) is certainly homogeneous.

So if we say those are like an array, we'd get that single-field structs are like 1-element arrays. Should we guarantee that those are like their element type (except for ABI)? Seems sensible to me.

@hanna-kruppe
Copy link

As far as I know the C standard does not technically guarantee that an array of one T has exactly the same size as T – padding between elements is ruled out but padding at the end isn't. I don't think any C implementation has ever done so, so we shouldn't worry about this hypothetical when we already break compatibility with actually-extant (and much saner) implementation options like CHAR_BIT > 8. So yes, it's a subset of #36. It would still be good to get consensus here though since #36 seems less of a clear-cut win to me.

@rodrimati1992
Copy link

rodrimati1992 commented Oct 13, 2018

Would the compiler be allowed to change the layout of types containing a #[repr(transparent)] type relative to the type it is wrapping?


#[repr(transparent)]
struct Transparent<T>(T)

fn is_transparent<T>()->bool{
    is_the_same_memory_layout::<Transparent<T>,T>()&&
    is_the_same_memory_layout::<Vec<Transparent<T>>,Vec<T>>()&&
    is_the_same_memory_layout::<Box<Transparent<T>>,Box<T>>()&&
    is_the_same_memory_layout::<Arc<Transparent<T>>,Arc<T>>()&&
    is_the_same_memory_layout::<HashMap<Transparent<T>>,HashMap<T>>()
}



Would this (hypothetical) function return true for all T ?

Edit:
I am aware that this is not safe for all type constructors since some types store a projection of their type parameters.
Eg:

struct ItemType<I>
where
    I:Iterator
{
    item:I::Item,
}

assert_eq!(
    is_the_same_memory_layout::<ItemType<Transparent<T>>,ItemType<T>>(),
    false
);


@Gankra
Copy link

Gankra commented Oct 14, 2018

Is there any particular reason why you think repr(transparent) is significant here? (I would indeed expect your assertion to pass, even if Transparent<T> weren't marked as repr(Transparent)).

@rodrimati1992
Copy link

Is there any particular reason why you think repr(transparent) is significant here? (I would indeed expect your assertion to pass, even if Transparent<T> weren't marked as repr(Transparent)).

That was just in case that it made a difference.

@nikomatsakis
Copy link
Contributor Author

There's been some interesting examples of why the "single-field layout guarantee" would be useful in rust-lang/rust#53514.

@nikomatsakis
Copy link
Contributor Author

@rodrimati1992

Would the compiler be allowed to change the layout of types containing a #[repr(transparent)] type relative to the type it is wrapping?

As @gankro said, I think that this issue basically proposing the answer is "no" -- and further that #[repr(transparent)] is not necessary (that attribute only affects the function call ABI, not memory layout, afaik).

@nikomatsakis
Copy link
Contributor Author

It seems to me there is a general consensus here that this is a good idea. I like the observation that this is a subset of #36, I had not realized that. But I agree with @rkruppe it's worth calling out on its own regardless.

@gnzlbg
Copy link
Contributor

gnzlbg commented Oct 19, 2018

offtopic: do we have a definition of repr somewhere? the attribute controls memory layout, function call abi, etc. I found this a bit confusing.

@nikomatsakis
Copy link
Contributor Author

@gnzlbg There is no central listing of all the knobs, but that seems like a good thing for the language reference. The structs section includes some discussion of struct-specific representation.

@RalfJung RalfJung added the S-writeup-needed Status: Ready for a writeup and no one is assigned label Nov 29, 2018
@RalfJung RalfJung added the C-open-question Category: An open question that we should revisit label Aug 14, 2019
@kupiakos
Copy link

Could there be optimization cases where it'd be desirable to over-align a struct based on its usage? Say you have a Foo([u8; 3]) and a profile-based optimization pass recognizes that giving Foo an 8-byte alignment requirement would improve performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-layout Topic: Related to data structure layout (`#[repr]`) C-open-question Category: An open question that we should revisit S-writeup-needed Status: Ready for a writeup and no one is assigned
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants