Skip to content

[lib] In-place initialization infrastructure #142518

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dingxiangfei2009
Copy link
Contributor

@dingxiangfei2009 dingxiangfei2009 commented Jun 14, 2025

This is the initial design of the in-place initialization interface, without the PinInit part.

I will be iterating on the documentation as the proposal matures over time.

We probably need a tracking issue number as well. I am using a placeholder for the moment. I will be happy to update it when it arrives.

r? @joshtriplett

cc @compiler-errors
cc @Darksonn @y86-dev

Lang experiment: rust-lang/lang-team#336

This is the initial design of the in-place initialization
interface, without the `PinInit` part.

Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>
Co-authored-by: Benno Lossin <lossin@kernel.org>
Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 14, 2025
Copy link
Contributor

@BennoLossin BennoLossin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll probably add some more comments in the next week. I think we need more examples on the Init trait on how to use "normally" and in more obscure ways (but feel free to let me provide them).

I think it makes sense to also add the PinInit trait, as we probably want to reference it several times from the Init docs

/// Implementers must ensure that if `init` returns `Ok(metadata)`, then
/// `core::ptr::from_raw_parts_mut(slot, metadata)` must reference a valid
/// value owned by the caller. Furthermore, the layout returned by using
/// `size_of` and `align_of` on this pointer must match what `Self::layout()`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By size_of and align_of, do you mean size_of_val and align_of_val (or even their raw versions, since it might not be possible to turn it into a reference)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree. Would it be a problem if these functions are still unstable?

I would not mind here.

Comment on lines 12 to 13
/// `core::ptr::from_raw_parts_mut(slot, metadata)` must reference a valid
/// value owned by the caller. Furthermore, the layout returned by using
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that the value is necessarily owned by the caller. For example, I should be allowed to do the following:

fn foo(foo: &mut Foo) {
    let init /*: impl Init<Foo, Error = Infallible> */ = Foo::new();
    let ptr: *mut Foo = foo;
    unsafe { ptr::drop_in_place(ptr) };
    unsafe { init.init(ptr.cast::<()>()) };
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&mut Foo implies ownership. I think you meant the usual rule of handling *muts which is non-aliasing? Is it more specific?

/// value owned by the caller. Furthermore, the layout returned by using
/// `size_of` and `align_of` on this pointer must match what `Self::layout()`
/// returns exactly.
///
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, we should require that if T: Sized, Init::layout returns exactly Layout::new::<T>().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied

Comment on lines 27 to 28
/// The layout needed by this initializer, which the allocation
/// should arrange the destination memory slot accordingly.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// The layout needed by this initializer, which the allocation
/// should arrange the destination memory slot accordingly.
/// The layout needed by this initializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied

///
/// Note that `slot` should be thought of as a `*mut T`. A unit type is used
/// so that the pointer is thin even if `T` is unsized.
///
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're missing the part that if it returns Err, then the slot can be repurposed in any way (though it might have been written to).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied with additional comments

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I forgot to add the hunk to the change set.

I would like to make it more explicit. Maybe it will become a future OpSem question. Should we make the Err case to be, as if the slot is populated with poison value so that a read from this slot pointer is UB? In effect, the implementation would act as if, upon exiting with Err, a StorageDead is called on the entire slot.

I think we can defer it for further clarification. We can come back to update the documentation at any time before calling for stabilisation.

/// The layout needed by this initializer, which the allocation
/// should arrange the destination memory slot accordingly.
#[lang = "init_layout"]
fn layout(&self) -> crate::alloc::Layout;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming it's not allowed to have layout being larger than actually result T::Metadata, then they are actually duplicated information and one implies the other. This means we the results of layout() and init() introduce an invariant of the relationship between them. It also makes the resulting metadata depending on the dynamic result of init() while it should not.

Would it be better to only expose the layout/metadata in a single place, like replacing layout with fn dest_metadata(&self) -> T::Metadata; and make unsafe fn init(self, slot: *mut ()) -> Result<(), Self::Error>; or even with slot: *mut T to ease implementations?

Then the trait user would do let layout = Layout::for_value_raw(std::ptr::from_raw_parts(slot, init.ptr_metadata()));. It also self-explains the reason why dynamic layout is needed when we already have {size,align}_of*. It had this confusion on the first glace of the trait Init definition.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the metadata should be able to depend on what init does. For example, I could decide to initialize a dyn MyTrait with either Foo or Bar that is dynamically decided by init.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the metadata should be able to depend on what init does. For example, I could decide to initialize a dyn MyTrait with either Foo or Bar that is dynamically decided by init.

Then T = dyn MyTrait, and you need to branch in both layout and init anyway, and their results are expected to agree each other. But I agree slot: *mut () works better in this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay fair. But how do you get the alignment & size from the metadata? I didn't understand what you wrote above:

Then the trait user would do let layout = Layout::for_value_raw(std::ptr::from_raw_parts(slot, init.ptr_metadata()));.

You don't have slot at that point.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling layout on the same initializer must always return the same value, and init must initialize a value whose layout matches it exactly. So the choice of layout must happen before init runs, and it's not okay to have layout require size=8, align=8 and then have init write a [u8; 8] into the slot.

Copy link
Contributor Author

@dingxiangfei2009 dingxiangfei2009 Jun 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to ask for a clarification. Did you mean the other way around, like layout only requiring size=8 and align=1 like a [u8; 8] but then init write a size=8, align=8 value whose type has to work with align=8? That is the problematic case we want to reject.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That case is wrong too, but I meant what I said. The layout must be an exact match - it's not okay for Init::layout to return a layout that is too large or overaligned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay fair. But how do you get the alignment & size from the metadata? I didn't understand what you wrote above:

Then the trait user would do let layout = Layout::for_value_raw(std::ptr::from_raw_parts(slot, init.ptr_metadata()));.

You don't have slot at that point.

Oh, I didn't realize that there is no method to get a Layout from T: Pointee and a T::Metadata instance directly. We can forge a NULL pointer for Layout::for_value_raw, which meets its safety condition of having a valid metadata part. MIRI also passes on playground. Not sure if it worth an addition/change to Layout methods.

unsafe fn layout_from_meta<T: ptr::Pointee + ?Sized>(meta: T::Metadata) -> Layout {
    unsafe {
        // wait, if `for_value_raw` only care about ptr metadata, why doesn't it just
        // accept a `T::Metadata` parameter?
        Layout::for_value_raw(ptr::from_raw_parts::<T>(ptr::null::<()>(), meta))
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the comment to make it very specific.

Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>
@rust-log-analyzer

This comment has been minimized.

@dingxiangfei2009
Copy link
Contributor Author

@rustbot ready

Signed-off-by: Xiangfei Ding <dingxiangfei2009@protonmail.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants