bluss / permutohedron

https://docs.rs/permutohedron/
Apache License 2.0
38 stars 5 forks source link

Consider swapping permutohedron::control::Control & friends with std::ops::ControlFlow #10

Open schneiderfelipe opened 8 months ago

schneiderfelipe commented 8 months ago

This (in fact the whole file) https://github.com/bluss/permutohedron/blob/afdb9433abb21ffa1dbd43f7aba58b2c62d3fee6/src/control.rs#L13-L20

could be swapped with std::ops::ControlFlow, available since Rust 1.55.0.

But signatures like https://github.com/bluss/permutohedron/blob/afdb9433abb21ffa1dbd43f7aba58b2c62d3fee6/src/lib.rs#L44-L46

would become (something like)

pub fn heap_recursive<T, F, B, C>(xs: &mut [T], mut f: F) -> std::ops::ControlFlow<B, C>
    where F: FnMut(&mut [T]) -> std::ops::ControlFlow<B, C>,

or maybe even

pub fn heap_recursive<T, F, B, C>(xs: &mut [T], mut f: F) -> Result<C, B>
    where F: FnMut(&mut [T]) -> std::ops::ControlFlow<B, C>,

since std::ops::Try is not yet stable (rust-lang/rust#84277); that would be a breaking change.

What do you think @bluss?

bluss commented 8 months ago

I think some of the finesse is removed when C=() is no longer allowed?

schneiderfelipe commented 6 months ago

I think some of the finesse is removed when C=() is no longer allowed?

This is very true. In any case, we can get rid of control::Control by making use of std::ops::ControlFlow<B, ()>. I'll make a PR to see how it looks like.

schneiderfelipe commented 6 months ago

I think some of the finesse is removed when C=() is no longer allowed?

On a second thought, even forcing the usage of std::ops::ControlFlow as the output of every closure would not remove any finesse (one could return std::ops::Continue(()) forever), but would severely hurt ergonomics (e.g., functions returning Result<(), E> would have to be wrapped, etc.).

Even using std::ops::Try (when it stabilizes) would disallow C = () (as () is currently not std::ops::Try). So #11 might be a nice middle ground, swapping control::Control for std::ops::ControlFlow, but still as an implementation of control::Control trait.