Skip to content

Add a function to convert &T to &MaybeUninit<T> #561

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

Closed
crlf0710 opened this issue Mar 15, 2025 · 7 comments
Closed

Add a function to convert &T to &MaybeUninit<T> #561

crlf0710 opened this issue Mar 15, 2025 · 7 comments
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@crlf0710
Copy link
Member

Proposal

Problem statement

See motivating examples.

Motivating examples or use cases

MaybeUninit::as_bytes(unstable) can see any readonly maybe-uninit value as a readonly sequence of maybe-uninit u8. If there's a function that can see any &T as &MaybeUnit<T>, it can work together with that function.

Solution sketch

Add a function to convert &T to &MaybeUninit<T>. I don't have any naming suggestions.

Alternatives

Add a function to directly convert &T to &[MaybeUninit<u8>], which is less flexible

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@crlf0710 crlf0710 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Mar 15, 2025
@hanna-kruppe
Copy link

The motivation only mentions chaining with MaybeUninit::as_bytes but I don’t understand what that combination would be useful for (I hardly know what the existing method can be used for). To know which of the bytes are initialized and can be used for anything, you generally have to know the layout of T very precisely. Do you have a concrete example where the detour through MaybeUninit<[u8]> lets you do something more easily than working in terms of the original &T?

@crlf0710
Copy link
Member Author

Frankly i'd consider &T to &MaybeUninit<T> a primitive operation. As it's not a shorthand of some combination of other primitive operation, this method is worth existing merely for teaching that this operation is correct.

I don't want to focus on whether MaybeUninit::as_bytes is useful(it is though, if all you want to do is read out a few bytes, without using for example bytemuck as dependency), as that is another topic.

@ChrisDenton
Copy link
Member

Frankly i'd consider &T to &MaybeUninit<T> a primitive operation.

This makes it sound like some kind of language support might be useful?

@lukas-code
Copy link
Member

lukas-code commented Mar 21, 2025

this method is worth existing merely for teaching that this operation is correct

This operation is unsound in combination with MaybeUninit::as_bytes, because it can can lead to LLVM noalias violations with !Freeze types:

#![feature(maybe_uninit_as_bytes)]

use std::cell::Cell;
use std::mem::MaybeUninit;

fn as_uninit<T>(v: &T) -> &MaybeUninit<T> {
    unsafe { &*(v as *const T as *const MaybeUninit<T>) }
}

// `b` is `noalias`, which means that any byte that is *accessed* (read or write)
// through `b` must not be written to for the duration of this call, except via
// pointers derived from `b`.
//
// In this example
// - `a` and `b` alias (they point to the same byte)
// - `a` is not derived from `b`
// - `b` is accessed (read)
// - `a` is written to
// => UB
fn foo(a: &Cell<u8>, b: &MaybeUninit<u8>) {
    a.set(1);
    let _v = *b;
}

fn main() {
    let v = Cell::new(0);
    foo(&v, &as_uninit(&v).as_bytes()[0]);
}

And miri correctly flags this as UB for both aliasing models:

stacked borrows
error: Undefined Behavior: not granting access to tag <1179> because that would remove [SharedReadOnly for <1166>] which is strongly protected
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cell.rs:503:31
    |
503 |         mem::replace(unsafe { &mut *self.value.get() }, val)
    |                               ^^^^^^^^^^^^^^^^^^^^^^ not granting access to tag <1179> because that would remove [SharedReadOnly for <1166>] which is strongly protected
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <1179> was created by a SharedReadWrite retag at offsets [0x0..0x1]
   --> src/main.rs:19:5
    |
19  |     a.set(1);
    |     ^^^^^^^^
help: <1166> is this argument
   --> src/main.rs:18:22
    |
18  | fn foo(a: &Cell<u8>, b: &MaybeUninit<u8>) {
    |                      ^
    = note: BACKTRACE (of the first span):
    = note: inside `std::cell::Cell::<u8>::replace` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cell.rs:503:31: 503:53
    = note: inside `std::cell::Cell::<u8>::set` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cell.rs:429:9: 429:26
note: inside `foo`
   --> src/main.rs:19:5
    |
19  |     a.set(1);
    |     ^^^^^^^^
note: inside `main`
   --> src/main.rs:25:5
    |
25  |     foo(&v, &as_uninit(&v).as_bytes()[0]);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
tree borrows
error: Undefined Behavior: write access through <1005> at alloc575[0x0] is forbidden
   --> /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:865:9
    |
865 |         crate::intrinsics::write_via_move(dest, src);
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ write access through <1005> at alloc575[0x0] is forbidden
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental
    = help: the accessed tag <1005> is foreign to the protected tag <993> (i.e., it is not a child)
    = help: this foreign write access would cause the protected tag <993> (currently Frozen) to become Disabled
    = help: protected tags must never be Disabled
help: the accessed tag <1005> was created here
   --> src/main.rs:19:5
    |
19  |     a.set(1);
    |     ^^^^^^^^
help: the protected tag <993> was created here, in the initial state Frozen
   --> src/main.rs:18:22
    |
18  | fn foo(a: &Cell<u8>, b: &MaybeUninit<u8>) {
    |                      ^
    = note: BACKTRACE (of the first span):
    = note: inside `std::mem::replace::<u8>` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/mem/mod.rs:865:9: 865:53
    = note: inside `std::cell::Cell::<u8>::replace` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cell.rs:503:9: 503:61
    = note: inside `std::cell::Cell::<u8>::set` at /playground/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/cell.rs:429:9: 429:26
note: inside `foo`
   --> src/main.rs:19:5
    |
19  |     a.set(1);
    |     ^^^^^^^^
note: inside `main`
   --> src/main.rs:25:5
    |
25  |     foo(&v, &as_uninit(&v).as_bytes()[0]);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

To make this sound, there would have to be a T: Freeze bound either on this function (as_uninit) or on as_bytes. Arguably, it should be on as_bytes because that's what causes the noalias attribute to be added in the first place and its soundness w.r.t. interior mutable types is already unclear (see rust-lang/rust#93092 (comment)). But anyway that's something to keep in mind with this proposal.

@m-ou-se
Copy link
Member

m-ou-se commented Mar 21, 2025

Frankly i'd consider &T to &MaybeUninit<T> a primitive operation.

Similar to &NonZeroU8 to &u8, etc.

All of these are cases of sub/supertyping that Rust doesn't natively support.

(If the language were to support this, we'd also have &[T] to &[MaybeUninit<T>] and &&&NonZeroU8 to &&&u8 and so on, thanks to (co)variance.)

@m-ou-se
Copy link
Member

m-ou-se commented Mar 21, 2025

cc @oli-obk - At FOSDEM we talked about MaybeUninit in the context of pattern types, which might allow conversions like this.

@joshtriplett
Copy link
Member

joshtriplett commented Mar 25, 2025

We discussed this in today's @rust-lang/libs-api. We agreed that there should be a Freeze bound on as_bytes. Once that bound is added, this method would be sound, and we can approve adding this method.

@Amanieu suggested the name MaybeUninit::from_ref.

@joshtriplett joshtriplett added the ACP-accepted API Change Proposal is accepted (seconded with no objections) label Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ACP-accepted API Change Proposal is accepted (seconded with no objections) api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

6 participants