Nicer self-ref structs

We think we finally solved it?

We could not get auto bounds to work (we tried) so this is as good as it gets :<

Any ideas if this is (un)sound?

Can you explain what we're looking at? A 600-line playground full of complicated generics and unsafe code deserves some context.

That's 500 lines of tests.

Fascinating experience… my 3 days old nightly compiler was too old to compile this code :sweat_smile:

8 Likes

After an initial look over the file, I’d like to remark that a thoroughly commented/explained example would probably help a lot with understanding what this is about and how it operates.

3 Likes

oh so that's why we couldn't get it to work before. XD

Admitted, the only change for older compilers is the need to add #![feature(explicit_generic_args_with_impl_trait)], and the error message explains it.

oh wait, so... the reason it doesn't work is because of our operate_in! workaround? (we didn't wanna write structs and OperateIn impls where we didn't need to... really wish we could get the closures to just work without this OperateIn stuff)

Here’s a soundness issue for you ^^

mod bad {
    use super::*;
    use std::rc::Rc;

    struct Foo<'a> {
        x: String,
        y: Rc<Cell<&'a str>>,
    }
    struct FooOpaque;
    opaque! {
        impl Opaque for FooOpaque {
            type Kind<'a> = Foo<'a>;
        }
    }
    impl<'a> SelfRef<'a> for Foo<'a> {}

    pub fn test() {
        let rc = Rc::new(Cell::new(""));
        let x = Holder::new_with(PhantomData::<FooOpaque>, |_| Foo {
            x: "Hello".to_owned(),
            y: rc.clone(),
        });
        let x = Box::pin(x);
        x.as_ref().operate_in(operate_in!(FooOpaque, (), |foo| {
            let foo = foo.get_ref();
            foo.y.set(&foo.x)
        }));
        dbg!(&rc);
        drop(x);
        dbg!(&rc); // use-after-free of the `String`
    }
}
fn main() {
    bad::test();
}
4 Likes

Ah so we are gonna need that proc macro... :‌(

(idea: given a struct like

selfref! {
  struct Foo<'a> {
   x: &'a Foo<'a>,
   y: &'a (),
  }
}

the proc macro should detect the existence of &'a Foo<'a> or perhaps get a hint that 'a is self-referential, and require that any uses of it are also self-referential, like so:

struct Foo<'a> where &'a Foo<'a>: SelfRef<'a>, &'a (): SelfRef<'a> {
  ...
}

which wouldn't compile because while there would be a blanket impl for &'a impl SelfRef<'a>, there would be no such thing for () (and by extension for &'a ()).

we almost wish the SelfRef trait was a lang item...)

yeahh we don't wanna deal with that proc macro.

okay, try it now? (we also got rid of some of the macros, they were... kinda pointless)

ngl really wish for for<'a> where 'k: 'a FnOnce(...) -> ...

You could use something like for<'a> FnOnce(..., [&'a &'k (); 0]) -> ... or for<'a> FnOnce(..., PhantomData<&'a &'k ()>) -> ....

we think we tried that and it didn't work

I see; I just tried myself, and it looks like you’re right.

indeed:

(Assuming the API is now sound in the first place,) I’m wondering if it’s still sound if Holder::operate_in introduces a lower bound on the lifetime 'a in terms of the 'i, i.e.

pub fn operate_in<'i, F, R>(self: Pin<&'i Self>, f: F) -> R
where
    F: OperateIn<'k, 'i, T, Output = R>, 

and

pub trait OperateIn<'k, 'i, T: Opaque + 'k> {
    type Output;

    fn operate_in<'a>(self, x: Pin<&'a T::Kind<'a>>) -> Self::Output
    where
        'k: 'a,
        'a: 'i;
}

(code untested)

This way it should become possible to do thing like e.g. return a (short-lived) reference to a field that doesn’t involve the self-referential lifetime from the Holder::operate_in call, which seems like quite a useful feature.

1 Like

nod...

meanwhile we're still wondering if the API is now sound in the first place XD

I just finished reading through the whole code and couldn’t come up with any more soundness issues so far.

1 Like

So we can fix the ergonomics, make this a lang item, and use it to implement async fn now, yeah? ^^ XD

Edit: We wonder if you rewrite known-problematic async-fn code to use this, if it still gets flagged up by miri?