implement continue_ok and break_ok for ControlFlow

[jogru0]

Tracking issue: #140266

r? @dtolnay

[dtolnay]

Thank you!

It would help people understand when they would benefit from these methods if the example code in the documentation showed a little bit more fleshed out skeleton of a real-world use case where using the new helpers is a nontrivial improvement.

The current example code is not useful for this purpose because it bears no resemblance to anything we would expect a user to write. In fact, even for the thing it is intended to show (that break_ok maps Break to Ok) I think it is not even the best way to show that. It could instead just say something like "control_flow.break_ok() is equivalent to: match control_flow { ...". But this is redundant with someone clicking the "Source" link if they are still confused after reading the 1-sentence method description and the updated example, so please decide if it's worth having after updating the example.

[jogru0]

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

[bors]

☔ The latest upstream changes (presumably #138087) made this pull request unmergeable. Please resolve the merge conflicts.

[dtolnay]

• Rebased to resolve conflict with Initial implementation of core_float_math #138087

[dtolnay]

@bors r+

[bors]

📌 Commit b981b84 has been approved by dtolnay

It is now in the queue for this repository.