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

Jump to bottom

Use ops::ControlFlow in rustc_data_structures::graph::iterate #76318

Merged
merged 2 commits into from
Sep 7, 2020
Merged

Use ops::ControlFlow in rustc_data_structures::graph::iterate #76318

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
fac2726
Use ops::ControlFlow in graph::iterate
scottmcm Sep 4, 2020
59e3733
Add `BREAK` too, and improve the comments
scottmcm Sep 4, 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
13 changes: 5 additions & 8 deletions compiler/rustc_data_structures/src/graph/iterate/mod.rs
View file
Open in desktop
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 @@ -290,7 +287,7 @@ where
) -> ControlFlow<Self::BreakVal> {
match prior_status {
Some(NodeStatus::Visited) => ControlFlow::Break(()),
_ => ControlFlow::Continue,
_ => ControlFlow::CONTINUE,
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_data_structures/src/lib.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
7 changes: 7 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
Expand Up @@ -65,3 +65,10 @@ 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.
#[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 •
edited
Loading

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 •
edited
Loading

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.

}