repr(ordered_fields)

[RustyYato]

View all comments

Add repr(ordered_fields) and provide a migration path to switch users from repr(C) to repr(ordered_fields), then change the meaning of repr(C) in the next edition.

This RFC is meant to be an MVP, and any extensions (for example, adding more reprs) are not in scope. This is done to make it as easy as possible to accept this RFC and make progress on the issue of repr(C) serving two opposing roles.

Rendered

To avoid endless bikeshedding, I'll make a poll if this RFC is accepted with all the potential names for the new repr. If you have a new name, I'll add it to the list of names in the unresolved questions section, and will include it in the poll.

[clarfonthey]

Not to add too many extra colours to the list, but repr(consistent) feels like a good name for this, since the purpose is to provide a consistent layout that does not depend on generics, compiler version, or target. The important thing is just that it's consistent, not that it matches what C does.

(Note: those three things should cover every case I've seen that uses repr(C) that should use repr(ordered_fields), but please feel free to correct me if I missed anything.)

Whereas repr(C) is explicitly, match what C does.

Also, while it may be more technical than most users need to understand, it would be helpful if the RFC reiterated the current issues with repr(C) that we want to fix, and potential future differences between repr(C) and repr(ordered_fields) that could pop up. I've read some of them but am not 100% sure of the details, and it would be nice to keep as part of the RFC.

[Lokathor]

Just as a small point of style the Guide Level Explanation is usually "what would be written in the rust tutorial book", and the Reference Level Explanation is "what could be written into the Rust Reference". This isn't a strict requirement, but personally I'd like to see the Reference Level part written out. Using the present tense, as if the RFC was accepted and implemented.

[programmerjake]

Suggested change

	`repr(ordered_fields)` is a new representation that can be applied to `struct`, `enum`, and `union` to give them a consistent, cross-platform, and predictable in memory layout.
	`repr(ordered_fields)` is a new representation that can be applied to `struct`, `enum`, and `union` to give them a consistent, cross-platform, and predictable in-memory layout.

"cross-platform" -- the layout will differ when there are different layouts for struct members' types, in particular primitive types can have different alignments which changes the amount of padding.

e.g., #[repr(ordered_fields)] struct S(u8, f64); doesn't have the same layout on x86_64 and i686

[RustyYato]

Good point, this will need to be documented as a hazard in the ordered_fields docs. However, the repr itself will be cross-platform. For example, #[repr(ordered_fields)] struct Cross([u8; 3], SomeEnum); will be truly cross-platform (given that SomeEnum is!).

[moonheart08]

Not to add too many extra colours to the list, but repr(consistent) feels like a good name for this, since the purpose is to provide a consistent layout that does not depend on generics, compiler version, or target. The important thing is just that it's consistent, not that it matches what C does.

(Note: those three things should cover every case I've seen that uses repr(C) that should use repr(ordered_fields), but please feel free to correct me if I missed anything.)

Whereas repr(C) is explicitly, match what C does.

Also, while it may be more technical than most users need to understand, it would be helpful if the RFC reiterated the current issues with repr(C) that we want to fix, and potential future differences between repr(C) and repr(ordered_fields) that could pop up. I've read some of them but am not 100% sure of the details, and it would be nice to keep as part of the RFC.

Just voicing support for repr(consistent) as naming.
Aside from the above, it more clearly hones in on the primary promises of the RFC, which is not just ordering but also exact type representation for things like enums. Field ordering is not the only thing it promises.

[joshtriplett]

Nominating this so that we can do a preliminary vibe-check on it in a lang triage meeting.

[scottmcm]

Big fan of doing this split, especially for structs. (It's less obvious what choices to make for other things, IMHO, but at least for structs this is something I've wanted for ages, so that for example Layout::extend can talk about it instead of C.)

Pondering the bikeshed: declaration_order or something could also be used to directly say what you're getting.

(This could be contrasted with other potential reprs that I wouldn't expect this RFC to add, but could consider as future work, like a deterministic_by_size_and_alignment where some restricted set of optimizations are allowed but you can be sure that usize and NonNull<String> can be mixed between different types while still getting the "same" field offsets, for example.)

[RustyYato]

I think this is also useful for unions, so we don't need to rely on repr(C) to ensure that all fields of a union are at offset 0.

This could be contrasted with other potential reprs that I wouldn't expect this RFC to add...

This also works as an argument against names like repr(consistent), since there are multiple consistent and useful repr, making it not descriptive enough.

[scottmcm]

I do think that declaration_order or ordered_fields is a bit weird on a union, because of course they're not really in any "order".

It makes me ponder whether we should just have repr(offset_zero) for unions to be explicit about it, or something.

(Which makes me think of other things like addressing rust-lang/unsafe-code-guidelines#494 by having a different constructs for "bag of maybeuninit stuff that overlap" vs "distinct options with an active-variant rule for enum-but-without-stored-discriminant". But those are definitely not this RFC.)

[RustyYato]

I don't mind spelling it as repr(offset_zero) for unions if that helps get this RFC accepted 😄. However, I have a sneaking suspicion that this isn't the contentious part of this RFC.
I know the name isn't optimal (intentionally). This can be hashed out after the RFC is accepted (or even give a different name for all of struct, union, and enum).
The most important bit for me is just that we do the split (for all of struct, union, and enum, to be consistent).

[RustyYato]

I've updated how enums's tags are specified, now they just defer to whatever repr(C)'s tag type is. This is done to reduce the friction of switching from repr(C) to repr(ordered_fields). To ensure that all uses of repr(ordered_fields) can be cross-platform, I've adding a lint to ensure that the user also adds an explicit repr for repr(uN)/repr(iN).

[Mark-Simulacrum]

I think this is too noisy. Most code out there using repr(C) is probably fine - IIUC, if you're not targeting Windows or AIX, maybe definitely fine? - and having a bunch of allow(...) across a bunch of projects seems unfortunate.

Maybe we can either (a) only enable the lint for migration, i.e., the next edition's cargo fix would add allows for you or (b) we find some new name... C2 for the existing repr(C) usage to avoid allows. But (b) also seems too noisy to me.

[clarfonthey]

Maybe it could just be an optional edition compatibility lint, so if someone enables e.g. rust_20xx_compatibility it shows up but otherwise not.

[RustyYato]

(a) only enable the lint for migration

That was the intention, hence the name edition_2024_repr_c. I'll make this more clear, that this is intended to be a migration lint.

Rustfix would update to #[repr(ordered_fields)] to preserve the current behavior. For the FFI crates, #![allow(edition_2024_repr_c)] at the top of lib.rs would suffice. If you have a mix of FFI and non-FFI uses of repr(C), then you'll have to do the work to figure out which is which, no matter what option is chosen to update repr(C) - even adding repr(C2), since then the FFI use case would need to update all their reprs to repr(C2).

Overall, I think this scheme only significantly burdens those who have a mix of FFI and non-FFI uses of repr(C). But they were going to be burdened no matter what option was chosen.

[RustyYato]

Is the new wording/lints too noisy still?

[joshtriplett]

I think the new wording is still too noisy. We shouldn't assume that most people using repr(C) are using it for ordering rather than FFI.

[RustyYato]

That wasn't my intention, but I don't see another way to do all of the following in the next edition:

• make repr(C) mean - same layout/ABI as what the standard C compiler does
• make repr(ordered_fields) - the same algorithm that's listed for repr(C) in the Rust reference
• ensure that everyone who upgrades to the next edition gets the layout they need (as long as they read the warnings and follow the given advice)
• make it as painless as possible for people who don't mix FFI and stable ordering cases (which I suspect is the vast majority of people). In other words, each crate currently uses repr(C) either exclusively for FFI or exclusively for some stable layout.
• for people who do mix FFI and stable ordering cases in one crate, at least the warning should give them all the places they need to double-check, and they can silence the warning on a case-by-case basis.

I'm open to suggestions on how to handle the diagnostics. Within these constraints, I think my solution is the only real option we have. If there are some objections to these constraints, I would like to hear those too, maybe I missed the mark with these constraints, and missed a potential solution because of it.

[RustyYato]

@DemiMarie said this in a comment:

Would it be possible to rename repr(C) to something else? That way repr(C) becomes a compile-time error rather than silently changing behavior.

I laid out my design axioms above. I believe that making a new repr and deprecating repr(C) is too costly. Especially when the most prevalent use-case of repr(C) is for FFI. A use-case which would benefit from bug fixes that could happen after this RFC.

[Jules-Bertholet]

• field: [T] results in a fat pointer, which C doesn't have.
• Except for a Clang bug, T field[0]; and T field[]; are equivalent on every C compiler I know of that supports both. This results from the historical fact of T field[0]; being a common extension that C99 decided to standardize with T field[]; syntax.

[briansmith]

• field: [T] results in a fat pointer, which C doesn't have.

No. See https://rust-lang.github.io/rust-bindgen/using-fam.html#using-dynamically-sized-types, for example.

• Except for a Clang bug, T field[0]; and T field[]; are equivalent on every C compiler I know of that supports both. This results from the historical fact of T field[0]; being a common extension that C99 decided to standardize with T field[]; syntax.

How they are handled in Rust would be determined by the ABI / C compiler, right? The issue isn't with C compilers that treat them the same, but where C compilers might not treat them the same. For example, a C compiler could flat-out reject uint8_t field[0]; since it isn't valid ISO C.

[briansmith]

It seems like the intended process for this RFC is to have the RFC make the minimal changes that will allow #[repr(ordered_fields)] to be added, while everything about #[repr(C#editionNext)] would be specified externally. But, as comments showed, there are still some universal things that need to be specified for #[repr(C#editionNext)]:

• Is a #[repr(C#editionCurr)] field allowed in a #[repr(C#editionNext)] type?
• Now to handle non-array#[repr(Rust)] ZST fields in a #[repr(C#editionNext)] type?
• What is the syntactic mapping between Rust and C for flexible array members?

Is it desired to resolve these issues in this RFC, or is there another place (where) to have that discussion?

View changes since the review

[RustyYato]

I understand wanting to specify as much as possible, especially since FFI is such a tricky topic already.

Is a #[repr(C#editionCurr)] field allowed in a #[repr(C#editionNext)] type?

Yes, and in cases where they agree there will be no issues. In cases where they disagree...

Now to handle non-array#[repr(Rust)] ZST fields in a #[repr(C#editionNext)] type?

If they are 1-ZSTs they can be ignored entirely, if they have a non-trivial alignment, then this can be treated like a T field[0] field in C (provided T as the same alignment as the ZST. I think these two should be equivalent on any reasonable C compiler (and all current C compilers). But correct me if I'm wrong.

But I don't think we need to specify non-trivial aligned ZSTs. Since they don't exists in C. If there is sufficient motivation for them, they can be specified later.

What is the syntactic mapping between Rust and C for flexible array members?

I think following current recommendations is probably best, so a trailing field of type [T; 0]. I understand that you like the idea of using [T], but slices are very different from flexible length array members. Slices have a well defined length, and can only be pointed to by a fat pointer. Flexible length arrays have an unknown length, and all pointers to then are thin (since all pointers are thin in C). These differences are significant enough, that I don't think we should conflate the two.

It may be a good idea to introduce a unsized type specifically for flexible length array members, but that's a different discussion entirely. For this RFC, following current norms is sufficient.

No. See https://rust-lang.github.io/rust-bindgen/using-fam.html#using-dynamically-sized-types, for example.

I don't think this says what you think it says. It's still using [T; 0] to represent flexible length array members for FFI, but trying to provide a cleaner interface when you are on the Rust side.

[briansmith]

Now to handle non-array#[repr(Rust)] ZST fields in a #[repr(C#editionNext)] type?

If they are 1-ZSTs they can be ignored entirely,

This sounds great to me.

if they have a non-trivial alignment, then this can be treated like a T field[0] field in C (provided T as the same alignment as the ZST. I think these two should be equivalent on any reasonable C compiler (and all current C compilers). But correct me if I'm wrong. But I don't think we need to specify non-trivial aligned ZSTs. Since they don't exists in C. If there is sufficient motivation for them, they can be specified later.

I agree.

What is the syntactic mapping between Rust and C for flexible array members?

I think following current recommendations is probably best, so a trailing field of type [T; 0]. I understand that you like the idea of using [T], but slices are very different from flexible length array members.

Sorry, I am not following. We already have to map [T] to flexible array members, right From https://google.github.io/zerocopy/zerocopy/trait.KnownLayout.html#dynamiNcally-sized-types:

#[repr(C)]
struct PacketHeader {
    ...
}

#[repr(C)]
struct Packet {
    header: PacketHeader,
    body: [u8],
}

Is this not exactly the Rust encoding of C flexible array members already today? Again, sorry if I'm missing something obvious here.

No. See https://rust-lang.github.io/rust-bindgen/using-fam.html#using-dynamically-sized-types, for example.

I don't think this says what you think it says. It's still using [T; 0] to represent flexible length array members for FFI, but trying to provide a cleaner interface when you are on the Rust side.

Specifically, I was referring to this:

pub unsafe fn flex_ref(&self, len: usize) -> &MyRecord<[::std::os::raw::c_char]> { ... }
pub unsafe fn flex_mut_ref(&mut self, len: usize) -> &mut MyRecord<[::std::os::raw::c_char]> { ... }

Where the field type is actually the slice DST [c_char].

[Jules-Bertholet]

Is this not exactly the Rust encoding of C flexible array members already today? Again, sorry if I'm missing something obvious here.

Both [T] and [T; 0] should map to flexible array members, because both are commonly used to represent them. The former is used when you want a fat pointer with checked indexing, the latter when you want a thin pointer to match C.

[RustyYato]

It would probably be fine to say that [T; N] is compatible with [T] when they are in the last field of a struct, and allow unsizing from Foo<[T; N]> to Foo<[T]>. Where Foo is a repr(C#editionNext) struct that has T as it's last field.

Of course, Foo<[T]> doesn't have a well defined ABI, and neither does a pointer to it. So they can't be used directly in FFI. But I think that should be fine for the use-cases you have shown.

I'm not so sure about general unsizing though (like to dyn traits), especially since that has more ways to conflict with the C layout. So maybe array -> slice unsizing is the only one that should be allowed for repr(C#editionNext) structs?

[Jules-Bertholet]

That's the point, to allow us to delay the discussion instead of forcing the issue now and locking us out forever

[RalfJung]

Given that you apparently want to discuss this now, we must have different definitions of what "delay the discussion" means. ;)

[Jules-Bertholet]

If we document now that this coercion is guaranteed to always work and that the layouts will always match, then unsafe code could rely on it, so it will be difficult/impossible to ever take back even for new targets. If we hedge now, and require unsafe authors to assert before relying on it, we avoid that risk

[briansmith]

Again, there is also the middle position of allowing the coercion on all current targets, but documenting that some future targets might not support it (and that unsafe code can't assume the layout matches without checking). That way, we don't completely lock Rust out of supporting platforms that don't uphold the assumption.

Alternatively, just restrict the coercion to situations where the offset of the field would not change between [T] and [T; N]. If there is ever a target added where that restriction is too strict, then an RFC could be written to add machinery more like what I mentioned above.

[RustyYato]

There's now a section for flexible array members where the guarantee is postponed to an unresolved questions. I think the lang team should decide on what guarantee is best.

[Jules-Bertholet]

• field: [T] results in a fat pointer, which C doesn't have.

No. See https://rust-lang.github.io/rust-bindgen/using-fam.html#using-dynamically-sized-types, for example.

That page doesn't contradict anything I've written?

The issue isn't with C compilers that treat them the same, but where C compilers might not treat them the same. For example, a C compiler could flat-out reject uint8_t field[0]; since it isn't valid ISO C.

If the C compiler simply rejects one of the two, we can just match the one it doesn't reject. There's only an problem if it accepts both but with different layout/ABI. And in that case, it's almost certainly just a bug in the C compiler, like the Clang issue I linked where it doesn't match GCC. Yes, a hypothetical C compiler could theoretically choose to deliberately distinguish them, but there's a lot of cursed stuff that a sufficiently adversarial C compiler could do (e.g. make int and long the same size but different alignments).

[RalfJung]

and field: [T] would map to whatever the C compiler would do for T field[];

I can't make sense of this -- one cannot even pass a [T] by value. [T] is a type with a dynamically known length thanks to the slice metadata. T field[] has entirely unknown length. Those types are so fundamentally different IMO it makes no sense to compare them (and the docs you link to are more confusing than helpful IMO).

[briansmith]

and field: [T] would map to whatever the C compiler would do for T field[];

I can't make sense of this -- one cannot even pass a [T] by value. [T] is a type with a dynamically known length thanks to the slice metadata.

I'm specifically talking about fields in a struct, specifically the last field in a structure. See https://doc.rust-lang.org/nomicon/exotic-sizes.html, and the zerocopy documentation I cited above:

// Can't be stored on the stack directly
struct MySuperSlice {
    info: u32,
    data: [u8],
}

T field[] has entirely unknown length. Those types are so fundamentally different IMO it makes no sense to compare them (and the docs you link to are more confusing than helpful IMO).

How about https://doc.rust-lang.org/reference/dynamically-sized-types.html#r-dynamic-sized.struct-field?:

Structs may contain a DST as the last field; this makes the struct itself a DST.

[briansmith]

If you define these rules as only referring to a struct with a T: ?Sized type parameter that is used for the type of the last field, and you keep this:

Then the exact offset will also be the same as the equivalent C type that has the field type as a flexible array member, i.e. T field[];

and remove this:

Then the exact offset will also be the same as the equivalent C type that has the field type as an array member, i.e. T field[N]; (of any N)"

Then I believe you will have exactly the C rules, without losing anything.

In particular, don't guarantee that this type has the same offset of data as Coercable:

#[repr(C#editionNext)]
struct WithArray {
    len: usize,
    data: [u64; N],
}

That is, the T: ?Sized used as the type of the last field should trigger the FAM alignment rules for the last field.

View changes since the review

[RustyYato]

If you define these rules as only referring to a struct with a T: ?Sized type parameter that is used for the type of the last field, and you keep this:

I don't want to make T: ?Sized fields special. Currently in Rust, if you manually monomorphize a type (and update all related uses), then you don't change behavior. In the extreme, one could make a tool to eliminate all generics in a binary Rust program, and there would be no change in behavior. I would like to preserve this property.

I would be fine with adding an attribute to trigger FAM alignment rules. But that also seems unnecessary, given that all major C compilers do put flexible array members and array members at the same offset.

[Jules-Bertholet]

I don't want to make T: ?Sized fields special. Currently in Rust, if you manually monomorphize a type (and update all related uses), then you don't change behavior.

This isn't quite true, the repr(Rust) layout algorithm already does something similar.

[RustyYato]

Coming back to this (oh boy, months later), I've warmed up to this idea. I'll update the text to only refer to the unsized tail. We do lose a bit of flexibility to non-generic types, but I think that's fine. It's easy enough to model them with an unsized tail to get the desired layout.

[Jules-Bertholet]

What does repr(C, packed) correspond to? Note that there is no equivalent to packed in standard C, only compiler extensions.

The following program (Godbolt):

#include <stdint.h>
#include <stdio.h>

struct attribute_pack {
    _Alignas(16) uint8_t foo;
} __attribute__((packed));

#pragma pack(push, 1)

struct pragma_pack {
    _Alignas(16) uint8_t foo;
};

#pragma pack(pop)

int main() {
    printf("attribute: %zu %zu\n", sizeof(struct attribute_pack),
           _Alignof(struct attribute_pack));

    printf("pragma: %zu %zu\n", sizeof(struct pragma_pack),
           _Alignof(struct pragma_pack));

    return 0;
}

When compiled with x86-64 Linux GCC, prints the following:

attribute: 16 16
pragma: 1 1

View changes since the review

[RalfJung]

I think _Alignas(16) has no equivalent in Rust. #[repr(C, aligned)] corresponds to __attribute__((aligned)).

Then both do the same: https://godbolt.org/z/feeGEEddK

I would specify repr(C, packed) as corresponding to __attribute__((packed)).

[Jules-Bertholet]

I think _Alignas(16) has no equivalent in Rust.

True. My #3806 would change that, but we can delay this question for that RFC.

#[repr(C, aligned)] corresponds to __attribute__((aligned)).

I would specify repr(C, packed) as corresponding to __attribute__((packed)).

These do not exist in MSVC; it has __declspec(align) and #pragma pack.

[RustyYato]

Having over-aligned items in a packed struct is a different issue entirely, more like #3718. So I would defer to that issue. Since repr(C#nextEdition) is intentionally not specified, I think making adjustments to refine the behavior of such over-aligned items can be done in the future.
(I understand that #3718 uses over-aligned types, but I think this is an equivalent idea just expressed with different syntax)

I would be fine with warning in cases where there are overaligned items in a repr(C#editionNext) type to indicate that this behavior is under-specified.

One question, if you don't have over-aligned items in your type, do these two ways of making a C struct packed differ?

[RalfJung]

These do not exist in MSVC; it has __declspec(align) and #pragma pack.

And is __declspec(align) something one puts on fields or on types?

[RustyYato]

I understand your concern, but there are many cases here to consider

• I don't think we can just ban repr(C#editionNext, packed) outright
• As you have noted we can't reliably error on these cases (due to generics), so this is at best a warning
• A warning on every instance of a generic parameter used by value in repr(C#editionNext, packed) may work, but I think would get pushback for being too noisy
  • Given the pushback I've already gotten on all the other warnings I've tried introducing in past versions on this RFC

So I think that the best way to get this RFC pushed through is to just leave some parts intentionally unspecified for now. This is ugly. But it won't be a permanent feature, and will be refined in the future. I think we should have big bold wording in the reference to shine a light on this issue, so people aren't caught unaware.

This is also an edge case that I hope most projects don't run into. Maybe we could do a crater run to see how many projects use repr(C, aligned) with over-aligned types (by making it a post-mono error for the crater run)? While this wouldn't be representative of only FFI use-cases, it should give an upper bound on how many projects would be affected.

[RalfJung]

As you have noted we can't reliably error on these cases

We can, post-mono.

[Jules-Bertholet]

And is __declspec(align) something one puts on fields or on types?

Both. https://learn.microsoft.com/en-us/cpp/cpp/align-cpp?view=msvc-180

[RalfJung]

#[repr(C, aligned)] corresponds to __attribute__((aligned)).

I would specify repr(C, packed) as corresponding to __attribute__((packed)).

These do not exist in MSVC; it has __declspec(align) and #pragma pack.

I guess then repr(align) and repr(packed) on MSVC targets have to correspond to those two (putting the __declspec(align) on the declaration of that type). 🤷
Doesn't seem like we have any choice here.

And on the GCC side, for the types we currently support, it seems like __attribute__((packed)) and #pragma pack behave the same. So we can say either works and if in the future it turns out it makes a difference then we'll make that choice.

[RustyYato]

We can, post-mono.

I guess with how trivial it is to trigger post-mono errors with const asserts, this isn't much worse. I'll update the RFC to use a post-mono error here, but I'll keep this as an unresolved question for now.

[RalfJung]

I don't like this hack. The type Foo<dyn T> still exists, and with repr(C) types people reasonably assume that they can be created "by hand" in unsafe code so just nerfing the safe constructor doesn't really help.

View changes since the review

[RustyYato]

I think a hard error on the coercion with a good error message will provide enough of a speedbump.

We can even ban it in ptr_metadata by giving dyn T and Foo<dyn T> different types for Pointee::Metadata. Furthermore, we could spec <Foo<dyn T> as Pointee>::Metadata to be uninhabited when Foo is repr(C#editionNext)

Edit: Just realized I already had a section on trait objects. I'll remove this new duplicate section. The old section says the same in more detail.

[RalfJung]

In the end I think a dyn Trait field should be treated pretty much like a repr(Rust) field. I guess we plan to allow those, but the improper_ctypes lint should fire. The docs should explicitly say that the layout of such "rusty" types is subject to change.

We don't do weird shenanigans like erroring in the struct constructor when there is a repr(Rust) field, so I don't see a good reason to do that for dyn Trait either.

IMO the two reasonable options for both of these are

• document as unspecified
• post-mono error

[RustyYato]

We don't do weird shenanigans like erroring in the struct constructor when there is a repr(Rust) field, so I don't see a good reason to do that for dyn Trait either.

maybe we should 😈

---

On a more serious note, I don't think this is similar to normal repr(Rust) fields. Allowing coercions from Foo<T> to Foo<dyn T> locks us into some layout guarantees for the unsized tail which I think may come back to bite us.

For example, a compliant C compiler may over align the last field, but that wouldn't be valid for Rust if we allowed coercing from Foo<T> to Foo<dyn T> esp. given that Foo<dyn T> would have the same vtable as dyn T, since you want to preserve the current behavior. We would be forced to keep the unsized tail aligned according to the type, and could not over-align it.

This is similar to the objections I initially had for FAMs, but there we at least had some use-cases that actually benefited from this extra guarantee. Here we don't have such use-cases, so I'm less inclined to support them (dyn T as an unsized tail in repr(C#editionNext) types).

[RalfJung]

I see. So the argument is that specifically the coercion relies on the offset being homogeneous across all Foo<_> in some sense, so that the offset can be recomputed from the vtable.

That is a much better argument for forbidding the coercion than saying that it will "provide enough of a speedbump".

That said, I still think that we should then say that this dyn Trait field just doesn't have an offset, i.e., the entire type is not valid. You were willing to accept post-mono errors for the aligned-in-packed thing; I expect dyn-trait-tails to be much less common than that.

[RustyYato]

I don't think we need post-mono errors here, I think my proposed solution avoids them well enough. For aligned-in-packed it's a lot harder to avoid post-mono errors.

[RalfJung]

I don't think it avoids potential issues well enough. We still compute a layout for this type that's just fundamentally bogus. We still allow you to project to a field for which the offset is nonsense. That just feels very un-Rusty to me, to produce garbage data and then try to paper over the easy ways to access the garbage data.

[Jules-Bertholet]

I think we should treat these the same as FAM/slice tails: we allow the type everywhere, but only allow the coercion on platforms where the offset is always the same, and emit a lint for using the type on platforms where it's not. Being more restrictive means breaking existing users more.

[RustyYato]

I'd like to note that since you first commented, I've updated this section.

Now, Foo<dyn T> is truly impossible to create (even for unsafe code). Same with one level of pointer indirection i.e. *const Foo<dyn T>. This is done by making <Foo<dyn T> as Pointee>::Metadata an uninhabited type.

As for computing layout, we no longer really need to.

• This type cannot exist on the stack, since it's unsized. So we don't need to compute a layout for that.
• (size|align)_of_val(_of_raw)? are all functions of fn(uninhabited) -> usize, so the trivial implementation works for them
• Accessing fields, this requires at some point loading a pointer to Foo<dyn T>, which is uninhabited. So again, trivial to implement.

I think this covers all cases. While it may be surprising that *const Foo<dyn T> is uninhabited, I don't think this is fundamentally problematic.

Does this address your concerns?

[RalfJung]

I think we should treat these the same as FAM/slice tails: we allow the type everywhere, but only allow the coercion on platforms where the offset is always the same, and emit a lint for using the type on platforms where it's not. Being more restrictive means breaking existing users more.

Emit a lint when exactly?

And is there a non-zero set of users that would be broken?

This is done by making <Foo as Pointee>::Metadata an uninhabited type.

That on its own does nothing, ::Metadata does not participate in type-checking regular uses of this type.

I guess what you mean is that this is the intended validity requirement. But what concretely does this mean for the compiler in terms of which code does or does not get accepted?

Does this address your concerns?

It's better than the previous proposal but IMO still worse than just saying that the type is "ill-formed", similar to e.g. a type that's too big.

[RalfJung]

Why is the alternative not "compute the layout that the C compiler would compute"?

View changes since the review

[RustyYato]

Why is the alternative not "compute the layout that the C compiler would compute"?

For this RFC? Because there's a whole other RFC for that. #3718

I don't want to bog this RFC down by expanding it's scope. And C compilers don't have a consistent answer (as far as I can tell)

[RalfJung]

That RFC is mostly about introducing new repr's to account for targets with more than one layout algorithm.

Given that this RFC introduces "a repr that works like C", and given rust-lang/rust#162160, I don't feel like saying "yes this repr indeed always works like C, also for aligned-in-packed" is extending its scope. This RFC anyway has to say how repr(align) and repr(packed) map to C in isolation. It seems like an entirely artificial limitation to then not allow them to be composed.

[RalfJung]

packed maps to align on MSVC? That does not sound right.

Also repr(C#editionNext, packed) is always applied to a type so the "when applied to a type" doesn't make much sense.

View changes since the review

[RustyYato]

I just copied your comment. I'm not that familiar with MSVC, so if there's a different attribute to use, I'm happy to switch to that instead.

I guess then repr(align) and repr(packed) on MSVC targets have to correspond to those two (putting the __declspec(align) on the declaration of that type). 🤷
Doesn't seem like we have any choice here.

#3845 (comment)

Also repr(C#editionNext, packed) is always applied to a type so the "when applied to a type" doesn't make much sense.

I was talking about __declspec(align) since that can be applied to fields as well. I'll try and clarify this.

edit: I guess it should read more like on MSVC repr(..., packed) maps to the combination of #pragma pack and __declspec(align)

[RalfJung]

I never suggested to map packed to align. Obviously packed maps to packed and align to align.

I was talking about __declspec(align) since that can be applied to fields as well. I'll try and clarify this.

Yes it can but that's not really relevant. We are describing a Rust-to-C mapping here. The fact that C can do other things not used by the mapping is irrelevant and a distraction.

[RalfJung]

Suggested change

	`repr(C#editionNext, packed)` will map to `__attribute__((packed))` on non-MSVC targets, and for MSVC it maps to `__declspec(align)` when applied to a type (and not a field).
	`repr(C#editionNext, packed(N))` will map to `__attribute__((packed))` on non-MSVC targets, and for MSVC it maps to `#pragma pack(push, N)` before the type and `#pragma pack(pop)` after the type. (The assumption is that for compilers that support both the attribute and the pragma, both do the same thing, which should be true for the types Rust can currently express.)

TODO: what is the GNU syntax for N > 1 packing?

[RalfJung]

You dropped this part

(The assumption is that for compilers that support both the attribute and the pragma, both do the same thing, which should be true for the types Rust can currently express.)

I think it is important for this assumption to be recorded explicitly, so that people don't assume that we made some sort of deliberate choice between two possible options here.

[RustyYato]

That's fair, I'll update this

[RalfJung]

I don't think this example is very relevant because these C types do not correspond to any Rust type.

View changes since the review

[RustyYato]

I think we should attempt to be future compatible, and consider types that we may add support for (even if we don't support them properly now). There have been discussions (see #3806) to allow such types, so it's not out of the realm of possibility that we will support them in the future.

[RalfJung]

That is what I would call scope creep. If #3806 adds things that cause problems, it is up to #3806 to deal with them, e.g. by forbidding #[align] attributes in #[repr(C#editionNext, packed)] structs. That's trivially checkable pre-mono.

repr(packed) and repr(align) are features of Rust today. This is not some hypothetical future. And for this RFC to achieve its goal it is important that as much as possible, existing code can just switch to C#editionNext. There shouldn't be much code out there that has aligned-type-in-packed-type, but I expect the number to be non-zero, so we should have a really good reason for not supporting a proper migration for that code. I don't think hypothetical future language features are a good reason.

[Jules-Bertholet]

If #3806 adds things that cause problems, it is up to #3806 to deal with them, e.g. by forbidding #[align] attributes in #[repr(C#editionNext, packed)] structs.

As the author of that RFC, I agree

[RalfJung]

Is #pragma pack(push, N) not a thing? It sounds like that should do the right thing. At least for values of N that are supported...

This combination of packed + aligned on the C side makes me nervous. I hope our C experts can confirm that that's indeed what we want. @Jules-Bertholet ?

View changes since the review

[RustyYato]

I don't think this supports all of what we want to support with repr(..., align). (emphasis mine)

(Optional) Specifies the value, in bytes, to be used for packing. If the compiler option /Zp isn't set for the module, the default value for n is 8. Valid values are 1, 2, 4, 8, and 16. The alignment of a member is on a boundary that's either a multiple of n, or a multiple of the size of the member, whichever is smaller.

[Jules-Bertholet]

__attribute__((packed, aligned(N))) will force the alignment up to N if it would have been lower otherwise; #pragma pack(push, N) will not. Also, __attribute__((packed, aligned(N))) will have no padding at all between structure elements.

[RalfJung]

attribute((packed, aligned(N))) will force the alignment up to N if it would have been lower

That's wrong then; packed(N) does not do that.

[RalfJung]

I don't think this supports all of what we want to support with repr(..., align). (emphasis mine)

True, but are you sure that the other one behaves correctly?
Maybe we just cannot support packed(N) for N > 16 on MSVC.

[RustyYato]

Ok, then what should the mapping be? Should we just use #pragma pack(push, N) on all targets?

[RalfJung]

If that's the only way to do N > 1 in GCC then yeah that's probably best.

FWIW for the pragma, GCC has this to say

The n value below always is required to be a small power of two and specifies the new alignment in bytes.

They are not saying how "small" is defined.

So both MSVC and GCC have such a limit. Our options are:

• Do the "natural" extrapolation from 1, 2, 4, 8, 16 to arbitrary powers of two. It seems hard to believe that if they ever allow 32 or 64 that it'll do something special. Possibly additionally lint for this to say that hey this type doesn't have a C equivalent.
• Hard error. Current repr(C, packed(128)) types cannot migrate to C#editionNext.