diff --git a/rs_bindings_from_cc/test/struct/nonunpin/nonunpin_test.rs b/rs_bindings_from_cc/test/struct/nonunpin/nonunpin_test.rs index b7fe01868..2c6999678 100644 --- a/rs_bindings_from_cc/test/struct/nonunpin/nonunpin_test.rs +++ b/rs_bindings_from_cc/test/struct/nonunpin/nonunpin_test.rs @@ -3,7 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #![feature(negative_impls)] -use ctor::{ctor, emplace, CtorNew, ReconstructUnchecked}; +use ctor::{ctor, emplace, CtorNew}; use googletest::prelude::*; use nonunpin::{Nonmovable, Nonunpin, NonunpinStruct, ReturnsNonmovable}; use std::pin::Pin; @@ -92,7 +92,7 @@ fn test_union_field() { })); assert_eq!(my_union.cxx_class.value(), 4); std::mem::ManuallyDrop::drop(&mut Pin::into_inner_unchecked(my_union.as_mut()).cxx_class); - my_union.as_mut().reconstruct_unchecked(ctor!(MyUnion { int: 2 })); + ctor::reconstruct(my_union.as_mut(), ctor!(MyUnion { int: 2 })); assert_eq!(my_union.int, 2); } } diff --git a/support/ctor.rs b/support/ctor.rs index b8e450d80..b87385cdc 100644 --- a/support/ctor.rs +++ b/support/ctor.rs @@ -208,7 +208,10 @@ pub unsafe trait Ctor: Sized { /// /// # Safety /// - /// `dest` is valid for writes, pinned, and uninitialized. + /// `dest` is valid for writes and uninitialized. + /// + /// This function pins `dest`, so unless `Output: Unpin`, dest must not be moved or otherwise + /// invalidated. unsafe fn ctor(self, dest: *mut Self::Output) -> Result<(), Self::Error>; /// Converts a `Ctor` with error type `Infallible` to one with the provided error type `E`. @@ -422,6 +425,18 @@ pub fn ctor_error(e: E) -> impl Ctor { CtorError { e, marker: PhantomData } } +/// Construct and return a Rust-movable value from a `Ctor`. +pub fn construct(ctor: impl Ctor) -> T { + let mut value = MaybeUninit::uninit(); + // SAFETY: `value` is valid for writes and uninitialized, and `T` is `Unpin`. + unsafe { + ctor.ctor(value.as_mut_ptr()).unwrap(); + } + + // SAFETY: `ctor.ctor()` initialized `value`. + unsafe { value.assume_init() } +} + /// Trait for smart pointer types which support initialization via `Ctor`. /// /// A typical example would be `Box`, allows emplacing a `Ctor` into @@ -1278,6 +1293,8 @@ macro_rules! ctor { /// Destroy-then-reconstruct. Sidesteps `operator=`, instead reconstructing /// in-place. /// +/// For Rust-movable types, this is equivalent to `*p.as_mut().into_inner() = construct(ctor)`. +/// /// If the object cannot be destroyed/reconstructed in place (e.g. it is a base /// class subobject), the behavior is undefined. /// @@ -1289,76 +1306,32 @@ macro_rules! ctor { /// assignment. /// /// That means that e.g. instead of calling `x.assign(&*emplace!(foo))`, you can -/// directly call `x.reconstruct_unchecked(foo)` -- provided you are OK with the +/// directly call `reconstruct(x, foo)` -- provided you are OK with the /// differing constructor/destructor ordering, and satisfy safety criteria. /// /// # Safety /// -/// Dividing sources of UB by language ruleset: -/// -/// **C++**: The behavior is undefined if `self` is a base class subobject +/// The behavior is undefined if `p` is a base class subobject /// or `[[no_unique_address]]` member. /// /// See: http://eel.is/c++draft/basic.life#8 /// /// And see https://chat.google.com/room/AAAAl3r59xQ/auNOifgQa1c for discussion. /// -/// **Rust**: This is safe. Note that since this calls `drop()` on -/// the pinned pointer, it satisfies the pin guarantee, and is allowed to then -/// re-init it with something else. In effect, this is just the in-place Ctor -/// version of the existing method `Pin::set(T)`. -pub trait ReconstructUnchecked: Sized { - /// # Safety - /// See trait documentation. - unsafe fn reconstruct_unchecked( - self: Pin<&mut Self>, - ctor: impl Ctor, - ) { - let self_ptr = Pin::into_inner_unchecked(self) as *mut _; - core::ptr::drop_in_place(self_ptr); - abort_on_unwind(move || { - ctor.ctor(self_ptr).unwrap(); - }); - } +/// If `p` refers to an object stored anywhere else, such as a (non-`no_unique_address`) +/// field of a struct, or a local variable, then this is safe. +/// +/// Note that since this calls `drop()` on the pinned pointer, it satisfies the pin +/// guarantee, and is allowed to then re-init it with something else. In effect, this +/// is just the in-place Ctor version of the existing method `Pin::set(T)`. +pub unsafe fn reconstruct(p: Pin<&mut T>, ctor: impl Ctor) { + let raw_ptr = Pin::into_inner_unchecked(p) as *mut _; + core::ptr::drop_in_place(raw_ptr); + abort_on_unwind(move || { + ctor.ctor(raw_ptr).unwrap(); + }); } -impl ReconstructUnchecked for T {} - -/// Destroy-then-reconstruct. Sidesteps `operator=`, instead reconstructing -/// in-place. -/// -/// If `ctor` unwinds, the process will crash. -/// -/// This is a bit more fragile than, and a lot less common than, `operator=`, -/// but allows for taking advantage of copy/move elision more aggressively, -/// rather than requiring materialization into a temporary before triggering -/// assignment. -/// -/// That means that e.g. instead of calling `x.assign(&*emplace!(foo))`, you can -/// directly call `x.reconstruct(foo)` -- provided you are OK with the differing -/// constructor/destructor ordering. -/// -/// # Implementation safety -/// -/// Implementors must ensure that it is safe to destroy and reconstruct the -/// object. Most notably, if `Self` is a C++ class, it must not be a base class. -/// See http://eel.is/c++draft/basic.life#8 -/// -/// # Safety -/// -/// Note that this is not safe to call on `[[no_unique_address]]` member -/// variables, but the interface is marked safe, because those can only be -/// produced via unsafe means. -pub unsafe trait Reconstruct: ReconstructUnchecked { - fn reconstruct(self: Pin<&mut Self>, ctor: impl Ctor) { - unsafe { self.reconstruct_unchecked(ctor) }; - } -} - -/// Safety: anything implementing `Unpin` is Rust-assignable, and -/// Rust-assignment is inherently destroy+reconstruct. -unsafe impl Reconstruct for T {} - /// Run f, aborting if it unwinds. /// /// Because this aborts on unwind, f is not required to be unwind-safe. @@ -1427,15 +1400,7 @@ where T: Unpin + CtorNew<&'a T, Error = Infallible>, { fn assign(mut self: Pin<&mut Self>, src: &'a Self) { - if core::mem::needs_drop::() { - let mut constructed = MaybeUninit::uninit(); - unsafe { - T::ctor_new(src).ctor(constructed.as_mut_ptr()).unwrap(); - *self = constructed.assume_init(); - } - } else { - self.reconstruct(T::ctor_new(src)); - } + *self = construct(T::ctor_new(src)); } } @@ -1444,15 +1409,7 @@ where T: Unpin + CtorNew, Error = Infallible>, { fn assign(mut self: Pin<&mut Self>, src: RvalueReference<'a, Self>) { - if core::mem::needs_drop::() { - let mut constructed = MaybeUninit::uninit(); - unsafe { - T::ctor_new(src).ctor(constructed.as_mut_ptr()).unwrap(); - *self = constructed.assume_init(); - } - } else { - self.reconstruct(T::ctor_new(src)); - } + *self = construct(T::ctor_new(src)); } } @@ -1461,15 +1418,7 @@ where T: Unpin + CtorNew, Error = Infallible>, { fn assign(mut self: Pin<&mut Self>, src: ConstRvalueReference<'a, Self>) { - if core::mem::needs_drop::() { - let mut constructed = MaybeUninit::uninit(); - unsafe { - T::ctor_new(src).ctor(constructed.as_mut_ptr()).unwrap(); - *self = constructed.assume_init(); - } - } else { - self.reconstruct(T::ctor_new(src)); - } + *self = construct(T::ctor_new(src)); } } diff --git a/support/ctor_test.rs b/support/ctor_test.rs index 97c5d5324..f9596d4d0 100644 --- a/support/ctor_test.rs +++ b/support/ctor_test.rs @@ -8,7 +8,7 @@ use ctor::{ copy, ctor, emplace, mov, try_emplace, Assign, Ctor, CtorNew, Emplace, FnCtor, - ManuallyDropCtor, Reconstruct, RecursivelyPinned, RvalueReference, Slot, UnreachableCtor, + ManuallyDropCtor, RecursivelyPinned, RvalueReference, Slot, UnreachableCtor, }; use googletest::gtest; use std::cell::RefCell; @@ -18,6 +18,12 @@ use std::pin::Pin; use std::rc::Rc; use std::sync::{Arc, Mutex}; +#[gtest] +fn test_construct() { + let x = ctor::construct(u32::ctor_new(())); + assert_eq!(x, 0); +} + /// Only really need one test for the new super-let syntax, as it uses the same /// building blocks as the old syntax. #[gtest] @@ -259,7 +265,9 @@ fn test_ctor_macro_union() { // First, should drop myunion.x. unsafe { ManuallyDrop::drop(&mut Pin::into_inner_unchecked(my_union.as_mut()).x) } // Second, we can overwrite. - my_union.as_mut().reconstruct(ctor!(MyUnion { y: 24 })); + unsafe { + ctor::reconstruct(my_union.as_mut(), ctor!(MyUnion { y: 24 })); + } assert_eq!(unsafe { my_union.y }, 24); }