jpernst / rental

Rust macro to generate self-referential structs
Apache License 2.0
211 stars 11 forks source link

There is no mutable counterpart for 'suffix' when using the 'covariant' argument. #41

Closed BattyBoopers closed 5 years ago

BattyBoopers commented 5 years ago

Consider the following code:

struct ContainsRef<'t>(
    &'t str,
    &'t mut str,
    // ... //
);

rental! {
    mod self_ref {
        use super::*;
        #[rental_mut(covariant)]
        pub struct Foo {
            data : String,
            borrow : ContainsRef<'data>,
        }
    }
}

fn suffix_mut<'t>(foo : &'t mut self_ref::Foo) -> &'t mut ContainsRef<'t> {
    unsafe {foo.all_mut_erased().borrow}
}

suffix_mut appears to be a sound interface to me, as it binds the returned reference to the lifetime of foo. I'd rather have this being generated as a method on foo when using #[rental_mut(covariant)] instead of messing around with unsafe myself.

jpernst commented 5 years ago

Unfortunately, this API would not be sound, since a mutable ref is not covariant over any lifetimes in its target type, so any such suffix_mut method can't safely coerce the suffix into an expressible lifetime.

BattyBoopers commented 5 years ago

Ok, maybe I'm missing something fundamental here. Could you give me another hint how I could cause UB with this construct?

I don't think the reference needs to be coerced into the lifetime of any particular field (which would surely be unexpressible), since all the fields live at least as long as the containing struct. While I hold the reference, I'm not allowed to borrow anything else from foo, so I don't see any of the borrowing rules violated here.

jpernst commented 5 years ago

For suffix_mut to work it needs to return a mutable ref. Since mutable refs are invariant over their target, whatever type is behind the mutable ref must be an exact match, including any lifetime params within that type. No variance is allowed. Since the rental field lifetimes are inexpressible outside of the struct, it's impossible to safely return such a reference.

For more information on lifetime variance and when and where you're allowed to apply it, see the nomicon chapter about it:

BattyBoopers commented 5 years ago

Thanks a lot! It took me quite some time to understand why this would be an unsound interface. I had read that chapter before but it's not the easiest... Anyways, of course it's an unsound interface:

#[macro_use]
extern crate rental; 

struct ContainsRef<'t>(
    &'t mut str,
);

rental! {
    mod self_ref {
        use super::*;
        #[rental_mut(covariant)]
        pub(super) struct Foo {
            data : String,
            borrow : ContainsRef<'data>,
        }
    }
}
use self_ref::Foo;

fn suffix_mut<'t>(foo : &'t mut Foo) -> &'t mut ContainsRef<'t> {
    unsafe {foo.all_mut_erased().borrow}
}

fn main() {
    let mut really_really_bad = {
        let mut victim = String::from("They won't even let me rest in peace after I die!");
        let mut desecrator = Foo::new(String::new(), |s| ContainsRef(s));
        let bad_reference = suffix_mut(&mut desecrator);
        bad_reference.0 = &mut victim;
        desecrator
    };
    let should_not_exist = suffix_mut(&mut really_really_bad);
    println!("Oooops: {}", should_not_exist.0);
}