Is there any particular downside to just making this guarantee for all enums similar to Option and Result in layout, like Poll for instance?

Yes that was mentioned in the Future Work portion. Basically it can probably become a semver hazard sort of thing if it's automatically done for all enums and people aren't opting in to the behavior. Keeping the RFC scoped small makes it more likely to be accepted and completed on the types people use most often.

Ahh, I see; I completely missed that part. I guess that what I'm failing to understand is how you can actually run into that kind of hazard unless you use something like #[non_exhaustive], since any changes to the type would be breaking anyway.

The examples need a bit more diversity. They are all of the form Result<T, ()>, while the proposal also covers Result<(), E> or Result<T, ZST>.

This needs some more discussion on which exact zero-sized types should have layout optimization guarantees, because it could be a semver hazard, particularly in FFI. A type struct Error {} could have new fields added in the future. Perhaps limit only to #[repr(C)] ZSTs? Or to structs of the form struct NoParams;, where any field addition is obviously a breaking change?

I have doubts whether the layout guarantees would be useful in practice, though. The primary motivation in the RFC isn't memory usage but FFI ergonomics. With Option<&T>, for example, a raw pointer is already treated as "either nothing or a valid *mut T" in C function APIs, so the guarantee is reasonable. But with fallible APIs, the conventions are all over the place. Some functions return negative integers on error, some - positive ones, some do both of those, some even return 0, or return garbage and set ERRNO. In principle, you could cover those cases, e.g. Result<(), NonZeroI32> covers the "0 for success, nonzero for error" part. But in practice, you would expect specific error codes and not just arbitrary integers, so you'd have to do some error classification on the Rust side anyway.

This means that a separate ABI-stable struct as the return type, with functions success(self) -> T and error(self) -> Error would likely be a better choice. Once the Try-trait gets stable, you could also use all the usual ? sugar with those types.

The point about ZST is also brought up last week by @tmandry during lang team triage meeting (notes). We could use limit it to #[repr(transparent)] or #[repr(C)] ZSTs.

Tbh, I'm not particularly keen on having guarentees on repr(C) ZSTs. They are not a thing in ISO C and compiler extensions may (or may not) do different things with them which makes them potentially an FFI hazzard.

@ChrisDenton Rust guarantees their specific behaviour, so this seems like a non-issue. On the FFI side none of those types would exist anyway, just like Result<_, _> doesn't. Though #[repr(C)] is likely indeed a poor choice for layout optimization guarantees, since one could just as easily add private fields as to a #[repr(Rust)] type. #[repr(transparent)] indeed looks like a better option.

repr(transparent) newtypes of () would be sufficient to allow for different errors, but a lot of people would probably ask why they have to put a "useless field" in their error type to make it work right.

Why would you make it a newtype of ()? I'm thinking of the following definitions:

struct ZST;
#[repr(transparent)]
struct ZST();
#[repr(transparent)]
struct ZST {}
#[repr(transparent)]
struct ZST<T>(PhantomData<T>);

I had thought that a repr(transparent) needed to have a single non-zero sized field, but double checking using your example.. it seems that there can be one or zero non-zero sized fields.

So sure, repr(transparent) (or fieldless) seems a fine requirement on the ZST. One moment and I'll make an edit.

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

• needs-to-mention-non_exhaustive resolved by RFC: result_ffi_guarantees #3391 (comment)

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

Overall seems reasonable.

@rfcbot concern needs-to-mention-non_exhaustive

It's implied by the definition of non_exhaustive that a non-exhaustive ZST can't be guaranteed here, but since we've had so many issues about it (rust-lang/rust#78586), I'd like to see it called out explicitly in here to make sure we get tests about it.

it seems that there can be one or zero non-zero sized fields.

Zero fields was FCPed in rust-lang/rust#77841

There's been some ideas of potentially modifying the calling convention to support Result more cheaply. Does this RFC interfere with that?

There's been some ideas of potentially modifying the calling convention to support Result more cheaply. Does this RFC interfere with that?

This RFC requires that the combined type be treated as the primitive type, so I guess "yes, it could interfere with that". However, since the primitive types are generally single-register (except and wide pointers and ints bigger than a machine register), this doesn't seem like a strong problem. Other forms of Result (eg: Result<(),()>) could still (in the future) be optimized differently (eg: have the function "return the value" via status flag being set or not).

