Skip to content

Revamp the Reconstruct/ReconstructUnchecked traits, and add a construct. #92

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.

Sign up for GitHub

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
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
+47 −90
Open

Revamp the Reconstruct/ReconstructUnchecked traits, and add a construct. #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions rs_bindings_from_cc/test/struct/nonunpin/nonunpin_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
121 changes: 35 additions & 86 deletions support/ctor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -422,6 +425,18 @@ pub fn ctor_error<T: ?Sized, E>(e: E) -> impl Ctor<Output = T, Error = E> {
CtorError { e, marker: PhantomData }
}

/// Construct and return a Rust-movable value from a `Ctor`.
pub fn construct<T: Unpin>(ctor: impl Ctor<Output = T, Error = Infallible>) -> 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<T>`, allows emplacing a `Ctor` into
Expand Down Expand Up @@ -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.
///
Expand All @@ -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<T>::set(T)`.
pub trait ReconstructUnchecked: Sized {
/// # Safety
/// See trait documentation.
unsafe fn reconstruct_unchecked(
self: Pin<&mut Self>,
ctor: impl Ctor<Output = Self, Error = Infallible>,
) {
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<T>::set(T)`.
pub unsafe fn reconstruct<T>(p: Pin<&mut T>, ctor: impl Ctor<Output = T, Error = Infallible>) {
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<T> 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<Output = Self, Error = Infallible>) {
unsafe { self.reconstruct_unchecked(ctor) };
}
}

/// Safety: anything implementing `Unpin` is Rust-assignable, and
/// Rust-assignment is inherently destroy+reconstruct.
unsafe impl<T: Unpin> Reconstruct for T {}

/// Run f, aborting if it unwinds.
///
/// Because this aborts on unwind, f is not required to be unwind-safe.
Expand Down Expand Up @@ -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::<Self>() {
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));
}
}

Expand All @@ -1444,15 +1409,7 @@ where
T: Unpin + CtorNew<RvalueReference<'a, T>, Error = Infallible>,
{
fn assign(mut self: Pin<&mut Self>, src: RvalueReference<'a, Self>) {
if core::mem::needs_drop::<Self>() {
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));
}
}

Expand All @@ -1461,15 +1418,7 @@ where
T: Unpin + CtorNew<ConstRvalueReference<'a, T>, Error = Infallible>,
{
fn assign(mut self: Pin<&mut Self>, src: ConstRvalueReference<'a, Self>) {
if core::mem::needs_drop::<Self>() {
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));
}
}

Expand Down
12 changes: 10 additions & 2 deletions support/ctor_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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]
Expand Down Expand Up @@ -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);
}

Expand Down