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 ops::ControlFlow in rustc_data_structures::graph::iterate #76318

Merged
merged 2 commits into from
Sep 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 6 additions & 9 deletions compiler/rustc_data_structures/src/graph/iterate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,8 @@ where
}

/// Allows searches to terminate early with a value.
#[derive(Clone, Copy, Debug)]
pub enum ControlFlow<T> {
Break(T),
Continue,
}
// FIXME (#75744): remove the alias once the generics are in a better order and `C=()`.
pub type ControlFlow<T> = std::ops::ControlFlow<(), T>;

/// The status of a node in the depth-first search.
///
Expand Down Expand Up @@ -260,12 +257,12 @@ where
_node: G::Node,
_prior_status: Option<NodeStatus>,
) -> ControlFlow<Self::BreakVal> {
ControlFlow::Continue
ControlFlow::CONTINUE
}

/// Called after all nodes reachable from this one have been examined.
fn node_settled(&mut self, _node: G::Node) -> ControlFlow<Self::BreakVal> {
ControlFlow::Continue
ControlFlow::CONTINUE
}

/// Behave as if no edges exist from `source` to `target`.
Expand All @@ -289,8 +286,8 @@ where
prior_status: Option<NodeStatus>,
) -> ControlFlow<Self::BreakVal> {
match prior_status {
Some(NodeStatus::Visited) => ControlFlow::Break(()),
_ => ControlFlow::Continue,
Some(NodeStatus::Visited) => ControlFlow::BREAK,
_ => ControlFlow::CONTINUE,
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#![doc(html_root_url = "https://doc.rust-lang.org/nightly/")]
#![allow(incomplete_features)]
#![feature(control_flow_enum)]
#![feature(in_band_lifetimes)]
#![feature(unboxed_closures)]
#![feature(generators)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#![feature(box_syntax)]
#![feature(const_fn)]
#![feature(const_panic)]
#![feature(control_flow_enum)]
#![feature(crate_visibility_modifier)]
#![feature(bool_to_option)]
#![feature(or_patterns)]
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<'mir, 'tcx> TriColorVisitor<&'mir Body<'tcx>> for Search<'mir, 'tcx> {
// A diverging InlineAsm is treated as non-recursing
TerminatorKind::InlineAsm { destination, .. } => {
if destination.is_some() {
ControlFlow::Continue
ControlFlow::CONTINUE
} else {
ControlFlow::Break(NonRecursive)
}
Expand All @@ -131,7 +131,7 @@ impl<'mir, 'tcx> TriColorVisitor<&'mir Body<'tcx>> for Search<'mir, 'tcx> {
| TerminatorKind::FalseEdge { .. }
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::Goto { .. }
| TerminatorKind::SwitchInt { .. } => ControlFlow::Continue,
| TerminatorKind::SwitchInt { .. } => ControlFlow::CONTINUE,
}
}

Expand All @@ -144,7 +144,7 @@ impl<'mir, 'tcx> TriColorVisitor<&'mir Body<'tcx>> for Search<'mir, 'tcx> {
}
}

ControlFlow::Continue
ControlFlow::CONTINUE
}

fn ignore_edge(&mut self, bb: BasicBlock, target: BasicBlock) -> bool {
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/adapters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,7 @@ where
) -> impl FnMut((), T) -> ControlFlow<(), B> + '_ {
move |(), x| match f(x) {
Some(x) => ControlFlow::Break(x),
None => ControlFlow::Continue(()),
None => ControlFlow::CONTINUE,
}
}

Expand Down
2 changes: 1 addition & 1 deletion library/core/src/iter/traits/double_ended.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ pub trait DoubleEndedIterator: Iterator {
mut predicate: impl FnMut(&T) -> bool,
) -> impl FnMut((), T) -> ControlFlow<(), T> {
move |(), x| {
if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::Continue(()) }
if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
}
}

Expand Down
14 changes: 7 additions & 7 deletions library/core/src/iter/traits/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2086,10 +2086,10 @@ pub trait Iterator {
#[inline]
fn check<T>(mut f: impl FnMut(T) -> bool) -> impl FnMut((), T) -> ControlFlow<(), ()> {
move |(), x| {
if f(x) { ControlFlow::Continue(()) } else { ControlFlow::Break(()) }
if f(x) { ControlFlow::CONTINUE } else { ControlFlow::BREAK }
}
}
self.try_fold((), check(f)) == ControlFlow::Continue(())
self.try_fold((), check(f)) == ControlFlow::CONTINUE
}

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

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

