-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Proposal: Else clauses for for and while loops #3163
Conversation
This would be redundant given RFC rust-lang/rust#63177 |
Perhaps I'm just tired - it's 11:30 PM here - but I fail to see the connection between that pull and this proposal |
See https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.try_find
Your motivating example is
Specifically for the motivating example, you only need let found = if let Some(value) = data.iter().find(|value| *value == target) {
true
} else {
println!("Couldn't find {}", target);
return;
}; If you think this is too complex, which is not at all, you can use the syntax proposed in the recently-merged let Some(found) = data.iter().find(|value| *value = target) {
true
} else {
println!("Couldn't find {}", target);
return;
} If you only care about whether it's found, regardless of the actual matched item, it's simply if data.iter().find(|value| *value = target).is_none() {
println!("Couldn't find {}", target);
return;
} For a more general case: try_find For an
So you can write something like: if let Ok(Some(needle)) = hay.iter().try_find(|value| Ok(value == needle)) {
/* do something */
} else {
/* do something else */
} |
The PR contains the following examples, which can all be written in a concise and readble way without proposed new syntax: Proposed syntax: let x = for i in 0..10 {
if (...) {
break i;
}
} else {
11 // default value
}; With let x = (0..10).find(|v| Ok(v == target)).unwrap_or_else(11); Proposed syntax: let x = while (...) {
// do work
if (...) {
break Some(value);
}
} else {
Some(42) // default value
}; With use std::iter::successors;
let x = successors(Some(initial_value), |v| {
Some(/* do something on v */)
}).try_find(|v| {
if /* outer conditional */ {
Ok(/* inner conditional */)
} else {
Err()
}
}).unwrap_or_else(42); You can use use std::iter::successors;
let x = successors(Some(initial_value), |v| {
if /* check on v */ {
Some(/* do something on v */)
} else {
None
}
}).last().flatten().unwrap_or_else(42); |
I feel like using dedicated syntax for this instead of just iterator adapters does have promise, and could help in a fair few numbers of cases where such adapters aren't possible, e.g. easily returning to the outer function or async code. I dunno. I like filling in this gap where |
Honestly would rather have #![feature(label_break_value)]
fn main() {
let x = 'foo: {
for &a in [1, 2, 3].iter() {
if a % 2 == 0 {
break 'foo a;
}
}
0
};
} In fact, you can do this today, on nightly: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=51e6caacaf3e6fae2d6dd68dfbc56c77 |
Rejected previously in #961 (comment) and #1767 (comment) |
Both comments state that they're open to revisiting this. |
I'm open to changing the |
That said: Here's a piece of code from one of my own projects (a Connect 4 AI). I cannot see a simple way of transforming this into any of the cases you propose; however it uses the exact pattern for/else is supposed to improve (in this case reaching the let owner = col.pieces[y];
let mut found_line = true;
for offset in 0..4 {
let col = &self.cols[x + offset];
if col.count < y + offset {
found_line = false;
break;
}
if col.pieces[y + offset] != owner {
found_line = false;
break;
}
}
if found_line {
return Some(match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
});
} I'd be interested in seeing how complex the functional approach can get with a real example (although obviously you do not have to write code for me if you do not want to) |
let owner = col.pieces[y];
let found_line = &self.cols[x..x + 4]
.iter()
.enumerate()
.find(|(offset, col)| col.count < y + offset || col.pieces[y + offset] != owner)
.is_none();
if found_line {
return Some(match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
});
} You can make it shorter: let owner = col.pieces[y];
&self.cols[x..x + 4]
.iter()
.enumerate()
.find(|(offset, col)| col.count < y + offset || col.pieces[y + offset] != owner)
.is_none()
.then(|| match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
}) |
If you consider the label-break-value desugaring: #![feature(label_break_value)]
fn main() {
let x = 'foo: {
for &a in [1, 2, 3].iter() {
if a % 2 == 0 {
break 'foo a;
}
}
0
};
} then a good keyword for it would be "then": fn main() {
let x =
for &a in [1, 2, 3].iter() {
if a % 2 == 0 {
break a;
}
} then {
0
};
} but do we really want to add a new keyword vs stabilizing an existing, already-implemented feature? (Also this is impossible to get wrong: if you forget to break the label you get an error ^-^) |
The “Prior art” section gives really good reasons not to have this syntax.
The Go proposal mentioned in the RFC got 23 👎 and no support vote, while the Julia proposal has been open since 2012 with little activity. This seems to indicate that people generally feel no need for such syntax.
If only 1 out of 4 Python users know what it means, and 1 out of 2 think it means something else, this indicates to me that copying this syntax with this semantic is a very bad idea. It's too niche for most users to know, and not intuitive for users who don't know. |
As Rust is a systems language, I would expect that more people would have an understanding of the desugar (at its core, a while loop just uses a guard to
I believe I mentioned this but the reason @lebensterben: It takes me far longer to parse that code than the equivalent for-else construct (which is really the point of the construct) - I can just about manage but it is definitely harder to read, which kinda supports my point. I'd be interested in a poll between the ❤️ let owner = col.pieces[y];
for offset in 0..4 {
let col = &self.cols[x + offset];
if col.count < y + offset {
break;
}
if col.pieces[y + offset] != owner {
break;
}
} else {
return Some(match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
});
} 👀 let owner = col.pieces[y];
let found_line = &self.cols[x..x + 4]
.iter()
.enumerate()
.find(|(offset, col)| col.count < y + offset || col.pieces[y + offset] != owner)
.is_none();
if found_line {
return Some(match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
});
} 🚀 'found: {
let owner = col.pieces[y];
for offset in 0..4 {
let col = &self.cols[x + offset];
if col.count < y + offset {
break 'found;
}
if col.pieces[y + offset] != owner {
break 'found;
}
}
return Some(match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
});
} 😕 let owner = col.pieces[y];
&self.cols[x..x + 4]
.iter()
.enumerate()
.find(|(offset, col)| col.count < y + offset || col.pieces[y + offset] != owner)
.is_none()
.then(|| match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
}) ETA: did a quick poll of my (small) office and all of them agreed the first is clearest |
This comment has been minimized.
This comment has been minimized.
I actually do like the idea of A for-then or while-then gets rid of that problem. I don't think that adding in an extra keyword should be seen as that big of a detriment, but we technically could use To me, for-then says "do this loop, then that" and it's not too difficult to understand that a break would skip the then. Whereas for-else is "do this loop, otherwise do this if it doesn't break," which is much harder to understand. |
What would you think about the following instead: for i in 0..2 {
// code
} broke (value) {
value
} else {
other_value
} The Does that seem like an acceptable compromise? N.B. The syntax for the |
Now that I'm thinking about it more, I actually like |
The "else" block avoids storing extra state whereas the "broke" block just calls a function for every broken value. |
That's the primary reason why it would be optional; in most cases for/else would be used but in cases with complicated exit code the |
#3152 suggests let owner = col.pieces[y];
for offset in 0..4 {
let col = &self.cols[x + offset];
if col.count < y + offset {
break;
}
if col.pieces[y + offset] != owner {
break;
}
} nobreak { // `k#nobreak` before Rust 2024 if RFC 3098 is merged
return Some(match owner {
Piece::White => GameResult::WhiteWin,
Piece::Black => GameResult::BlackWin,
});
} Personally, I'd still prefer the iterator or |
As someone who has been programming in Python for ~18 years and is familiar with the Python equivalent to this proposal, I'd say that Rust being a systems language is more reason not to implement such a feature. Rust is already a fairly complex language and we don't want to become C++. This doesn't feel like it benefits enough to be worth what it costs to the language's complexity budget. Heck, as someone who was introduced to this kind of In my experience, they're just too niche to be worth it. |
For the record, I would expect a In python, it has this rfc behavior: https://book.pythontips.com/en/latest/for_-_else.html (which seems controversial) The |
@rfcbot fcp close I thank the author for taking the time to prepare the RFC, but I tend to agree with those who have said that the history of this feature in Python is actually evidence against its inclusion. It's just a bit too niche and confusing, and the meaning of |
Team member @nikomatsakis has proposed to close this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
In Python I find this feature both useful (as this RFC says at the beginning, it's a common pattern that is cumbersome to solve with a boolean) and also very confusing. I can never remember that So yeah, I agree this is a problem, but I do not support using |
@durka I agree entirely with that characterization: we need a recommended, idiomatic, Rustic solution to the underlying problem (handling loops that don't find what they're looking for), but not |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
It’s a shame that something so apparently hastily written was submitted here. I had been drafting a much more elaborate proposal that actually addresses the Python objection. But now that this is being rejected, it makes me doubt it’s going to have a fair hearing. |
@fstirlitz I don't hold prejudice against a concept based on prior proposals and I don't get the impression Rust team members do either. Your idea is certainly better, with (eg. What about |
@fstirlitz |
@lebensterben Fair, I might need to look for something more elaborate than @ssokolow I would hope so, but I do recognise proceeding RFCs comes with its own fatigue. At some point people may get tired of the whole subject. The primary motivation for an Also, adding a loop-exhaustion block is a pretty natural modification of the current loop desugaring, in a way in which singling out no-iterations is not: where If there is a burning need, one may attempt to use macros to build an equivalent of the * FootnoteAlthough i would have been pretty easy to express with the non-reducible/state-machine loop construct I discussed once: let mut it = /* obtain iterator */;
fsm_loop First(it.next()) {
First(Some(item)) => {
continue Next(Some(item));
},
First(None) => {
/* no iterations */
break;
},
Next(Some(item)) => {
/* got an item */
continue Next(it.next());
},
Next(None) => {
/* exhausted after non-zero iterations */
break;
}
} |
Note that For example, that "you can write your own Iterator::find" example becomes let found = haystack.into_iter().try_for_each(|item| try {
if predicate(item) {
return ControlFlow::Break(item);
}
}).break_value(); And in general the So for-else feels like it's a strange intermediate, to me. It's more complex than the basic structured control flow stuff, but also not particularly generalizable to more complex situations. I think I'd be inclined more towards leaving it out, but having either label-break-value or the more elaborate custom-fsm construct instead. The loop-break-value version is let found = 'found: {
for item in haystack {
if predicate(item) {
break 'found Some(item);
}
}
None
}; which is nearly exactly the same length as the (And you can make a stable version of that by just adding a
I wouldn't worry too much about that. Take #1603, for example, which was one of the more heavily 👎'd RFCs on this repo and was not accepted. But the same idea came back a year and a bit later as #2113 where it was well-received, and it's soon going to be required, with edition 2021. If there's a change that's a good-enough fit, it'll happen, even if similar things were rejected many times before. |
I specifically remember being overwhelmed by the amount of options Rust has for loops when I first learned the language. With the existence of standard
While I personally find the proposed alternatives using Iterators to be very unreadable, the |
That’s 461 words spent on an argument that boils down to ‘but it’s not very familiar to the average programmer’. To which I reply: unfamiliarity is temporary. This is not a difficult concept to grasp once explained, and it can be explained in less than two minutes. Someone who has never seen Rust before is going to spend much more time on lifetime annotations than on this relative triviality. You’d have to be pretty selective with your snippets already if you want to find something that will be immediately obvious to someone entirely unfamiliar with Rust. (And even then you’re making assumptions about them being familiar with something else. To someone unfamiliar with any programming, all code is incomprehensible.) The same argument could have been made about the |
Without reading the reference-level explanation (which shouldn't be necessary to understand your proposal), I sincerely have no idea what is even being proposed. I am aware that other languages have for/else & where/else, but have never seen it used in the wild (probably because nobody knows what it does). The comparison to pattern matching with You stated five days ago that "now that this is being rejected, it makes me doubt it’s going to have a fair hearing." It has not yet been rejected, as FCP has not yet completed. There is a ten day period for a reason. If there are clear and obvious reasons to include this, then it should be so stated. But when an experienced programmer has no idea what's even being proposed, that by itself is reason to close in my opinion. |
The final comment period, with a disposition to close, as per the review above, is now complete. As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed. The RFC is now closed. |
Did I do this right?