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.

Sign up for GitHub

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

Rename and expose LoopState as ControlFlow #76204

Merged
merged 2 commits into from
Sep 3, 2020
Merged

Rename and expose LoopState as ControlFlow #76204

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d0af125
Rename and expose LoopState as ControlFlow
NoraCodes Sep 1, 2020
96eb5e1
Format ControlFlow changes with rustfmt
NoraCodes Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions library/core/src/iter/adapters/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::cmp;
use crate::fmt;
use crate::intrinsics;
use crate::ops::{Add, AddAssign, Try};
use crate::ops::{Add, AddAssign, Try, ControlFlow};

use super::{from_fn, LoopState};
use super::from_fn;
use super::{DoubleEndedIterator, ExactSizeIterator, FusedIterator, Iterator, TrustedLen};

mod chain;
Expand Down Expand Up @@ -1164,10 +1164,10 @@ where
#[inline]
fn find<T, B>(
f: &mut impl FnMut(T) -> Option<B>,
) -> impl FnMut((), T) -> LoopState<(), B> + '_ {
) -> impl FnMut((), T) -> ControlFlow<(), B> + '_ {
move |(), x| match f(x) {
Some(x) => LoopState::Break(x),
None => LoopState::Continue(()),
Some(x) => ControlFlow::Break(x),
None => ControlFlow::Continue(()),
}
}

Expand Down Expand Up @@ -1864,13 +1864,13 @@ where
flag: &'a mut bool,
p: &'a mut impl FnMut(&T) -> bool,
mut fold: impl FnMut(Acc, T) -> R + 'a,
) -> impl FnMut(Acc, T) -> LoopState<Acc, R> + 'a {
) -> impl FnMut(Acc, T) -> ControlFlow<Acc, R> + 'a {
move |acc, x| {
if p(&x) {
LoopState::from_try(fold(acc, x))
ControlFlow::from_try(fold(acc, x))
} else {
*flag = true;
LoopState::Break(Try::from_ok(acc))
ControlFlow::Break(Try::from_ok(acc))
}
}
}
Expand Down Expand Up @@ -1963,8 +1963,8 @@ where
{
let Self { iter, predicate } = self;
iter.try_fold(init, |acc, x| match predicate(x) {
Some(item) => LoopState::from_try(fold(acc, item)),
None => LoopState::Break(Try::from_ok(acc)),
Some(item) => ControlFlow::from_try(fold(acc, item)),
None => ControlFlow::Break(Try::from_ok(acc)),
})
.into_try()
}
Expand Down Expand Up @@ -2135,11 +2135,11 @@ where
fn check<T, Acc, R: Try<Ok = Acc>>(
mut n: usize,
mut fold: impl FnMut(Acc, T) -> R,
) -> impl FnMut(Acc, T) -> LoopState<Acc, R> {
) -> impl FnMut(Acc, T) -> ControlFlow<Acc, R> {
move |acc, x| {
n -= 1;
let r = fold(acc, x);
if n == 0 { LoopState::Break(r) } else { LoopState::from_try(r) }
if n == 0 { ControlFlow::Break(r) } else { ControlFlow::from_try(r) }
}
}

Expand Down Expand Up @@ -2246,11 +2246,11 @@ where
fn check<'a, T, Acc, R: Try<Ok = Acc>>(
n: &'a mut usize,
mut fold: impl FnMut(Acc, T) -> R + 'a,
) -> impl FnMut(Acc, T) -> LoopState<Acc, R> + 'a {
) -> impl FnMut(Acc, T) -> ControlFlow<Acc, R> + 'a {
move |acc, x| {
*n -= 1;
let r = fold(acc, x);
if *n == 0 { LoopState::Break(r) } else { LoopState::from_try(r) }
if *n == 0 { ControlFlow::Break(r) } else { ControlFlow::from_try(r) }
}
}

Expand Down Expand Up @@ -2414,10 +2414,10 @@ where
state: &'a mut St,
f: &'a mut impl FnMut(&mut St, T) -> Option<B>,
mut fold: impl FnMut(Acc, B) -> R + 'a,
) -> impl FnMut(Acc, T) -> LoopState<Acc, R> + 'a {
) -> impl FnMut(Acc, T) -> ControlFlow<Acc, R> + 'a {
move |acc, x| match f(state, x) {
None => LoopState::Break(Try::from_ok(acc)),
Some(x) => LoopState::from_try(fold(acc, x)),
None => ControlFlow::Break(Try::from_ok(acc)),
Some(x) => ControlFlow::from_try(fold(acc, x)),
}
}

