Use ops::ControlFlow in rustc_data_structures::graph::iterate #76318
Conversation
| /// 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(()); |
There was a problem hiding this comment.
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 (()).)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
scottmcm
added
the
S-waiting-on-review
scottmcm
force-pushed
the
one-control-flow
branch
from
bc9508b to
fac2726
Compare
|
LGTM. We should mention the associated constants in the unresolved questions on #75744 and ping T-libs somewhere, since I think we should start trying to build consensus as early as possible. r=me once that is done. |
|
Updated the tracking issue; Conversation about the constants started in https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Simplifying.20continue-with-unit.20in.20.60ControlFlow.60/near/209151522 @bors r=ecstatic-morse rollup |
|
📌 Commit 59e3733 has been approved by |
bors
added
S-waiting-on-bors
…ic-morse Use ops::ControlFlow in rustc_data_structures::graph::iterate Since I only know about this because you mentioned it, r? @ecstatic-morse If we're not supposed to use new `core` things in compiler for a while then feel free to close, but it felt reasonable to merge the two types since they're the same, and it might be convenient for people to use `?` in their traversal code. (This doesn't do the type parameter swap; NoraCodes has signed up to do that one.)
Rollup of 18 pull requests Successful merges: - rust-lang#76273 (Move some Vec UI tests into alloc unit tests) - rust-lang#76274 (Allow try blocks as the argument to return expressions) - rust-lang#76287 (Remove an unnecessary allowed lint) - rust-lang#76293 (Implementation of incompatible features error) - rust-lang#76299 (Make `Ipv4Addr` and `Ipv6Addr` const tests unit tests under `library`) - rust-lang#76302 (Address review comments on `Peekable::next_if`) - rust-lang#76303 (Link to `#capacity-and-reallocation` when using with_capacity) - rust-lang#76305 (Move various ui const tests to `library`) - rust-lang#76309 (Indent a note to make folding work nicer) - rust-lang#76312 (time.rs: Make spelling of "Darwin" consistent) - rust-lang#76318 (Use ops::ControlFlow in rustc_data_structures::graph::iterate) - rust-lang#76324 (Move Vec slice UI tests in library) - rust-lang#76338 (add some intra-doc links to `Iterator`) - rust-lang#76340 (Remove unused duplicated `trivial_dropck_outlives`) - rust-lang#76344 (Improve docs for `std::env::args()`) - rust-lang#76346 (Docs: nlink example typo) - rust-lang#76358 (Minor grammar fix in doc comment for soft-deprecated methods) - rust-lang#76364 (Disable atomics on avr target.) Failed merges: - rust-lang#76304 (Make delegation methods of `std::net::IpAddr` unstably const) r? @ghost
Since I only know about this because you mentioned it,
r? @ecstatic-morse
If we're not supposed to use new
corethings in compiler for a while then feel free to close, but it felt reasonable to merge the two types since they're the same, and it might be convenient for people to use?in their traversal code.(This doesn't do the type parameter swap; NoraCodes has signed up to do that one.)