Rust: Tracking issue for `ControlFlow` enum, for use with `try_fold` and in `Try`

Created on 20 Aug 2020  路  10Comments  路  Source: rust-lang/rust

(edited to turn this into a tracking issue, as it's referenced by the unstable attributes)

This is a tracking issue for the std::ops::ControlFlow type.
The feature gate for the issue is #![feature(control_flow_enum)].

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also uses as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

Implementation history

Initial PR that added LoopState as an implementation detail: https://github.com/rust-lang/rust/pull/45595
PR that exposed as unstable and renamed to ControlFlow: https://github.com/rust-lang/rust/pull/76204
Added BREAK/CONTINUE associated constants: https://github.com/rust-lang/rust/pull/76318
Changed type parameter order and defaulted C = (): https://github.com/rust-lang/rust/pull/76614
Add is_break and is_continue methods: #78200


I work with an organization that has a large amount of Rust graph traversal code as part of its core business. We used to use itertools's fold_while for short-circuiting functional-style loops over slices of our graphs, which is now deprecated in favor of try_fold.

try_fold is great, but the lack of a standard library provided Try implementation that makes the loop semantics clear is confusing. We created our own, which is fine, but I think it would make a lot of sense to expose LoopState and provide an example in the docs.

Originally related to: rust-itertools/itertools#469


A-control-flow A-iterators C-tracking-issue Libs-Tracked T-libs

Most helpful comment

Apologies for taking forever on this. I set up a Rust dev env on my new PC and fixed the nits in that PR so we should be good to go. I'll move forward with the other work, and documentation.

All 10 comments

Big +1 for this. A Break/Continue enum is also the correct abstraction for the Try trait instead of Result IMO, so I think this is worthy inclusion in the standard library (although in that case a name change would be in order). FWIW, I usually call this enum ControlFlow.

As the one who added LoopState initially, I'm not at all tied to that name, so morse's rename sounds good to me -- in fact I've also been calling it ControlFlow in the Try discussions.

@NoraCodes Want to take on making a PR here? There's probably a few parts:

  • Renaming from LoopState to ControlFlow
  • Making it pub and having appropriate #[unstable] markers on it
  • Probably moving it out of core::iter; maybe to core::ops instead?
  • Reexporting from std
  • Derive a few more things on it now that it's intended to be a real type and not just something internal -- probably at least Clone, Copy, Debug (like morse's example has) in addition to its current PartialEq. (I would personally avoid PartialOrd and Ord for now -- if we stay order-agnostic for now we can change the discriminants later to make interconversion with result as cheap as possible without breaking people.)

Feel free to ping me (here, in a draft PR, on community Discord, or on Zulip) if you have questions.

Its methods are probably imperfect right now, but if it's unstable that's ok. People can start using it on nightly as we'll find out what needs fixing before stabilization that way.

I would be happy to do this. I may ask for some help getting a development environment set up as I have yet to contribute to the core libraries.

@rustbot assign @NoraCodes

Congrats on your first core libraries PR, @NoraCodes!

Want to take on the generic parameter reordering (that morse brought up in https://github.com/rust-lang/rust/pull/76204#discussion_r481357223) next?

Thanks @scottmcm! I definitely do want to do that. I'll put in a PR either this weekend or Tuesday.

Another example of a type that's pretty close to this: the Transition type in https://deislabs.io/posts/a-fistful-of-states/, which is essentially ControlFlow<Result<()>, Box<dyn State>> -- either execution is done (possibly with an error), or it continues (in the specified state).

An MCP to use this more often in the compiler: https://github.com/rust-lang/compiler-team/issues/374

Conveniently that also suggests that ControlFlow<BreakType> would be better than the current ControlFlow<(), BreakType>.

EDIT: A great comment on the PR that implements the MCP (https://github.com/rust-lang/rust/pull/78182#issuecomment-713695594):

This PR is awesome (and exposes so many issues of different sublety)

Nice to see confirmation of the value 馃檪

Apologies for taking forever on this. I set up a Rust dev env on my new PC and fixed the nits in that PR so we should be good to go. I'll move forward with the other work, and documentation.

Personally, I don't have much of an opinion on the CONTINUE and BREAK constants. It seems mildly unidiomatic, but I can also see how those constants are useful in writing concise code.

I do think we should default B = (); this makes the simplest cases the simplest to write, and doesn't really increase complexity for more complex cases.

Was this page helpful?
0 / 5 - 0 ratings