From 3c5032d2babf97fdda91b13eb4730f14d91b93ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Tue, 31 Jan 2023 10:06:57 +0100 Subject: [PATCH 1/4] Add 'with()' and 'with_mut()' as associated functions to ErasedPtr This allows one to use ErasedPtr where rust won't allow a typed reference. For example in recursive types. --- crates/erasable/src/lib.rs | 48 ++++++++++++++++++++++++++++++++++ crates/erasable/tests/smoke.rs | 27 +++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/crates/erasable/src/lib.rs b/crates/erasable/src/lib.rs index 8b15078..3c4ca3e 100644 --- a/crates/erasable/src/lib.rs +++ b/crates/erasable/src/lib.rs @@ -185,6 +185,54 @@ pub unsafe trait ErasablePtr { /// /// The erased pointer must have been created by `erase`. unsafe fn unerase(this: ErasedPtr) -> Self; + + /// Run a closure on a borrow of the real pointer. Unlike the `Thin` wrapper this does + /// not carry the original type around. Thus it is required to specify the original impl + /// type when calling this function. + /// + /// ``` + /// # use {erasable::*, std::rc::Rc}; + /// let rc: Rc = Rc::new(123); + /// + /// let erased: ErasedPtr = ErasablePtr::erase(rc); + /// + /// let cloned = unsafe { + /// as ErasablePtr>::with(&erased, |rc| rc.clone()) + /// }; + /// + /// assert_eq!(*cloned, 123); + /// ``` + /// + /// The main purpose of this function is to be able implement recursive types that would + /// be otherwise not representable in rust. + /// + /// # Safety + /// + /// * The erased pointer must have been created by `erase`. + /// * The specified impl type must be the original type. + unsafe fn with(this: &ErasedPtr, f: F) -> T + where + Self: Sized, + F: FnOnce(&Self) -> T, + { + f(&ManuallyDrop::new(&Self::unerase(*this))) + } + + /// Run a closure on a mutable borrow of the real pointer. Unlike the `Thin` wrapper + /// this does not carry the original type around. Thus it is required to specify the + /// original impl type when calling this function. + /// + /// # Safety + /// + /// * The erased pointer must have been created by `erase`. + /// * The specified impl type must be the original type. + unsafe fn with_mut(this: &mut ErasedPtr, f: F) -> T + where + Self: Sized, + F: FnOnce(&mut Self) -> T, + { + f(&mut ManuallyDrop::new(&mut Self::unerase(*this))) + } } /// A pointee type that supports type-erased pointers (thin pointers). diff --git a/crates/erasable/tests/smoke.rs b/crates/erasable/tests/smoke.rs index 6452a08..8868792 100644 --- a/crates/erasable/tests/smoke.rs +++ b/crates/erasable/tests/smoke.rs @@ -32,3 +32,30 @@ fn thinning() { Thin::with_mut(&mut thin, |thin| *thin = Default::default()); let boxed = Thin::into_inner(thin); } + +#[test] +fn withfn() { + let boxed: Box = Default::default(); + + let erased: ErasedPtr = ErasablePtr::erase(boxed); + + unsafe { + as ErasablePtr>::with(&erased, |bigbox| { + assert_eq!(*bigbox, Default::default()); + }) + } +} + +#[test] +fn with_mut_fn() { + let boxed: Box = Default::default(); + + let mut erased: ErasedPtr = ErasablePtr::erase(boxed); + + unsafe { + as ErasablePtr>::with_mut(&mut erased, |bigbox| { + bigbox.0[0] = 123456; + assert_ne!(*bigbox, Default::default()); + }) + } +} From 1492d5f5773d1f66a26b89963f2e0356a80554ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 1 Feb 2023 18:23:26 +0100 Subject: [PATCH 2/4] Fix: ErasablePtr::with() * remove references of the unerased value * fix memory leaks in tests by unerase/drop the erased values --- crates/erasable/src/lib.rs | 3 ++- crates/erasable/tests/smoke.rs | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/erasable/src/lib.rs b/crates/erasable/src/lib.rs index 3c4ca3e..fa8f98a 100644 --- a/crates/erasable/src/lib.rs +++ b/crates/erasable/src/lib.rs @@ -201,6 +201,7 @@ pub unsafe trait ErasablePtr { /// }; /// /// assert_eq!(*cloned, 123); + /// # unsafe { as ErasablePtr>::unerase(erased)}; // drop it /// ``` /// /// The main purpose of this function is to be able implement recursive types that would @@ -215,7 +216,7 @@ pub unsafe trait ErasablePtr { Self: Sized, F: FnOnce(&Self) -> T, { - f(&ManuallyDrop::new(&Self::unerase(*this))) + f(&ManuallyDrop::new(Self::unerase(*this))) } /// Run a closure on a mutable borrow of the real pointer. Unlike the `Thin` wrapper diff --git a/crates/erasable/tests/smoke.rs b/crates/erasable/tests/smoke.rs index 8868792..62f07e1 100644 --- a/crates/erasable/tests/smoke.rs +++ b/crates/erasable/tests/smoke.rs @@ -34,7 +34,7 @@ fn thinning() { } #[test] -fn withfn() { +fn with_fn() { let boxed: Box = Default::default(); let erased: ErasedPtr = ErasablePtr::erase(boxed); @@ -44,6 +44,9 @@ fn withfn() { assert_eq!(*bigbox, Default::default()); }) } + + // drop it, otherwise we would leak memory here + unsafe { as ErasablePtr>::unerase(erased) }; } #[test] From 0288c33b4ce8bfafe97f6a3dac4938e4b12b2ab5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 1 Feb 2023 21:15:08 +0100 Subject: [PATCH 3/4] Fix: ErasedPtr::with_mut() - using scopeguard to write the pointer back into its original place - correct the ManuallyDrop() to wrap the actual value and take it out in the scopeguard --- crates/erasable/src/lib.rs | 7 ++++++- crates/erasable/tests/smoke.rs | 25 +++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/crates/erasable/src/lib.rs b/crates/erasable/src/lib.rs index fa8f98a..6fc5693 100644 --- a/crates/erasable/src/lib.rs +++ b/crates/erasable/src/lib.rs @@ -232,7 +232,12 @@ pub unsafe trait ErasablePtr { Self: Sized, F: FnOnce(&mut Self) -> T, { - f(&mut ManuallyDrop::new(&mut Self::unerase(*this))) + // SAFETY: guard is required to write potentially changed pointer value, even on unwind + let mut that = scopeguard::guard(ManuallyDrop::new(Self::unerase(*this)), |unerased| { + ptr::write(this, ErasablePtr::erase(ManuallyDrop::into_inner(unerased))); + }); + + f(&mut that) } } diff --git a/crates/erasable/tests/smoke.rs b/crates/erasable/tests/smoke.rs index 62f07e1..4e7859e 100644 --- a/crates/erasable/tests/smoke.rs +++ b/crates/erasable/tests/smoke.rs @@ -61,4 +61,29 @@ fn with_mut_fn() { assert_ne!(*bigbox, Default::default()); }) } + + // drop it, otherwise we would leak memory here + unsafe { as ErasablePtr>::unerase(erased) }; +} + +#[test] +fn with_mut_fn_replacethis() { + let boxed: Box = Default::default(); + + let mut erased: ErasedPtr = ErasablePtr::erase(boxed); + let e1 = erased.as_ptr() as usize; + unsafe { + as ErasablePtr>::with_mut(&mut erased, |bigbox| { + let mut newboxed: Box = Default::default(); + newboxed.0[0] = 123456; + *bigbox = newboxed; + assert_ne!(*bigbox, Default::default()); + }) + } + + let e2 = erased.as_ptr() as usize; + assert_ne!(e1, e2); + + // drop it, otherwise we would leak memory here + unsafe { as ErasablePtr>::unerase(erased) }; } From 25524cbf9ec6fab31ece02de831d0dee0c7c2d74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 1 Feb 2023 21:28:25 +0100 Subject: [PATCH 4/4] cleanup, no need for usize conversion (debug artifact) --- crates/erasable/tests/smoke.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/erasable/tests/smoke.rs b/crates/erasable/tests/smoke.rs index 4e7859e..b0f3c59 100644 --- a/crates/erasable/tests/smoke.rs +++ b/crates/erasable/tests/smoke.rs @@ -71,7 +71,7 @@ fn with_mut_fn_replacethis() { let boxed: Box = Default::default(); let mut erased: ErasedPtr = ErasablePtr::erase(boxed); - let e1 = erased.as_ptr() as usize; + let e1 = erased.as_ptr(); unsafe { as ErasablePtr>::with_mut(&mut erased, |bigbox| { let mut newboxed: Box = Default::default(); @@ -81,7 +81,7 @@ fn with_mut_fn_replacethis() { }) } - let e2 = erased.as_ptr() as usize; + let e2 = erased.as_ptr(); assert_ne!(e1, e2); // drop it, otherwise we would leak memory here