implement continue_ok and break_ok for ControlFlow by jogru0 · Pull Request #140267 · rust-lang/rust (original) (raw)

Thanks for the feedback!

I attempted to add some more fleshed out examples for both methods. Please have a look.

I think my example for continue_ok makes roughly sense, and is basically the initial motivation for the ACP: Have an interface function to validate that an operation was completed, which uses Result since that conveys the intended semantics.

I tried to find a sensible analogous usecase for break_ok, but as long as ControlFlow uses () for C, it feels a bit arbitrary to work with Result<B, ()> instead of just using break_value to get an Option<B>. For now, my example demonstrates how to implement find that way, which of course has this exact issue just mentioned: Conventionally, find would return a Option<&T>, so it would make more sense to implement it via break_value.

Therefore, a proper good usage example for break_value requires an usecase for ControlFlow where C is not (). I don't really know what that would be, but maybe someone else has a good idea? If we find one, maybe my current find example can be converted to an example for break_value, as that currently also doesn't have any fleshed out example in the docs.

Finally, something I noticed: My examples all derive from the existing ControlFlow module test. Interestingly, for the find implementation, I had to fight quite a bit with the borrow checker until I managed to get it to compile. It was only possible after adding explicit lifetimes to the traverse_inorder method from the module example. Without that, the compiler told me:

note: due to current limitations in the borrow checker, this implies a `'static` lifetime
  --> src/main.rs:26:22
   |
26 |         f: &mut impl FnMut(&T) -> ControlFlow<B>,
   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^

So I was wondering, should we maybe add the explicit lifetimes in the module wide example as well, if that seems to be necessary to be able to use the method for certain usecases?

@rustbot label -S-waiting-on-author +S-waiting-on-review