Expand Down Expand Up @@ -2638,10 +2638,10 @@ where
let error = &mut *self.error;
self.iter
.try_fold(init, |acc, x| match x {
Ok(x) => LoopState::from_try(f(acc, x)),
Ok(x) => ControlFlow::from_try(f(acc, x)),
Err(e) => {
*error = Err(e);
LoopState::Break(Try::from_ok(acc))
ControlFlow::Break(Try::from_ok(acc))
}
})
.into_try()
Expand Down
55 changes: 0 additions & 55 deletions library/core/src/iter/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,8 +308,6 @@

#![stable(feature = "rust1", since = "1.0.0")]

use crate::ops::Try;

#[stable(feature = "rust1", since = "1.0.0")]
pub use self::traits::Iterator;

Expand Down Expand Up @@ -368,56 +366,3 @@ mod range;
mod sources;
mod traits;

/// Used to make try_fold closures more like normal loops
#[derive(PartialEq)]
enum LoopState<C, B> {
Continue(C),
Break(B),
}

impl<C, B> Try for LoopState<C, B> {
type Ok = C;
type Error = B;
#[inline]
fn into_result(self) -> Result<Self::Ok, Self::Error> {
match self {
LoopState::Continue(y) => Ok(y),
LoopState::Break(x) => Err(x),
}
}
#[inline]
fn from_error(v: Self::Error) -> Self {
LoopState::Break(v)
}
#[inline]
fn from_ok(v: Self::Ok) -> Self {
LoopState::Continue(v)
}
}

impl<C, B> LoopState<C, B> {
#[inline]
fn break_value(self) -> Option<B> {
match self {
LoopState::Continue(..) => None,
LoopState::Break(x) => Some(x),
}
}
}

impl<R: Try> LoopState<R::Ok, R> {
#[inline]
fn from_try(r: R) -> Self {
match Try::into_result(r) {
Ok(v) => LoopState::Continue(v),
Err(v) => LoopState::Break(Try::from_error(v)),
}
}
#[inline]
fn into_try(self) -> R {
match self {
LoopState::Continue(v) => Try::from_ok(v),
LoopState::Break(v) => v,
}
}
}
7 changes: 3 additions & 4 deletions library/core/src/iter/traits/double_ended.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::iter::LoopState;
use crate::ops::Try;
use crate::ops::{Try, ControlFlow};

/// An iterator able to yield elements from both ends.
///
Expand Down Expand Up @@ -309,9 +308,9 @@ pub trait DoubleEndedIterator: Iterator {
#[inline]
fn check<T>(
mut predicate: impl FnMut(&T) -> bool,
) -> impl FnMut((), T) -> LoopState<(), T> {
) -> impl FnMut((), T) -> ControlFlow<(), T> {
move |(), x| {
if predicate(&x) { LoopState::Break(x) } else { LoopState::Continue(()) }
if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::Continue(()) }
}
}

Expand Down
41 changes: 20 additions & 21 deletions library/core/src/iter/traits/iterator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,8 @@
// can't split that into multiple files.

use crate::cmp::{self, Ordering};
use crate::ops::{Add, Try};
use crate::ops::{Add, Try, ControlFlow};