/// Searches for an element of an iterator that satisfies a predicate.
Expand Down Expand Up @@ -2201,7 +2201,7 @@ pub trait Iterator {
mut predicate: impl FnMut(&T) -> bool,
) -> impl FnMut((), T) -> ControlFlow<(), T> {
move |(), x| {
if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::Continue(()) }
if predicate(&x) { ControlFlow::Break(x) } else { ControlFlow::CONTINUE }
}
}

Expand Down Expand Up @@ -2236,7 +2236,7 @@ pub trait Iterator {
) -> impl FnMut((), T) -> ControlFlow<(), B> {
move |(), x| match f(x) {
Some(x) => ControlFlow::Break(x),
None => ControlFlow::Continue(()),
None => ControlFlow::CONTINUE,
}
}

Expand Down Expand Up @@ -2278,7 +2278,7 @@ pub trait Iterator {
R: Try<Ok = bool>,
{
move |(), x| match f(&x).into_result() {
Ok(false) => ControlFlow::Continue(()),
Ok(false) => ControlFlow::CONTINUE,
Ok(true) => ControlFlow::Break(Ok(x)),
Err(x) => ControlFlow::Break(Err(x)),
}
Expand Down
43 changes: 43 additions & 0 deletions library/core/src/ops/control_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,46 @@ impl<R: Try> ControlFlow<R::Ok, R> {
}
}
}

impl<B> ControlFlow<(), B> {
/// It's frequently the case that there's no value needed with `Continue`,
/// so this provides a way to avoid typing `(())`, if you prefer it.
///
/// # Examples
///
/// ```
/// #![feature(control_flow_enum)]
/// use std::ops::ControlFlow;
///
/// let mut partial_sum = 0;
/// let last_used = (1..10).chain(20..25).try_for_each(|x| {
/// partial_sum += x;
/// if partial_sum > 100 { ControlFlow::Break(x) }
/// else { ControlFlow::CONTINUE }
/// });
/// assert_eq!(last_used.break_value(), Some(22));
/// ```
#[unstable(feature = "control_flow_enum", reason = "new API", issue = "75744")]
pub const CONTINUE: Self = ControlFlow::Continue(());
Copy link
Member Author

@scottmcm scottmcm Sep 4, 2020

Choose a reason for hiding this comment

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

Comments welcome on whether this is a good idea.

I could also imagine other things like

enum ControlFlow<B, C=()> { BreakWith(B), ContinueWith(C) }
impl<B> ControlFlow<B> { pub const Continue: Self = ControlFlow::ContinueWith(()); }

But that doesn't seem like how things are done elsewhere...

(And of course there's always the answer of just not having a constant and typing the (()).)

Copy link
Contributor

@ecstatic-morse ecstatic-morse Sep 4, 2020

Choose a reason for hiding this comment

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

All are intriguing. Also possible would be an associated constant with the exact same name as the variant, since type inference should do a good job of choosing between the enum variant constructor (fn(()) -> ControlFlow) and the constant (ControlFlow) in most cases.

I tend to think we should avoid "magic" (a constant that looks like a variant), so I think I prefer either the upper-case associated constant or writing out the (()). If you like the associated constant, there should probably be one for Break as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also possible would be an associated constant with the exact same name as the variant

I tried this out for fun, and it's weird. I don't understand why it's allowed (since I thought constants and variants were both in value namespace), but the constant seems to always be hidden: #76347

I'll add the BREAK equivalent to this PR and ping libs as requested.

}

impl<C> ControlFlow<C, ()> {
/// APIs like `try_for_each` don't need values with `Break`,
/// so this provides a way to avoid typing `(())`, if you prefer it.
///
/// # Examples
///
/// ```
/// #![feature(control_flow_enum)]
/// use std::ops::ControlFlow;
///
/// let mut partial_sum = 0;
/// (1..10).chain(20..25).try_for_each(|x| {
/// if partial_sum > 100 { ControlFlow::BREAK }
/// else { partial_sum += x; ControlFlow::CONTINUE }
/// });
/// assert_eq!(partial_sum, 108);
/// ```
#[unstable(feature = "control_flow_enum", reason = "new API", issue = "75744")]
pub const BREAK: Self = ControlFlow::Break(());
}