Zero fields was FCPed in rust-lang/rust#77841

It seems that the reference didn't get an update.

concern needs-to-mention-non_exhaustive

Updated.

one thing i'd specifically want to reserve space for is representing Result errors with unwinding (distinct from panicking) when the user opts into it, probably through an attribute on the error type or on the function. the idea is this is more efficient for functions that very rarely fail, because the generated LLVM IR don't have to construct Result values or match on them.

this may need a specific carve-out exception in the defined ABI for Result, so we should carve it out now before it becomes a breaking change.

Hm, could you say more about what you'd expect to be changed within the RFC to have that carved out? Some mentioning in Future Possibilities? I'm completely unfamiliar with how unwinding interacts with the foreign ABI modes.

one thing i'd specifically want to reserve space for is representing Result errors with unwinding (distinct from panicking) when the user opts into it,

This RFC covers the FFI case, which wouldn't use that optimisation. And if it did then the future RFC proposing that would have a lot of specifying to do, and I think that specifying belongs there rather than here.

Hm, could you say more about what you'd expect to be changed within the RFC to have that carved out? Some mentioning in Future Possibilities? I'm completely unfamiliar with how unwinding interacts with the foreign ABI modes.

I'm basically expecting that the list of requirements to have Result have a defined ABI would have an entry kinda like this:

• Future RFCs may introduce additional requirements, e.g. some new 1-ZST error types may be marked such that they opt out of stable ABI for Result<T, MyErr> e.g. by representing the Err variant by unwinding instead of returning normally.

this way we tell users they can't just just check the current list of requirements for any arbitrary type and be guaranteed to always forevermore have stable ABI when those requirements are met, future RFCs may introduce exceptions for types using new features.

basically I want to say that the following code is unsound:

pub fn f<T>(v: extern "C" fn() -> u32) -> extern "C" fn() -> Result<NonZeroU32, T> {
    assert!(size_of::<T>() == 0 && align_of::<T>() == 1);
    unsafe { transmute_copy(&v) }
}

I think I'm with josh, if there was some new way to work with results that would be some new abi, not the existing abis.

If the rfcs says "you need to meet these requirements, and also maybe we'll add more requirements later", that's not a stable guarantee at all.

Your example code is basically exactly what I want to have be allowed: Assuming some T that's trivially constructable.

I think I'm with josh, if there was some new way to work with results that would be some new abi, not the existing abis.

If the rfcs says "you need to meet these requirements, and also maybe we'll add more requirements later", that's not a stable guarantee at all.

it is a stable guarantee, because it's guaranteed to work with all types not using features defined in any future RFCs adding exceptions, e.g. it always works with Result<NonZeroU32, ()> because it would be a breaking change to add those new features to any existing 1-ZST types because of the ABI change.

Your example code is basically exactly what I want to have be allowed: Assuming some T that's trivially constructable.

but the point is that example code doesn't check for any of the new features that might be on some T passed in by arbitrary user code.

if ZST types had to be explicitly marked as ABI stable for Result, that would resolve my concern about needing a carve-out for unwind-on-error types, since we could simply make that ABI stable for Result attribute mutually exclusive with the unwind-on-error attribute.

@afetisov

@ChrisDenton Rust guarantees their specific behaviour, so this seems like a non-issue. On the FFI side none of those types would exist anyway, just like Result<_, _> doesn't.

It wouldn't matter in this case, since the RFC is only specifying the behavior of Result<T, E>, which would not otherwise have any ABI guarantee even when T and/or E are types that do have an ABI guarantee when standing alone.

...But it's an interesting question whether, in general, #[repr(C)] unit structs should be guaranteed ABI-compatible with C empty structs (which are an extension), at least on platforms whose standard compilers support that extension. I did a bit of research. rustc currently claims they are not FFI-safe, which apparently happened as a result of this issue from 2015. The topic came up again in 2018 with the AArch64 ABI and there is a still-open followup issue, though the question really shouldn't be specific to AArch64.

Admittedly, it's a topic for another thread, especially now that the RFC no longer mentions #[repr(C)]. Just thought I'd point out that the situation is more complicated than 'ZSTs don't exist in FFI'.

@scottmcm I believe your concern has been resolved if you want to review again.

