Debugging issues with traits, trait bounds, and closures

help
1

The issue[1]:

I can't clean-up / figure out - two of my code samples. Let's call them V7 and V8.

V7 Playground: Link
Lots of errors. Too many places where the issue may be.

V8 Playground: Link
This, I assume, was the "solution" to V7, but on closer inspection, it is not.

Stopping point 1:

V7.1 Playground: Link - Reduced errors to one: type annotations needed. if I comment out all the FnMut code, the error re-appears on the FnOnce code. - So something is overlapping.

help: try using a fully qualified path to specify the expected types
52 -     let v2 = w2.get_value()(1);
52 +     let v2 = <Wrap<'_, _> as GetValue<T>>::get_value(&w2)(1);

Sanity Check:
SC1 Playground: Link - Implement 3 closures with Fn, FnMut, FnOnce trait bounds. Looks OK to me.

Stopping point 2:
On a fluke, I tried combining all the trait implementations into one with multiple bounds -- since all three execute the same statement. [progress unlocked].

Sanity Check:
SC2 Playground: Link - A struct with Fn FnMut FnOnce closure traits -- No lifetimes needed. No dyn keyword needed. No type annotations on the trait definition needed. Big wins here.

Applying my learnings:
V7.2 Playground: Link - The FnMut and FnOnce closures are not taking, even though they has been checked dozens of times. -- But get this -- comment out the Fn() trait bound -- and everything works.

25 |     let f2 = |x: i32| {
   |              ^^^^^^^^ this closure implements `FnMut`, not `Fn`
26 |         n_copy += x;
   |         ------ closure is `FnMut` because it mutates the variable `n_copy` here
...
39 |     let v2 = w2.value.get_value(2);
   |              -------- --------- required by a bound introduced by this call
   |              |
   |              the requirement to implement `Fn` derives from here

V8.1 Playground: Link - I realized that in V8, the FnMut closure was not correct. Updating it. I ran into an issue with the borrow. But touching the borrows in V8.1 just blows everything up.

error[E0596]: cannot borrow data in a `&` reference as mutable
35 |     let v2 = w2.get_value()(1);
   |              ^^^^^^^^^^^^^^ cannot borrow as mutable

I need a better way to go about debugging V7.2 and V8.1. The whole Every function you define creates a unique, zero-sized type called a function item thing, adds a layer of difficulty in getting the trait bounds right.

So, given a closure X, how do you go about finding the trait bound (or implementation) that best matches X?
.
.
.
.
In hindsight, some compiler suggestions (triggered by user error) that I shouldn't have taken:

  • try borrowing.(&)
  • try adding a lifetime('a)
  • try wrapping in a Box::new()
  • try using dyn Fn()

  1. My apologies in advance, I know that versions of this question has been asked over and over again on the forum. ↩︎

2

At least given your first (V7) example, a workable implementation seems to be simpler than you're attempting. This compiles:

struct Wrap<'a, T> {
    value: &'a T,
}

trait GetValue<T> {
    fn get_value(&self) -> &T;
}

impl<'a, T> GetValue<T> for Wrap<'a, T> {
    fn get_value(&self) -> &T {
        &self.value
    }
}

fn main() {
    // Fn
    let f1 = || 1;
    // Fn Mut
    let f2 = |x: i32| {
        let x = x + 1;
        x
    };
    // FnOnce
    let f3 = |x: i32| {
        let y = x + 1;
        drop(x);
        y
    };
    let w1 = Wrap { value: &f1 };
    let w2 = Wrap { value: &f2 };
    let w3 = Wrap { value: &f3 };
    let v1 = w1.get_value()();
    let v2 = w2.get_value()(1);
    let v3 = w3.get_value()(2);
    println!("v1: {}", v1);
    println!("v2: {}", v2);
    println!("v3: {}", v3);
}

The secret sauce is that neither Wrap nor GetValue has any need to take notice of T being a function type. They can treat it like a completely opaque type. Only the clients of Wrap and GetValue - who set the type for T - care what T is.

You say that this is "not the solution" - why not?

3

I’m not sure what advice would best help you, because you haven’t said what your GetValue trait is meant to achieve — it’s all very abstract code, so I don’t know for sure whether anything that I might suggest is a good solution or defeating the point. That said, I’ll comment on the various things that you have shown:

As a general rule, if you have “too many errors”, the thing to do is to focus on the very first error reported by cargo check (or, sometimes, the very first warning — it depends) and solve that one alone, then re-check. Later errors may be influenced by previous ones and be misleading or entirely spurious. In any case, if you fix the first error, the compiler will often be able to give you better help for later errors.

When I look at your “V7” code, the first thing I see is that you are implementing traits for types containing dyn. This is often not the best plan; it is often (though not always) best to implement the trait for types that satisfy the trait bound, whether or not they are dyn types. That is, instead of writing, say,

impl Foo for Wrap<dyn Bar> {}

you would write

impl<T: ?Sized + Bar> Foo for Wrap<T> {}

This makes your trait implementation more flexible, because it can be used for non-dyn types, and because there are many different dyn types too (e.g. dyn Fn() is different from dyn Fn() + Sync).
That said, that advice doesn’t directly apply because it's unclear what the GetValue trait is supposed to be offering. It’s so generic that I don't know how it's supposed to work.

The second thing that I notice is that Wrap contains a reference &T, but you are implementing for Wrap<&T>, meaning that its value field is of type &&T. This is usually not what you want., because taking a reference to a reference is overly constraining. Here’s a version of your code that fixes that and contains satisfiable lifetime annotations:

impl<'a> GetValue<dyn Fn() + 'a> for Wrap<'a, dyn Fn() + 'a> {
    fn get_value(&self) -> &(dyn Fn() + 'a) {
        self.value
    }
}
impl<'a> GetValue<dyn FnMut() + 'a> for Wrap<'a, dyn FnMut() + 'a> {
    fn get_value(&self) -> &(dyn FnMut() + 'a) {
        self.value
    }
}
impl<'a> GetValue<dyn FnOnce() + 'a> for Wrap<'a, dyn FnOnce() + 'a> {
    fn get_value(&self) -> &(dyn FnOnce() + 'a) {
        self.value
    }
}

It still doesn’t compile and run, because the types you have put in Wrap in main() are not dyn types, but at least the trait impls are internally consistent. Let’s try making them generic like I suggested, and adjust things a bit to try to make it compile:

impl<'a, T: ?Sized + Fn()> GetValue<dyn Fn() + 'a> for Wrap<'a, T> {
    fn get_value(&self) -> &(dyn Fn() + 'a) {
        &self.value
    }
}
impl<'a, T: ?Sized + FnMut()> GetValue<dyn FnMut() + 'a> for Wrap<'a, T> {
    fn get_value(&self) -> &(dyn FnMut() + 'a) {
        &self.value
    }
}
impl<'a, T: ?Sized + FnOnce()> GetValue<dyn FnOnce() + 'a> for Wrap<'a, T> {
    fn get_value(&self) -> &(dyn FnOnce() + 'a) {
        &self.value
    }
}

