How do I make my callback parameter generic?

I am creating a struct that holds a bool and a callback. I want to be able to initialize the callback member of the struct with a closure that takes a single parameter of generic type and returns a bool. I tried this:

//! Edge Detection module

/// Input that detects when an input goes from low (false) to high (true)
/// Generic for callback CB and parameter P

pub struct EdgeTrigger<P, CB> where CB: Fn(P) -> bool {
    pub prev_state: bool,
    pub callback: CB,
} 

I got this error from rustc:

error[E0392]: parameter `P` is never used
 --> src/edge.rs:6:24
  |
6 | pub struct EdgeTrigger<P, CB> where CB: Fn(P) -> bool {
  |                        ^ unused parameter
  | 
  = help: consider removing `P`, referring to it in a field, or using a marker such as `core::marker::PhantomData`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0392`.

I was able to get this to compile instead:

//! Edge Detection module
use crate::chip::Chip;

/// Input that detects when an input goes from low (false) to high (true)
/// Generic for callback CB that takes a &Chip parameter

pub struct EdgeTrigger<CB> where CB: Fn(&Chip) -> bool {
    pub prev_state: bool,
    pub callback: CB,
}

But that is not what I want. I want to have signal edge detection using callback functions that take a parameter of whatever type is needed, not just &Chip. What do you all suggest?

Thanks,
David H

The error message suggests using PhantomData, which looks like this:

use std::marker::PhantomData;

pub struct EdgeTrigger<P, CB> where CB: Fn(P) -> bool {
    pub prev_state: bool,
    pub callback: CB,
    _data: PhantomData::<P>,
}

Note that this means that an EdgeTrigger<Foo, _> can only store a closure that takes a Foo. You can't have a generic closure in Rust, currently.

1 Like

Minor improvement over @mbrubeck's solution: use PhantomData<fn(P) -> bool> rather than PhantomData<P> (this, for instance, will allow a struct where P = &'a str to be used where P = &'static str is expected)

3 Likes

Or even better, don't specify trait bounds on the type, but on the methods that need them. This almost always reduces the complexity of your code, makes the type easier to use and more generally applicable.

For example, in std: Cell has no trait bounds on the type, but you need to implement Copy or Default to use it nicely. HashMap doesn't require Hash + Eq until you try and insert or remove elements. You can even create a HashMap that you can't use because HashMap::new doesn't require Hash + Eq. This is an explicit design decision in order to make HashMap easier to use (especially in generic code).

So do this:

pub struct EdgeTrigger<CB> {
    pub prev_state: bool,
    pub callback: CB,
}

One caveat: if you need to use the trait impls on Drop, then you must put those trait bounds on your type, but barring that, don't put trait bounds on types. It just leads to unnecessary headaches.

8 Likes

Thank you. I tried your suggestion and it got that module to compile. However, when my app calls the triggered() method on that struct I now get borrow-related compile errors I didn't get with the more concrete version.

Relevant method:

impl<P, CB> EdgeTrigger<P, CB> where CB: Fn(P) -> bool {
    /// Return true if the input goes from low to high
    pub fn triggered(&mut self, param: P) -> bool {
        let new_state = (self.callback)(param);
        let result = !self.prev_state && new_state;
        self.prev_state = new_state;
        result
    }
}

Code where it is used, most stuff snipped out for brevity:

    let mut onehertz = EdgeTrigger::new(true,
                                        |chip: &Chip| !chip.get_d6());
...
    loop {
        chip.sercom.write_bytes(b"\r\n");
...
        'clock_mode: loop {
            while !onehertz.triggered(&chip) {
                }
            }
        }
...
    }

The compiler errors:

error[E0502]: cannot borrow `chip.sercom` as mutable because it is also borrowed as immutable
   --> src/bin/clockctl.rs:170:9
    |
170 |         chip.sercom.write_bytes(b"\r\n");
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mutable borrow occurs here
...
177 |             while !onehertz.triggered(&chip) {
    |                    --------           ----- immutable borrow occurs here
    |                    |
    |                    immutable borrow later used here

The chip.sercom.write_bytes() method call results in a mutable borrow of chip while the .triggered() method call requires an immutable reference, and it doesn't like that. But of course passing a mutable reference (&mut Chip) to .triggered() would only make things worse as you can't have two mutable references to the same thing.

The non-generic version of this code, where the callback was explicitly declared to take a &Chip parameter, didn't have these borrow problems.

Anything obviously foolish that jumps out from the (admittedly brief) code?

In case anyone is curious, the entire code is here: https://bitbucket.org/dhandy2013/robotics/src/master/circuit-playground/rust/

src/edge.rs is the implementation of EdgeTrigger that works.

src/edge2.rs, etc. are the experiments I'm trying.

src/bin/clockctl.rs is the app using it.

Basically you want that for P = &'_ Chip, the bound become for<'any> Fn(&'any Chip) -> bool, i.e., Fn(&'_ Chip) -> bool, which is a higher-rank trait bound (HRTB).
But given you genericity over P, in your case this becomes P = &'unknown_but_fixed Chip and Fn(&'unknown_but_fixed Chip) -> bool, meaning that all the borrows in your loop must be using the same lifetime, which is not possible, hence the error.


Try @RustyYato's suggestion for your case:

pub
struct EdgeTrigger<CB> {
    prev_state: bool,
    callback: CB,
}

impl<CB> EdgeTrigger<CB> {
    pub
    fn triggered<P> (self: &'_ mut Self, param: P)
      -> bool
    where
        CB : Fn(P) -> bool,
    {
        let new_state = (self.callback)(param);
        ::core::mem::replace(&mut self.prev_state, new_state) && new_state
    }
}
1 Like

No way! I tried that... and it actually worked!

No strange PhantomData required.

No obscure borrow checker errors popped up that weren't there before.

Wow, my mind is blown. I wouldn't have thought to put constraints on the generic types on the methods that weren't present on the struct. I didn't know you could do that.

Here's what that looks like, for anyone else reading this later:

impl<CB> EdgeTrigger<CB> {
    /// init_state: The initial value of the prev_state field
    /// callback: Closure that reads input and returns its value
    pub fn new(prev_state: bool, callback: CB) -> EdgeTrigger<CB> {
        EdgeTrigger { prev_state, callback }
    }

    /// Return true if the input goes from low to high
    pub fn triggered<P>(&mut self, param: P) -> bool
        where CB: Fn(P) -> bool
    {   
        let new_state = (self.callback)(param);
        let result = !self.prev_state && new_state;
        self.prev_state = new_state;
        result
    }
}   

Update: Future readers please also note the more elegant implementation of triggered() by @Yandros above.

1 Like

Thank you all very much for your educational and informative answers! I'm still going to have to digest the answer from @Yandros as to why the answer from @mbrubeck gave me borrow-checker errors. But the answer from @RustyYato works great! Thank you all!

1 Like

One minor change that I would make: change Fn to FnMut, so that the closure can mutate watever state it captures without any fuss.

1 Like

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.