Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ControlFlow for IDLE callback return type #194

Open
mordak opened this issue Apr 21, 2021 · 2 comments
Open

Use ControlFlow for IDLE callback return type #194

mordak opened this issue Apr 21, 2021 · 2 comments

Comments

@mordak
Copy link
Contributor

mordak commented Apr 21, 2021

The the ControlFlow type might be nicer than the bool type as a return for the IDLE callbacks. If / when the type is stabilized have a look at implementing it there.

Originally posted by @jonhoo in #186 (comment)

@scottmcm
Copy link

scottmcm commented Apr 25, 2021

It's available on nightly if anyone wants to try an experiment to see what it would look like: https://doc.rust-lang.org/nightly/std/ops/enum.ControlFlow.html

(I agree with waiting for it to stabilize before merging such a change, though.)

EDIT: And I have a PR up to stabilize it, though libs has yet to weigh in.

@scottmcm
Copy link

It just missed 1.54, but ControlFlow is now stabilized for 1.55.

JohnTitor added a commit to JohnTitor/rust that referenced this issue Jun 15, 2021
…m-basics, r=m-ou-se

Stabilize `ops::ControlFlow` (just the type)

Tracking issue: rust-lang#75744 (which also tracks items *not* closed by this PR).

With the new `?` desugar implemented, [it's no longer possible to mix `Result` and `ControlFlow`](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=13feec97f5c96a9d791d97f7de2d49a6).  (At the time of making this PR, godbolt was still on the 2021-05-01 nightly, where you can see that [the mixing example compiled](https://rust.godbolt.org/z/13Ke54j16).)  That resolves the only blocker I know of, so I'd like to propose that `ControlFlow` be considered for stabilization.

Its basic existence was part of rust-lang/rfcs#3058, where it got a bunch of positive comments (examples [1](rust-lang/rfcs#3058 (comment)) [2](rust-lang/rfcs#3058 (review)) [3](rust-lang/rfcs#3058 (comment)) [4](rust-lang/rfcs#3058 (comment))).  Its use in the compiler has been well received (rust-lang#78182 (comment)), and there are ecosystem updates interested in using it (rust-itertools/itertools#469 (comment), jonhoo/rust-imap#194).

As this will need an FCP, picking a libs member manually:
r? `@m-ou-se`

## Stabilized APIs

```rust
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum ControlFlow<B, C = ()> {
    /// Exit the operation without running subsequent phases.
    Break(B),
    /// Move on to the next phase of the operation as normal.
    Continue(C),
}
```

As well as using `?` on a `ControlFlow<B, _>` in a function returning `ControlFlow<B, _>`.  (Note, in particular, that there's no `From::from`-conversion on the `Break` value, the way there is for `Err`s.)

## Existing APIs *not* stabilized here

All the associated methods and constants: `break_value`, `is_continue`, `map_break`, [`CONTINUE`](https://doc.rust-lang.org/nightly/std/ops/enum.ControlFlow.html#associatedconstant.CONTINUE), etc.

Some of the existing methods in nightly seem reasonable, some seem like they should be removed, and some need more discussion to decide.  But none of them are *essential*, so [as in the RFC](https://rust-lang.github.io/rfcs/3058-try-trait-v2.html#methods-on-controlflow), they're all omitted from this PR.

They can be considered separately later, as further usage demonstrates which are important.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants