POC: It is possible to define callbacks without using macros if you use a helper trait

[alsuren]

There is a discussion in the gotham gitter around doing a similar thing for route handling: https://gitter.im/gotham-rs/gotham?at=5ef9c6f4405be935cdcc4b7b . I thought I should share our approach here in case you are interested.

https://gist.github.com/87ab93979d770314e6698a9867d1e7e5 is probably worth reading first

GooseTaskCallback<'a> is equivalent to AsyncCallback<'a> in that gist.

There is a bunch of ugliness lying around (like the fact that I had to wrap it in Arc, because fn implements Clone but Fn does not). I suspect that there is a way around this, because we don't need this in Gotham.

Do you want me to keep pulling on this thread, or are you okay with your current macro-based approach?

[LionsAd]

Would this allow us to pass in a closure as well?

Right now with the macro approach we had the problem that passing in a closure is not supported.

But a closure would be perfect to pass in dynamic paths.

[alsuren]

I'm afraid this is another hit and run proof of concept from me. I probably won't have time to work on it again until next weekend (11th).

Have a play and see if it does what you want. I would also be interested in hearing feedback about whether the compiler errors you get are any good. If there are any error messages that could be improved with some hints, please post them here, and I might have a go at fixing them upstream.

Also I think that it might be possible to store GooseTask.function as a for<'a> Arc<dyn GooseTaskCallback<'a>> or something. I just haven't had more than half an hour to myself all week, so haven't been able to try it.

You did a really good job of simplifying my original async prototype. I'm looking forward to seeing what you can do with this one.

[alsuren]

Would this allow us to pass in a closure as well?

Yes. Closures implement FnOnce, which is what this blanket impl covers.

[alsuren]

... actually I tried it and it turns out it doesn't get the lifetimes right. That's sad.

[jeremyandrews]

@alsuren Any ideas on how we could make this work with closures? That's the big inspiration for any further changes to how wrapping tasks works, to make Goose far more useful as a library.

[alsuren]

I'm afraid I've not had the head space for this after work, so I've not looked into it.

Next things I would try would probably be:

• See if there is a way to annotate the lifetimes on the closure as you create it
• Take a step back to the previous approach (which managed to use closures as part of the workflow just fine, but couldn't accept them as a macro argument because we only accept ident as input) and see if there is a way to accept a closure as a macro argument. (Prototype this without using macros first maybe?)

[jeremyandrews]

Very interesting, thanks! I'm currently traveling, but will dig into this when I return in a little over a week.

It looks like the only "error" is a clippy warning about complexity recommending we instead declare a type.

Do you want me to keep pulling on this thread, or are you okay with your current macro-based approach?

We welcome it if you want to keep pulling on this thread! :) As it could solve a problem we've run into with passing in a closure in place of a function, we'll likely also continue to dig in as well.

[jeremyandrews]

Our primary interest in this was supporting closures, but @LionsAd got this working in #120 which landed today (and still uses macros).

I did run some performance comparisons on this PR, compared to the original implementation, compared to our newly added support for closures, and all are essentially the same:

• https://docs.google.com/document/d/1xZPCBKRVs74kqg74sn6O4LD2pX_B8Fb_6oaoIl2hKrI/edit#
• https://docs.google.com/spreadsheets/d/1yPaYO37wx7MmGfuvoMLNDI8V2w6TFP7jE2ObV-5OKK0/edit#gid=0

What would the advantage of this PR be at this point? Quicker compile times?

[alsuren]

Looks like #120 has incorporated the key interesting ideas behind this POC. Nice work. Seems reasonable to close this pr.

[LionsAd]

I might still pursue this to create a more generic approach of capturing a function name into a closure and to make the macro just do:

GooseTask::from_function(my_function);

but yeah I will create a new PR for that :).

[LionsAd]

I am re-opening this - as it's easiest to communicate!

I did it - I was able to create a generic helper for this:

// Generic implementation
// -----

trait AsyncCallback<'a, U, T> {
    type Output: std::future::Future<Output=T> + 'a + Send;
    fn call(self, arg: &'a U) -> Self::Output;
}

impl<'a, Fut: 'a, F, U: 'a, T> AsyncCallback<'a, U, T> for F
where
    F: FnOnce(&'a U) -> Fut,
    Fut: std::future::Future<Output=T> + Send,
{
    type Output = Fut;
    fn call(self, arg: &'a U) -> Fut {
        self(arg)
    }
}

type AsyncTaskFunction<U,T> = Arc<dyn for<'r> Fn(&'r U) -> Pin<Box<dyn Future<Output = T> + Send + 'r>> + Sync + Send>;

fn from_async_function<F, U, T>(cb: F) -> AsyncTaskFunction<U, T>
where
    for<'a> F: AsyncCallback<'a, U, T> + Clone + 'static + Send + Sync,
{
        Arc::new(move |s| Box::pin(cb.clone().call(s)))
}

This makes it dead-simple to create new async task function types:

type GooseTaskFunction = AsyncTaskFunction<MyStruct, ()>;

And to wrap a function is as simple as:

    let wrapped_function = from_async_function(my_function);

Full playground:

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

[LionsAd]

Works also still perfectly for mutable things:

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

[alsuren]

I will admit that I am a bit lost.

GooseTask::from_function(my_function) is basically what I was aiming for in this PR.

As was pointed out, this doesn't really give you anything over what you get from a macro, unless you can also have a constructor that accepts a closure.

It may even be possible to make the macro accept an arbitrary expression (rather than just an ident), and handle closures that way. I've not tried.

[LionsAd]

Well - my goal was to make handling of async fn as simple as normal fns.

And the generic approach seems to work well.

Wrapping an async fn is now possible without even knowing it’s type.

Eg what I have written could go into a helper crate.

I consider that a win.