Skip to content

Commit

Permalink
Attempt to fix cyclic re-borrow issue
Browse files Browse the repository at this point in the history
The fix for the issue described in #28 (reborrow_dependent_cyclic.rs) allows
compiling leak_dependent.rs which should not be possible. This is a first
attempt and incomplete as seen by the stubbed out impls for with_dependent and
with_dependent_mut.

DO NOT MERGE
  • Loading branch information
Voultapher committed Oct 11, 2021
1 parent f94e898 commit 090eac6
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 13 deletions.
33 changes: 21 additions & 12 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,21 +489,30 @@ macro_rules! self_cell {
unsafe { self.unsafe_self_cell.borrow_owner::<$Dependent<'_q>>() }
}

$Vis fn with_dependent<Ret>(&self, func: impl for<'_q> FnOnce(&'_q $Owner, &'_q $Dependent<'_q>) -> Ret) -> Ret {
unsafe {
func(
self.unsafe_self_cell.borrow_owner::<$Dependent>(),
self.unsafe_self_cell.borrow_dependent()
)
}
$Vis fn with_dependent<'outer_fn, Ret>(
&self,
func: impl for<'_q> FnOnce(&'_q $Owner, &'outer_fn $Dependent<'_q>
) -> Ret) -> Ret {
// unsafe {
// func(
// self.unsafe_self_cell.borrow_owner::<$Dependent>(),
// self.unsafe_self_cell.borrow_dependent()
// )
// }
panic!()
}

$Vis fn with_dependent_mut<Ret>(&mut self, func: impl for<'_q> FnOnce(&'_q $Owner, &'_q mut $Dependent<'_q>) -> Ret) -> Ret {
let (owner, dependent) = unsafe {
self.unsafe_self_cell.borrow_mut()
};
$Vis fn with_dependent_mut<'outer_fn, Ret>(
&mut self,
func: impl for<'_q> FnOnce(&'_q $Owner, &'outer_fn mut $Dependent<'_q>) -> Ret
) -> Ret {
panic!()

// let (owner, dependent) = unsafe {
// self.unsafe_self_cell.borrow_mut()
// };

func(owner, dependent)
// func(owner, dependent)
}

$crate::_covariant_access!($Covariance, $Vis, $Dependent);
Expand Down
2 changes: 2 additions & 0 deletions src/unsafe_self_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ impl<ContainedIn, Owner, DependentStatic> UnsafeSelfCell<ContainedIn, Owner, Dep
}
}

// Calling any of these *unsafe* functions with the wrong Dependent type is UB.

pub unsafe fn borrow_owner<'a, Dependent>(&'a self) -> &'a Owner {
let joined_ptr =
transmute::<NonNull<u8>, NonNull<JoinedCell<Owner, Dependent>>>(self.joined_void_ptr);
Expand Down
34 changes: 34 additions & 0 deletions tests/invalid/reborrow_dependent_cyclic.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use std::cell::RefCell;

use self_cell::self_cell;

struct Bar<'a>(RefCell<(Option<&'a Bar<'a>>, String)>);

self_cell! {
struct Foo {
owner: (),

#[not_covariant]
dependent: Bar,
}
}

fn main() {
let mut x = Foo::new((), |_| Bar(RefCell::new((None, "Hello".to_owned()))));

x.with_dependent(|_, dep| {
dep.0.borrow_mut().0 = Some(dep);
});

x.with_dependent_mut(|_, dep| {
let r1 = dep.0.get_mut();
let string_ref_1 = &mut r1.1;
let mut r2 = r1.0.unwrap().0.borrow_mut();
let string_ref_2 = &mut r2.1;

let s = &string_ref_1[..];
string_ref_2.clear();
string_ref_2.shrink_to_fit();
println!("{}", s); // prints garbage
});
}
2 changes: 1 addition & 1 deletion tests/self_cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl PackedAst {
self.ast_cell.borrow_owner()
}

fn with_ast(&self, func: impl for<'a> FnOnce(&'a String, &'a Ast<'a>)) {
fn with_ast<'o>(&self, func: impl for<'a> FnOnce(&'a String, &'o Ast<'a>)) {
self.ast_cell.with_dependent(func)
}

Expand Down

0 comments on commit 090eac6

Please sign in to comment.