Can't reference an FnMut from two struct instances


#1

Hi,
With the following code:
(See also in the playground here)

fn main() {
    let mut drop_counter = 0;
    struct IntWrap<F> (u32, Box<F>) where F : FnMut();

    impl<F> Drop for IntWrap<F> where F : FnMut() {
        fn drop(&mut self) {
            self.1();
        }
    }
    {
        let mut increase = || { drop_counter += 1; };
        let i1 = IntWrap(3, Box::new(&increase));
        let i2 = IntWrap(2, Box::new(&increase));
    }
    
    println!("drop_counter={}", drop_counter);
}

I get:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnMut`
  --> src/main.rs:11:28
   |
11 |         let mut increase = || { drop_counter += 1; };
   |                            ^^   ------------ closure is `FnMut` because it mutates the variable `drop_counter` here
   |                            |
   |                            this closure implements `FnMut`, not `Fn`
12 |         let i1 = IntWrap(3, Box::new(&increase));
   |                  ------- the requirement to implement `Fn` derives from here

Motivation:
I want to count the number of times all instances of a struct was dropped, in order to ensure I’m not creating circular references (actual code is much more complicated). It’s part of a unit test, not “real” code, and I’ve simplified it here.
But I can’t figure out how to do it. The way I tried above is by saving a closure inside the struct, where the closure increases the counter.

I think the problem is that, since the closure is mutating the environment, I can’t use it more than once, no matter what I do. Which means that what I want to do can’t be done at all. Which is sad :frowning:.

But… I could be wrong. But then again the error message isn’t helping much and looks like a bug. I mean, I’ve clearly stated that F implements FnMut, and yet somehow there’s a requirement (due to IntWrap, says the error message??) that the closure implements Fn.


#2

In short, use a Cell to store the drop counter: play

FnMut borrows itself mutably so you’d need a mutable borrow of the closure, while you have an immutable. But, as you said, you won’t be able to have both mutable borrows at the same time. So the Cell approach allows you to mutate the counter safely and have the closure shared.

P.S. on mobile so the above is terse - ask away if there’s something unclear.


#3

Also, in this particular example, you don’t need a Box to hold the closure. Not sure what your real code looks like, and you didn’t ask about this part, so kept that as-is in the playground :slight_smile:


#4

Thanks!
To spell out your solution (it took me a few seconds of head scratching to understand this fully):

Since the increase closure borrows its environment mutably (modifies the mutable drop_counter), it cannot be borrowed more than once (*).

So the trick you suggested is make drop_counter non-mutable. This is done by wrapping the actual value in a Cell.
This makes the closure to be Fn instead of FnMut, so now it’s ok to borrow it more than once.

Oh, the Box is just a part of my many changes I started making blindly around in order to solve this, you’re right that it’s not really necessary.

For reference, without the box, the struct in my code looks like this:

        struct IntWrap<'a, F> (u32, &'a F) where F : 'a+Fn();

        impl<'a, F> Drop for IntWrap<'a, F> where F : 'a+Fn() {
            fn drop(&mut self) {
                self.1();
            }
        }

I must say though that this was a really counter-intuitive solution, and I felt it was somehow inelegant… although it did the job :slight_smile:

Originally I did try going for the “internal mutability” solution myself, but I tried doing that by hiding the FnMut inside an Rc<RefCell<F>> wrapper. I still feel like it’s the more straight-forward solution.
That solved the problem of borrowing it more than once. However, when I tried unwrapping it using borrow_mut inside drop I got a compile error which I couldn’t see how to solve: cannot borrow immutable borrowed content as mutable.

After playing with this a bit more now, and looking carefully at the RefMut functions and their signatures, I found the deref_mut function. With it, I managed to write a version which uses FnMut, doesn’t use Cell, drop_counter is still mutable, and still it compiles and runs successfully.
Here it is:

use std::cell::RefCell;
use std::cell::RefMut;
use std::rc::Rc;
use std::ops::DerefMut;

fn main() {
    let mut drop_counter = 0;
    struct IntWrap<F> (u32, Rc<RefCell<F>>) where F : FnMut();

    impl<F> Drop for IntWrap<F> where F : FnMut() {
        fn drop(&mut self) {
            (RefMut::deref_mut(&mut self.1.borrow_mut()))();
        }
    }
    {
        let increase = || { drop_counter += 1; };
        let wrap = Rc::new(RefCell::new(increase));
        let _i1 = IntWrap(3, Rc::clone(&wrap));
        let _i2 = IntWrap(2, Rc::clone(&wrap));
    }
    
    println!("drop_counter={}", drop_counter);
}

Link to playground here

From the docs I get the feeling that deref_mut isn’t meant to be called directly, but from the examples there I think it was meant for mutable lvalues. In this case, the function call is an rvalue (I think? not a language lawyer) and so the automatic coercion to DerefMut doesn’t kick in and I had to specify it myself.

Thanks again for help, Vitaly :smile:


#5

(&mut *self.1.borrow_mut())();
It’s not intuitive. The immutable borrow is one of the misleading errors rustc gives as it tries multiple thing that fail but does not give other possibilities. Splitting over a few steps can help.

let m = self.1.borrow_mut();
let m = *m;
m();

error[E0507]: cannot move out of borrowed content
  --> src/main.rs:15:22
   |
15 |             let _m = *m;
   |                      ^^
   |                      |
   |                      cannot move out of borrowed content
   |                      help: consider using a reference instead: `&*m`

Which still does not quite suggest the correct fix.

Also can use wrap.clone()


#6

It can be just struct IntWrap<F>(u32, F) - you don’t need to force a reference there.

As for Rc<RefCell<F>>, yes, that works too. However, it’s inefficient and prone to runtime panics if you get the dynamic borrows wrong (there’re also some peculiarities about when a RefCell borrow gets dropped when you have it as an expression at the end of a block).

In this case, I think a Cell is the best approach.