Tracking Issue for result_ffi_guarantees (RFC 3391)

[tmandry]

This is a tracking issue for the RFC "result_ffi_guarantees" (rust-lang/rfcs#3391).

About tracking issues

Tracking issues are used to record the overall progress of implementation. They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions. A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature. Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

• [ ] Implement (test) the RFC
• [ ] Adjust documentation (see instructions on rustc-dev-guide)
• [x] ~Stabilization PR (see instructions on rustc-dev-guide)~ (not necessary, reference FCP should be all that's needed)

Unresolved Questions

None

Implementation history

[RalfJung]

I did a bunch of work on ABI tests for repr(transparent) that could also be useful for testing this RFC:

• Miri now has this test: https://github.com/rust-lang/miri/blob/master/tests/pass/function_calls/abi_compat.rs
• and rustct will hopefully soon have this: https://github.com/rust-lang/rust/pull/115372

[MasterAwesome]

Is anyone still working on this? If not, I'd love to try my shot at it.

[Lokathor]

I have not heard word of anyone working on this in a very long time. It's probably a safe bet to begin a PR.

[RalfJung]

Besides adjusting the improper-ctypes lint (which is happening in https://github.com/rust-lang/rust/pull/122253), some other things that should be done:

• For Option we have this documentation. I assume we want the same for Result? Is there some way to not repeat the table?
• Miri has a test here that currently only checks Option, not Result.

[MasterAwesome]

Should the documentation change be a separate PR?

[RalfJung]

Yes, that probably makes review easier.

[Lokathor]

The lint update PR has merged.

[RalfJung]

I hope docs are next, then. :) We shouldn't go too long with such a gap between what we document and what we implement.

The Result type itself should get suitable documentation. And probably there's somewhere in the reference as well that needs updating?

[Lokathor]

philosophically speaking I'm not sure how much the reference needs to say in addition to what the standard library type docs say, but yeah docs on Option and Result are the next step. I will likely have time for a docs PR very soon if no one else does it first

[Lokathor]

The docs are merged.

@tmandry Is there more to do or can we officially call this done?

[RalfJung]

I think Miri still needs tests. Those would go around here.

[Lokathor]

Followup: The MIRI test cases have been merged.

I believe we're done? Should we put this into the release notes two releases from now?

[RalfJung]

Yeah sounds right! I have added the relnotes label to https://github.com/rust-lang/rust/pull/124870. But with that PR being "closed" rather than "merged" I don't know if that will work properly...

[cuviper]

This is still unstable, right? What exactly are we wanting to promise in relnotes?

e.g. This still warns if the feature gate is not enabled: playground

//#![feature(result_ffi_guarantees)]

extern "C" {
    pub fn foo() -> Result<(), std::num::NonZeroU32>;
}

warning: `extern` block uses type `Result<(), NonZero<u32>>`, which is not FFI-safe
 --> src/lib.rs:4:21
  |
4 |     pub fn foo() -> Result<(), std::num::NonZeroU32>;
  |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not FFI-safe
  |
  = help: consider adding a `#[repr(C)]`, `#[repr(transparent)]`, or integer `#[repr(...)]` attribute to this enum
  = note: enum has no representation hint
  = note: `#[warn(improper_ctypes)]` on by default

[Lokathor]

I guess there needs to be a final stabilization PR for the feature, but it doesn't need T-lang signoff?

[RalfJung]

Why would it not need t-lang signoff?

[Lokathor]

that's just what @tmandry wrote in the start of the thread next to the checked box.

[RalfJung]

Where is the reference PR they are mentioning there?

[Lokathor]

I don't know, actually. I just saw a checked box and assumed the box wouldn't be checked if it wasn't completed yet.

[tmandry]

I must have been under the impression that this was only a documentation change. It probably needs a lang team signoff since it's more than that.