use super::super::LoopState;
use super::super::TrustedRandomAccess;
use super::super::{Chain, Cloned, Copied, Cycle, Enumerate, Filter, FilterMap, Fuse};
use super::super::{FlatMap, Flatten};
Expand Down Expand Up @@ -2088,12 +2087,12 @@ pub trait Iterator {
F: FnMut(Self::Item) -> bool,
{
#[inline]
fn check<T>(mut f: impl FnMut(T) -> bool) -> impl FnMut((), T) -> LoopState<(), ()> {
fn check<T>(mut f: impl FnMut(T) -> bool) -> impl FnMut((), T) -> ControlFlow<(), ()> {
move |(), x| {
if f(x) { LoopState::Continue(()) } else { LoopState::Break(()) }
if f(x) { ControlFlow::Continue(()) } else { ControlFlow::Break(()) }
}
}
self.try_fold((), check(f)) == LoopState::Continue(())
self.try_fold((), check(f)) == ControlFlow::Continue(())
}

/// Tests if any element of the iterator matches a predicate.
Expand Down Expand Up @@ -2141,13 +2140,13 @@ pub trait Iterator {
F: FnMut(Self::Item) -> bool,
{
#[inline]
fn check<T>(mut f: impl FnMut(T) -> bool) -> impl FnMut((), T) -> LoopState<(), ()> {
fn check<T>(mut f: impl FnMut(T) -> bool) -> impl FnMut((), T) -> ControlFlow<(), ()> {
move |(), x| {
if f(x) { LoopState::Break(()) } else { LoopState::Continue(()) }
if f(x) { ControlFlow::Break(()) } else { ControlFlow::Continue(()) }
}
}

self.try_fold((), check(f)) == LoopState::Break(())
self.try_fold((), check(f)) == ControlFlow::Break(())
}

/// Searches for an element of an iterator that satisfies a predicate.
Expand Down Expand Up @@ -2203,9 +2202,9 @@ pub trait Iterator {
#[inline]
fn check<T>(
mut predicate: impl FnMut(&T) -> bool,
) -> impl FnMut((), T) -> LoopState<(), T> {
) -> impl FnMut((), T) -> ControlFlow<(), T> {
move |(), x| {
if predicate(&x) { LoopState::Break(x) } else { LoopState::Continue(()) }
if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::Continue(()) }
}
}

Expand Down Expand Up @@ -2235,10 +2234,10 @@ pub trait Iterator {
F: FnMut(Self::Item) -> Option<B>,
{
#[inline]
fn check<T, B>(mut f: impl FnMut(T) -> Option<B>) -> impl FnMut((), T) -> LoopState<(), B> {
fn check<T, B>(mut f: impl FnMut(T) -> Option<B>) -> impl FnMut((), T) -> ControlFlow<(), B> {
move |(), x| match f(x) {
Some(x) => LoopState::Break(x),
None => LoopState::Continue(()),
Some(x) => ControlFlow::Break(x),
None => ControlFlow::Continue(()),
}
}

Expand Down Expand Up @@ -2274,15 +2273,15 @@ pub trait Iterator {
R: Try<Ok = bool>,
{
#[inline]
fn check<F, T, R>(mut f: F) -> impl FnMut((), T) -> LoopState<(), Result<T, R::Error>>
fn check<F, T, R>(mut f: F) -> impl FnMut((), T) -> ControlFlow<(), Result<T, R::Error>>
where
F: FnMut(&T) -> R,
R: Try<Ok = bool>,
{
move |(), x| match f(&x).into_result() {
Ok(false) => LoopState::Continue(()),
Ok(true) => LoopState::Break(Ok(x)),
Err(x) => LoopState::Break(Err(x)),
Ok(false) => ControlFlow::Continue(()),
Ok(true) => ControlFlow::Break(Ok(x)),
Err(x) => ControlFlow::Break(Err(x)),
}
}

Expand Down Expand Up @@ -2352,10 +2351,10 @@ pub trait Iterator {
#[inline]
fn check<T>(
mut predicate: impl FnMut(T) -> bool,
) -> impl FnMut(usize, T) -> LoopState<usize, usize> {
) -> impl FnMut(usize, T) -> ControlFlow<usize, usize> {
// The addition might panic on overflow
move |i, x| {
if predicate(x) { LoopState::Break(i) } else { LoopState::Continue(Add::add(i, 1)) }
if predicate(x) { ControlFlow::Break(i) } else { ControlFlow::Continue(Add::add(i, 1)) }
}
}

Expand Down Expand Up @@ -2411,10 +2410,10 @@ pub trait Iterator {
#[inline]
fn check<T>(
mut predicate: impl FnMut(T) -> bool,
) -> impl FnMut(usize, T) -> LoopState<usize, usize> {
) -> impl FnMut(usize, T) -> ControlFlow<usize, usize> {
move |i, x| {
let i = i - 1;
if predicate(x) { LoopState::Break(i) } else { LoopState::Continue(i) }
if predicate(x) { ControlFlow::Break(i) } else { ControlFlow::Continue(i) }
}
}

Expand Down
67 changes: 67 additions & 0 deletions library/core/src/ops/control_flow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
use crate::ops::Try;

/// Used to make try_fold closures more like normal loops
#[unstable(feature="control_flow_enum", reason="new API", issue="75744")]
#[derive(Debug, Clone, Copy, PartialEq)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#[derive(Debug, Clone, Copy, PartialEq)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]

Hash and especially Eq seem handy, although they are not needed to replace LoopState and are less essential for ControlFlow than for Option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that seems sensible! I don't know when you'd want to hash this but I suppose there's no reason to prevent it.

pub enum ControlFlow<C, B> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have found that C = () is very common when using this enum, common enough that it might deserve a defaulted type parameter. We could add one backwards compatibly but only if C follows B in the parameter list. Otherwise we would have to default B at the same time and anyone that wishes to override it would have to override C as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! It's even usually that way in the uses this PR touches, like
image
I suppose we could even do ControlFlow<B = (), C = ()> the same way break; is break ();.

(Though I suppose the reorder might make the diff messier, so could also be a separate PR.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be confusing to do this without changing the order of the variants as well, and we may as well do that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On consideration, I think a separate PR would be best.

/// Continue in the loop, using the given value for the next iteration
Continue(C),
/// Exit the loop, yielding the given value
Break(B),
}

#[unstable(feature="control_flow_enum", reason="new API", issue="75744")]
impl<C, B> Try for ControlFlow<C, B> {
type Ok = C;
type Error = B;
#[inline]
fn into_result(self) -> Result<Self::Ok, Self::Error> {
match self {
ControlFlow::Continue(y) => Ok(y),
ControlFlow::Break(x) => Err(x),
}
}
#[inline]
fn from_error(v: Self::Error) -> Self {
ControlFlow::Break(v)
}
#[inline]
fn from_ok(v: Self::Ok) -> Self {
ControlFlow::Continue(v)
}
}

impl<C, B> ControlFlow<C, B> {
/// Converts the `ControlFlow` into an `Option` which is `Some` if the
/// `ControlFlow` was `Break` and `None` otherwise.
#[inline]
#[unstable(feature="control_flow_enum", reason="new API", issue="75744")]
pub fn break_value(self) -> Option<B> {
match self {
ControlFlow::Continue(..) => None,
ControlFlow::Break(x) => Some(x),
}
}
}

impl<R: Try> ControlFlow<R::Ok, R> {
Copy link
Contributor

@ecstatic-morse ecstatic-morse Sep 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the second parameter be R::Error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see - we can't do this because it's an unconstrained type parameter. Is there a good way to solve that?

Copy link
Member

@scottmcm scottmcm Sep 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the break part here is intentionally the full impl Try type (Result, etc). They're definitely strange APIs, though -- break_value is something that could plausibly stabilize, but these ones probably aren't.

Maybe leave this particular impl block over in iter as non-pub so they can still be used there, but don't show up in the rustdoc? (And won't need #[unstable] since they'll be unusable outside iter.)

I've been contemplating making a pass through the implementations to use feature(try_blocks) instead of explicit Try::foo calls where possible; this PR going in would be a good impetus to go do that -- and hopefully delete these methods while I'm at it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see. I misunderstood how these were being used. You're right, it's a very strange API.

/// Create a `ControlFlow` from any type implementing `Try`.
#[unstable(feature="control_flow_enum", reason="new API", issue="75744")]
#[inline]
pub fn from_try(r: R) -> Self {
match Try::into_result(r) {
Ok(v) => ControlFlow::Continue(v),
Err(v) => ControlFlow::Break(Try::from_error(v)),
}
}

/// Convert a `ControlFlow` into any type implementing `Try`;
#[unstable(feature="control_flow_enum", reason="new API", issue="75744")]
#[inline]
pub fn into_try(self) -> R {
match self {
ControlFlow::Continue(v) => Try::from_ok(v),
ControlFlow::Break(v) => v,
}
}
}
4 changes: 4 additions & 0 deletions library/core/src/ops/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ mod index;
mod range;
mod r#try;
mod unsize;
mod control_flow;

#[stable(feature = "rust1", since = "1.0.0")]
pub use self::arith::{Add, Div, Mul, Neg, Rem, Sub};
Expand Down Expand Up @@ -191,3 +192,6 @@ pub use self::unsize::CoerceUnsized;

#[unstable(feature = "dispatch_from_dyn", issue = "none")]
pub use self::unsize::DispatchFromDyn;

#[unstable(feature="control_flow_enum", reason="new API", issue="75744")]
pub use self::control_flow::ControlFlow;