Open remexre opened 6 years ago
I definitely agree with taking the most general Fn
-family bound where possible. (i.e. FnOnce
on coproducts, FnMut
on HLists).
For FuncMut
, the picture is less clear. You can see some existing discussion here: https://github.com/lloydmeta/frunk/pull/106 . In that PR I discuss my own independent attempts at adding user-implementable function traits, and indeed I chose to add a trio of Fun/FunMut/FunOnce
to parallel the stdlib. One of the big problems is that there is no ergonomical way to recover the Fun < FunMut < FunOnce
subtrait tree, which only works well in Rust due to magically autogenerated impls.
The current choice of Func
in frunk
sidesteps the issue by not having the function even take a context. If you look closely, you'll see there is no &self
argument and hence less impetus for adding a hierarchy of traits (though this no doubt comes at a great loss of power).
Now, your PR actually seems to have a neat solution to this that I'm not sure if we've tried or discussed before, which is that one can use different newtype wrappers Poly
versus PolyMut
to select which trait is the "primary" trait. I mentioned about a similar idea in this comment, and it seemed I wasn't too enthusiastic about it, but I can't recall why. Perhaps the terrain of design space is different, so long as we can outline precisely which use cases we are aiming to support and which ones we aren't.
I'm using this so I can call a trait method on an HList of values that all belong to the same trait. The specific code is here:
There's not really a huge reason to have PolyMut<F> where F: FuncMut
over PolyMut<F> where F: FnMut
, though, since I should be able to make SystemStepper
into a function that returns impl 'a + FnMut
instead of a struct.
FnMut
seems good; the order of calls don't seem confusing wrt. mutation.poly_fn_mut!
to consistency?The question of consistency is currently a null point; Func takes no context, so poly_fn!
provides no way to describe context (which is a fine limitation since it is difficult to do ergonomically).
Furthermore, @remexre's use case is a generic closure, which is even more monumentally difficult to make a comfortable interface around. It's only one step away from the most complicated example I have in my own experimental repo
unbox!{
// Define a generic context
pub struct Contains<'a, T>(&'a [T]) where T: 'a;
// Make it a generic closure
impl Fn <'b, U: 'b>(&self, u: &'b U) -> bool
where T: PartialEq<U>,
{ self.0.iter().any(|t| t == u) }
}
(remexre's example is almost as complicated, but with a monomorphic context)
Honestly, in my own projects, I just write my own traits now for mapping over hlists in particular ways with certain types of context, since there's no generic solution I've found comfortable to use.
Here are my recent thoughts:
If you've tried it and it works well for you, FuncMut
and PolyMut
are probably fine to add. My reasons for being against it are probably letting perfect be the enemy of good, and this is not normally the bar that frunk
holds its features to. There are plenty of experimental features in frunk
like transmogrification, which have nasty edge cases that are difficult or impossible to solve.
If FuncMut/PolyMut
isn't good enough for somebody's use case, they can create an issue and we can iterate on the design.
Func
should be revised to take a &self
context in the next major version bump.poly_fn_mut!
is unnecessary and too difficult to implement.I haven’t taken a thorough look (currently on vacation with spotty internet) but the proposed changes and feedback look reasonable to me :)
Also :+1: for poly_fn_mut!
perhaps being too complex to implement nicely. It might be nice to have though, and I’m wondering now if we should have a frunk_extras
module to hold things that are not-exactly-polished-but-still-useful like that sort of thing, along with Transmogifier
, which we can iterate quickly (if needed, backward-breakingly) on as experiments.
This has been sitting here for a while, should I just push the big green button?
I need to add examples+tests still; it's finals week here so I'm gonna be a bit busy until the 19th or 20th; I should be able to do them then, though.
This also makes
HMappable<F>
acceptFnMut
instead of justFn
; I don't believe this would be breaking, but I can split it out if it is.