@rfcbot resolve needs-to-mention-non_exhaustive

Suggestion: add a new layout attribute, #[repr(ZST)]. This attribute can only be placed on types which are considered to be guaranteed ZSTs according to the current language rules. The attribute doesn't change the fact whether the type is a ZST, but it serves as a public guarantee that the zero-sized layout is intended and can be relied on, and not just a temporary accident. Removing #[repr(ZST)] is a breaking change. Putting it on a non-ZST is a compile error.

More specifically, #[repr(ZST)] can be applied only to structsshould we also include unions?, such that all fields are #[repr(ZST)], including one of the primitive ZST types. The primitive ZSTs include (), [T; 0], PhantomData<T>, and function pointer types (should there be others?).

#[repr(ZST)] can be combined with #[repr(transparent)], but doesn't change the layout in this case, since a transparent type wrapping a ZST is already a ZST. However, #[repr(ZST)] is a public guarantee that a transparent type stays ZST, and thus is a stronger guarantee.

In particular, ZST-based layout optimizations, like the ones proposed in this RFC, apply only to #[repr(ZST)] types, and not to accidentally ZSTs or to #[repr(transparent)] wrappers around #[repr(ZST)] types.

#[repr(ZST)] cannot be combined with #[repr(C)] since the C standard doesn't have the concept of ZST. In particular, C++ compilers turn empty structs into types of size 1.

Is this fine? Perhaps this should be extracted into a separate RFC, but it doesn't have any current purpose without the layout optimization guarantees, so I'd add it to the current RFC.

Is this fine? Perhaps this should be extracted into a separate RFC, but it doesn't have any current purpose without the layout optimization guarantees, so I'd add it to the current RFC.

there is a current purpose:

mod some_crate {
    #[repr(ZST)]
    pub struct SomeStruct {
        f: ()
    }
}

#[repr(transparent)]
pub struct MyStruct {
    a: u8,
    b: some_crate::SomeStruct, // guaranteed to not be compile error
}

@afetisov One problem there is that ZST-ness is necessary, but not sufficient. repr(transparent) specifically needs a 1-ZST, because [u128; 0] is a ZST but you still can't have a #[repr(transparent)] struct Foo(u8, [u128; 0]);.

Also, I think that reprs are a poor way to communicate public guarantees. I think that traits communicate "yes, everyone can rely on this" better, especially since the compiler can help with it. And it could let the compiler allow #[repr(transparent)] struct Foo<M: Definitely1ZST>(u8, M); that's impossible right now. Not to mention that it's something that any semver checker would already handle.

@rfcbot fcp reviewed

This makes sense to me.

This is now entering its final comment period, as per the review above.

The primitive ZSTs include (), [T; 0], PhantomData<T>, and function pointer types

Slight correction: function item types. Function pointers have sizes.

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

This RFC was discussed in this week's lang team triage meeting. We (I) weren't sure whether the compiler already treats types like Result<NonZeroI32, ()> with the same ABI as C int32_t, vs with the same ABI as C struct { int32_t; }. (The distinction between these 2 in extern "C", unlike in extern "Rust", is why RFC 1758 repr(transparent) was needed.)

lokathor: does compiler already do this?

nikomatsakis: Pretty sure it does.

cramertj: is this insta-stable?

nikomatsakis: I think so, or else we can fcp merge the PR to the reference.

If Result<NonZeroI32, ()> already works like int32_t on all platforms then implementing this RFC is a matter of exempting the appropriate types from improper_ctypes / improper_ctypes_definitions, and testing; if it works like struct { int32_t; } there would need to be codegen changes.

As far as I could tell, the former is the case, at least in my usual suspect ABI (32-bit x86).

// cargo asm --target i686-unknown-linux-gnu

use std::num::NonZeroI32;

#[repr(C)]
pub struct Wrapper {
    pub primitive: i32,
}

pub extern "C" fn primitive() -> i32 {
    1
}

// different code than `primitive`
pub extern "C" fn wrapper() -> Wrapper {
    Wrapper { primitive: 1 }
}

// same code as `primitive`
pub extern "C" fn result_nonzero() -> Result<NonZeroI32, ()> {
    Ok(unsafe { NonZeroI32::new_unchecked(1) })
}

The @rust-lang/lang team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here:
rust-lang/rust#110503