Trying to impl trait with FnOnce parameter but 'cannot infer an appropriate lifetime'

help
#1

Hello and thanks in advance :slight_smile:

I am trying to get the the trait in the following simplified example implemented, but it fails with 'cannot infer an appropriate lifetime'.

The general intention is to implement a trait which passes a FnOnce that expects to be called with an mutable reference to an instance of the same type (ideal trait notation - in terms of simplicity) :

trait DoStuff {
    fn do_with_magic_computation_scope<R, F: FnOnce(&mut Self) -> R>(&mut self, f: F) -> R;
}

I want the trait to be implemented on a Context struct that has a reference to a buffer and a scope variable.

In the actual implementation the new value for the scope variable depends on the function of the trait that was called and other parameters. The Context owns the buffer and the scope is changed before and after calling the callback with mem::replace (so two calls per callback call). But this is too slow, thus the idea of utilizing the stack where only the buffer-reference has to be 'moved' (so far the idea). Ideally/in the next iteration, the context would no longer own but reference the scope variable (Option<&Scope>) as well.

I cannot get the lifetimes figured out in the example above - if this is possible at all. I would prefer to adjust the trait as little as possible to not leak implementation details onto the interface.

#2

I may be wrong but I think line 32 you return a reference that outlives what it's referring to

Edit : I added a few comments, what do you think ?

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=0988ea42ce1f705611321c4396b1dff4

#3

It really helps to expand out all the lifetimes to understand the problem. These errors are usually due to overly restrictive requirements. Here's a version that expands on what @Teln0 did but taking it further

Playground

The problematic code is this

pub struct Context<'a> {
    buffer: &'a mut Vec<u8>,
    scope: Option<usize>,
}

impl<'a> Context<'a> {
    fn scope_pushed<R, F>(self: &'a mut Context<'a>, f: F, scope: usize) -> R
    where
        for<'b> F: FnOnce(&'b mut Context<'a>) -> R
    {
        let mut context = Context {
            buffer: self.buffer,
            scope: Some(scope),
        };
        f(&mut context)
    }
}

Context holds a borrow of lifetime 'a. You've also specified through the use of &'b Self on F which I've expanded to &'b mut Context<'a> which is whats implied. This means that your function is expecting an argument Context<'a>. This works if you were passing in the original first argument (self) to scope_pushed. However you are instead passing in a new Context object. And this new object actually borrows from Context in which case the lifetime of the buffer argument must be something less than 'a because its borrowing ownership from self: Context<'a>. In light of this, a solution to this is to reuse the argument of self rather than creating a new context object.

Playground

impl<'a> Context<'a> {
    fn scope_pushed<'c, R, F>(&'c mut self, f: F, scope: usize) -> R
    where
        for<'b> F: FnOnce(&'b mut Self) -> R
    {
        self.scope = Some(scope);
        let res = f(self);
        self.scope = None;
        res
    }
}

Here we're passing in an exclusive reference to self which is Context<'a> and just reusing it for the call to f(..). Since we have exclusive access to self, its fine to assign scope temporarily for the function call.

The other thing that I changed that you can see more obviously in the prior example is the lifetime of the reference to a new lifetime ('c) associated with the method rather than reuse the 'a from the trait. The basic idea behind this change is that what you had was effectively &'a mut Context<'a>. This usually is overly restrictive because it requires the lifetime of the borrow of the Context struct to exactly match the lifetime of the borrow of the data in the struct. By making the lifetimes different, the compiler allows for those two lifetime to be different which is usually what you want.

1 Like
#4

Thanks for your responses @Telno and @drewkett. But this doesn't solve my issue. I don't want to overwrite self.scope, because I would have to restore it after the call as well. In my current version this is already implemented like so:

        let old = core::mem::replace(&mut self.scope, Some(scope));
        let res = f(self);
        let scope = core::mem::replace(&mut self.scope, old);
        debug_assert!(scope.unwrap()....);
        res

But the calls to replace are quite visible in the flamegraph, because the real scope is with >=32 bytes also quite large and each call invokes two replaces. Ideally, I want this simplified version to work playground. The new scope instance is create within the trait call on the stack anyway, so re-packaging it and borrowing over the buffer should be fast and easy for the compiler to optimize away any memcopy(?)

#5

Can't you use something like a refcell for your buffer ? A bit of overhead but nothing like copying 32 bytes. And you won't have to worry about lifetimes : an object is destroyed when there's 0 references to it.

#6

Does this need to be a trait? The basic problem is that the type of the temporary context is not the same as the original context because of lifetimes. Without the trait it would look something like this

impl <'a> Context<'a>{
    fn do_with_magic_computation_scope<R, F: for <'b>FnOnce(&mut Context<'b>) -> R>(&mut self, f: F) -> R {
        let magic_computation_result = 3;
        f(&mut Context {
            buffer: self.buffer,
            scope: Some(magic_computation_result),
        })
    }
}

If you want it to apply to multiple context types, you could use a macro.

I may be wrong but I don’t believe there’s a way to define this specific method on a trait because it looks like it requires higher kinded types.

Another option is to instead put the computation result in a Box. Then the method of copying in and copying back out of the context object will only copy a single pointer. If the computation result is a large amount of data this very well may be faster since the compiler may not optimize away all mem copies in the other approaches

Playground

impl<'a> Context<'a> {
    fn scope_pushed<R, F>(self: &mut Context<'a>, f: F, scope: Box<usize>) -> R
    where
        for<'b> F: FnOnce(&'b mut Context<'a>) -> R
    {
        let orig = std::mem::replace(&mut self.scope,Some(scope));
        let res = f(self);
        self.scope = orig;
        res
    }
}
1 Like
#7

Not implementing the trait is not an option. I also played around with using a Vec to push/pop the Scope instead, but it was slightly slower. Using Box is quite a bit slower. I guess I have to live with it or get the Scope struct smaller.

Thanks for your effort @drewkett and @Teln0 :slight_smile:

1 Like
#8

Does:

pub struct Context<'a> {
    buffer: &'a mut Vec<u8>,
    scope: Option<usize>,
}

trait DoStuff<'reborrow> {
    type ReborrowedSelf;

    fn do_with_magic_computation_scope<R, F> (
        self: &'reborrow mut Self,
        f: F,
    ) -> R
    where
        F : FnOnce(&'_ mut Self::ReborrowedSelf) -> R,
    ;
}

impl<'a, 'reborrow> DoStuff<'reborrow> for Context<'a> {
    type ReborrowedSelf = Context<'reborrow>;

    fn do_with_magic_computation_scope<R, F> (
        self: &'reborrow mut Self,
        f: F,
    ) -> R
    where
        F : FnOnce(&'_ mut Context<'reborrow>) -> R,
    {
        let magic_computation_result = 3;
        f(&mut Context {
            buffer: self.buffer,
            scope: Some(magic_computation_result),
        })
    }
}

suit you?

2 Likes
#9

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.