Solving coherence problems with trait impls for closures: provide a `Closure` wrapper in libcore and make closure expressions evaluate to that type

The Problem

If you want to implement a trait "for closures", you have to write a blanket impl, roughly like this:

impl<F: FnOnce(bool) -> String> MyTrait for F { ... }

The problem with these kinds of impls is that they could apply to every type, thus causing a lot of problems with coherence and orphan rules. This is something I encountered a lot already. The solution often involves creating a wrapper that contains the closure and then implementing the trait for Wrapper<F>. It works, but is not optimal: the wrapper has to be added to the API and users have to wrap their closures into that wrapper to use it as MyTrait.

Just to give a specific example, in one crate I'm developing right now, I have something similar to this:

trait Map<Key> {
    type Value;
    fn get(&self, key: Key) -> Self::Value;
}

impl<Key, Value> Map<Key> for std::collections::HashMap<Key, Value> { ... }

// That would be useful:
impl<Key, Value, F: Fn(Key) -> Value>  Map<Key, Value> for F { ... }

It would be really useful to also implement the trait for closures, but this leads to the coherence problems mentioned above. For example, with above closure impl it's not possible anymore to to implement the trait for &T where T: Map<Key, Value>, which is a super useful impl as well. It gets even worse when other crates want to implement Map for their own types.

So I had to do this:

struct FnMap<F>(F);

impl<Key, Value, F: Fn(Key) -> Value>  Map<Key, Value> for FnMap<F> { ... }

Again, it works, but now I have FnMap in my API and users always have to import FnMap and wrap their closure in it. Absolutely not optimal!


The idea

We could add such a wrapper to libcore already. For example, core::ops::Closure (the specific module path is not important right now, though). It would be roughly defined as:

#[derive(Clone, ...)]
pub struct Closure<F>(F);

// Implement `Fn*` traits by passing through to `F`
impl<Args, F: FnOnce<Args>> FnOnce<Args> for Closure<F> { ... }
impl<Args, F: FnMut<Args>> FnMut<Args> for Closure<F> { ... }
impl<Args, F: Fn<Args>> Fn<Args> for Closure<F> { ... }

But the main part of the idea is that a closure expression |...| ... would automatically "result" in Closure<F>. That way the user does not need to wrap it themselves. I see two ways to do that:

  1. When generating the closure type, the compiler actually just wraps it into Closure automatically. This would mean that all closure expressions would have a type Closure<F>, where F is the anonymous generated type.
  2. Closure types could automatically coerce to Closure<F>. I'm not sure if this is better or worse, but it would be possible.

What would this idea solve?

Well, for one, it is backwards compatible. Since Closure<F> also implements all traits that F implements, and all existing usages of closures are fully generic, wrapping closures into Closure would just work. I also thought about non-capturing closures that coerce to function pointers, but I don't think this would be a problem.

With this, traits could be implemented for Closure<F> instead of just F, solving the issues with coherence and orphan rules. This makes handling closures a lot less annoying in many situation, IMO.


Feedback

What do you think? Anything important I missed? Do you also think this problem is worth solving somehow or have you never encountered it? Additional ideas?

2 Likes

This looks like it would become obsolete if a more general feature was ever added, for mitigating the orphan rule.

I would also worry about breaking the Fn* abstraction. Code that requires a function shouldn't care if the caller happens to pass a closure, an fn item or a method.

do_something(|a| a + 1); // ok

fn add1(a: i32) -> i32 {
    a + 1
}
do_something(add1); // not ok!
3 Likes

This looks like it would become obsolete if a more general feature was ever added, for mitigating the orphan rule.

Yes, that is true. But quite honestly, I don't expect a more general solution to land anytime soon. Coherence is a complicated topic, the problems caused by it are very old and the progress since 1.0 in this area has been very slow. So I think it's still worth it to add solutions that only fix problems until a broader solution is found. There have been many of those in Rust already.

Code that requires a function shouldn't care if the caller happens to pass a closure, an fn item or a method.

Good point. But that should be possible to fix by also coercing function pointers/items to Closure<_>, right? Of course the, Closure should probably be renamed to sth else.

I like this idea, I have run into this exact issue a number of times, and this looks like a nice fix.

On the issue of function pointers, we could have then be the same type as Closure<_>, and just have it spelled the same way as today. This way it just works without breaking backwards compatibility. (Basically, make the current spelling a type alias for the more general feature). We don't have to worry about fn items becayse those are unnamable, so we could change those over to Closure<_> as well.

1 Like

Maybe i'm wrong, but there actually is a type that acts just like your Closure: Box<dyn Fn*>. All Fn* traits are passed through them. So maybe you can just implement MyTrait on them instead?

That would require boxing closures, which isn't desirable. Also, there is a generic parameter on Closure<_> that identifies exactly which closure it is.

Currently, there are some coercion rules with closure types that go <noncapturing |Args| -> Ret> => fn(Args) -> Ret. This seems to interact a bit poorly with this wrapping behavior?

If Closure<_> is a lang item (which it already has to be), then this should be fine. I view this as given a common name to all functions/closures, so it doesn't actually change any behavior. It just allows generalizing over all functions/closures.

I note that Fn is already marked #[fundamental]:

Since it's already special because of that, it makes me wonder if the overlap checking for blanket impls could take more advantage of it than it already does...

2 Likes

To summarize/reiterate some potential problems:

Whenever a function (or something else) accepts a closure, it should also accept function pointer and function items. That's how it works today and restricting something to only actual closures does not make sense (I think). So if we want closures to have the type Closure<_>, then function pointer and items have to be that type too. Let's rename Closure to Callable for now, as we are also dealing with non-closure function-things.

If I'm not mistaken there are three kinds of types that implement the Fn* traits:

Only function pointer types can actually be named in Rust code (the other two are Voldemort types). The task is now to make all those types to actually be of type Callable<_> but still behave exactly like before (so that we can remain backwards compatibility). Implementing all traits (like Clone, PartialEq, ...) that are implemented for those types for Callable is not a problem, I think. However, function pointers can be cast (via as) to raw pointers or integers (see here). This is more tricky to solve.

So to make it work, Callable<_> has to be a lang item which it has to be anyway, as @Yato said. Then there need to exist some special rules in the compiler regarding Callable. Finally, function pointer types could then just be an alias for Callable<_>. As function items and closure types are Voldemort types, we don't need to worry about aliasing them.

In summary so far: it should work in a completely backwards compatible way, but it requires a new lang item with a couple of rules. More feedback is welcome! :slight_smile:

3 Likes