This gives two new errors:

error[E0277]: expected a `FnMut()` closure, found `&'a T`
error[E0277]: expected a `FnOnce()` closure, found `&'a T`

These are because you simply cannot call a FnMut or FnOnce through an & reference — FnMut needs &mut, and FnOnce needs Box. Your main() code can never work as long as it wants to combine dyn and FnOnce and use the single GetValue trait.

Ah, I see you figured out writing generic implementations. I still don't know what your goal is, from this.

Every type that implements FnMut also implements FnOnce, and every type that implements Fn also implements FnMut. Therefore, given a FnMut and an un-annotated call to .get_value(), it is ambiguous whether you want the I32Mut impl or the I32Once impl. (The compiler doesn’t care that calling either one would produce the same result.)

This is just the same as writing F: Fn(i32), because the other bounds are already implied.

f3 implements FnMut, not just FnOnce. So, when you comment out the Fn bound, all three cases can satisfy the FnMut trait bound. (In order to make a closure that is FnOnce only, the closure must move. not copy, something out of itself — it must give away a part of itself, so that it cannot execute a second time. For example, { let s = String::from("hi"); || s } produces a FnOnce closure, because s is given away by the closure.

This one fails because you can't call a FnMut through an & reference.

I would say it is better to write the function trait bound your generic code needs, and then write closures that satisfy it. Adjust the bound when you find that a change is necessary, but try to make sure to understand what the change actually means. For example, you need to know that F: FnMut means that the function can only be called with &mut F, not &F.

And, finally, as I said at the top, if you tell us more about what your actual goal is, we’ll be able to help you write code that actually satisfies that goal.

1 Like
4

Thank you for posting that. I'm reading and re-reading the diff to make sure I understand it.

***************
*** 1 ****
! struct Wrap<'a, T: ?Sized> {
--- 1 ----
! struct Wrap<'a, T> {
***************
*** 5 ****
! trait GetValue<T: ?Sized> {
--- 5 ----
! trait GetValue<T> {
***************
*** 8,19 ****
! impl GetValue<dyn Fn()> for Wrap<'_, &dyn Fn()> {
!     fn get_value(&self) -> &dyn Fn() {
!         &self.value
!     }
! }
! impl GetValue<dyn FnMut()> for Wrap<'_, &dyn FnMut()> {
!     fn get_value(&self) -> &dyn FnMut() {
!         &self.value
!     }
! }
! impl GetValue<dyn FnOnce()> for Wrap<'_, &dyn FnOnce()> {
!     fn get_value(&self) -> &dyn FnOnce() {
--- 8,10 ----
! 
! impl<'a, T> GetValue<T> for Wrap<'a, T> {
!     fn get_value(&self) -> &T {

The secret sauce is that neither Wrap nor GetValue has any need to take notice of T being a function type. They can treat it like a completely opaque type. Only the clients of Wrap and GetValue - who set the type for T - care what T is.

I need to keep this in mind. Somewhere down the line when I first had to deal with this, I probably got the notion to handle each closure type individually.

So the take-away for me is - I need to better understand the intersection between generics and traits.

5

Thank you for the step-by-step on V7. The ~making things consistent in this one part~ was a helpful thing to remember.

This is just the same as writing F: Fn(i32), because the other bounds are already implied.

I never would have guessed that I could just write that.

And, finally, as I said at the top, if you tell us more about what your actual goal is, we’ll be able to help you write code that actually satisfies that goal.

I think these two code experiments are from when I was dealing with some wasm_bindgen closure issues. I was recently linting / formating a bunch of old experiments - but I got stuck on clearing errors on these two - which sent me down a rabbit hole.

This one fails because you can't call a FnMut through an & reference.

I just don't have these things memorized, and I need to.

I think the deal with V7 and V8, the more I deal with them, is that I naively applied complier suggestions I didn't need. I thought I needed those & references, but after doing SC2, I probably don't.

6

The code does compile, but the V7 FnMut block was incorrect. When changed to an actual FnMut -- let mut n = 10; let f2 = |x| { n += x; n } It gets the same error as V8.1 cannot borrow data in a & reference as mutable Which kpreid already mentioned is not possible.

7

Yeah. Borrowing function types seriously constrains how you can call them, and while that doesn't matter Fn calls, it does matter for FnOnce and FnMut calls. The only way to support all three is to give up ownership of the function value when it's called (since FnOnce consumes its function argument), and if you rule out FnOnce, you still have to give up a mutable reference if you need to support FnMut (since it mutates the function argument).

If you're looking for a way to store a function in a struct and call it later, without constraining the types of functions allowed, I'd keep it as simple as possible and remove most, if not all, of the traits and impls you've added:

struct Wrap<T> {
  value: T,
  // other fields as appropriate
}

fn main() {
    let mut x = 5;
    let fx = move || { x += 1; x };

    let mut v = Wrap {
      value: fx,
    };

    assert_eq!(6, (v.value)()); // requires v to be mut, since it mutates v.value
    assert_eq!(7, (v.value)());
}

The trait is only necessary if there's some code elsewhere that uses the trait as a constraint or dyn type. I would assume that you have such a constraint, so if you can show us what that code looks like, we might be able to offer a more targeted suggestion.

1 Like
8

The code, looking back on it, seems related to wanting an abstraction over creating wasm_bindgen closures because in JS you don't have the Fn, FnMut, FnOnce -thing like rust.

The final code that is currently in use based on the 2022 topic linked above:

pub trait ToClosureFnOnce<Dyn: ?Sized> {
    fn to_closure_fnonce(self) -> Closure<Dyn>; }
impl<F> ToClosureFnOnce<dyn FnMut()> for F
where
    F: FnOnce() + 'static, {
    fn to_closure_fnonce(self) -> Closure<dyn FnMut()> {
        let bx_closure = Box::new(self) as Box<dyn FnOnce()>;
        Closure::once(bx_closure)}}

pub trait ToClosureFnMut<Dyn: ?Sized> {
    fn to_closure_fnmut(self) -> Closure<Dyn>; }
impl<F> ToClosureFnMut<dyn FnMut()> for F
where
    F: FnMut() + 'static, {
    fn to_closure_fnmut(self) -> Closure<dyn FnMut()> {
        let bx_closure = Box::new(self) as Box<dyn FnMut()>;
        Closure::wrap(bx_closure)}}
impl<F, A, R> ToClosureFnMut<dyn FnMut(A) -> R> for F
where
    F: FnMut(A) -> R + 'static,
    A: wasm_bindgen::convert::FromWasmAbi + 'static,
    R: wasm_bindgen::convert::IntoWasmAbi + 'static, {
    fn to_closure_fnmut(self) -> Closure<dyn FnMut(A) -> R> {
        let bx_closure = Box::new(self) as Box<dyn FnMut(A) -> R>;
        Closure::wrap(bx_closure)}}

-- that is called like this (only 3 of many listed):

   //
   let closure = closure_r.to_closure_fnmut();
   let closure_ref = closure.as_ref().unchecked_ref();
   a1_htmlanchorelement.set_onclick(Some(closure_ref));
   closure.forget();
   //
   let closure_1 = closure_r_1.to_closure_fnonce();
   let closure_ref_1 = closure_1.as_ref().unchecked_ref();
   set_timeout(closure_ref_1, 100);
   closure_1.forget();
   //
   let slides_2 = slides.clone();
   let closure_r_2 = move || {
       load_slide(&slides_2, index);
   };
   let closure_2 = closure_r_2.to_closure_fnmut();

What I wanted was a way to define 1 closure method for all my callbacks. Ultmately, I settled for two traits and three implementations. Experiments V7 and V8 (among others) were my attempts to get one trait/method to handle all closure scenarios.