Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd a `let...else` expression, similar to Swift's `guard let...else` #1303
Conversation
mbrubeck
added some commits
Sep 30, 2015
mbrubeck
force-pushed the
mbrubeck:if-not-let
branch
from
8cd47d4
to
cf95a3b
Oct 2, 2015
mbrubeck
closed this
Oct 2, 2015
mbrubeck
reopened this
Oct 2, 2015
This comment has been minimized.
This comment has been minimized.
|
This would be awesome to have. I often want to check error conditions and early-return at the start of a function and have the rest of the code be clear and un-indented. I'm usually pretty conservative when it comes to new syntax sugar, but I've wanted this exact feature so many times. Importantly, it can un-nest the core logic of even a simple function: fn foo() {
if let Some(x) = bar() {
// body goes on for a while
// .
// .
// .
// .
// .
// .
} // wait, why am I doubly nested here?
}vs fn foo() {
if !let Some(x) = bar() { return; }
// body goes on for a while
// .
// .
// .
// .
// .
// .
// end as usual
}In my experience this reduces the cognitive load of reading such code. The error conditions are out of sight and out of mind while reading the body. In the former case the error condition lingers in the form of nesting, and possibly an else block, and it only gets worse in many real examples with multiple things to be unwrapped and matched. Before the inevitable bikeshed sets in, I'll just say it's not that important whether we really use |
This comment has been minimized.
This comment has been minimized.
zslayton
commented
Oct 2, 2015
|
This looks cool! I wonder though: if the primary use case would be fn main() {
let maybe: Option<usize> = None;
if let None = maybe {
println!("It was 'None'.");
}
let result: Result<(), ()> = Err(());
if let Err(_) = result {
println!("It was 'Err'.");
}
}? Definitely neat for more complicated types! |
This comment has been minimized.
This comment has been minimized.
|
@zslayton Your examples don't allow the user to get the "success" value out of the |
This comment has been minimized.
This comment has been minimized.
|
Doesn't look bad, though I'm concerned about the implemention. Detecting divergence is easy enough (it more-or-less already exists, you'd just not get particularly nice error messages) but it's the rest that I'm worried about. It can't be implemented by desugaring, as it has an effect on the entire function, not just the little part it occupies syntactically. As such, this isn't really an extension of the current Instead, it is much closer to |
mbrubeck
added some commits
Oct 2, 2015
This comment has been minimized.
This comment has been minimized.
|
This feels too different from |
This comment has been minimized.
This comment has been minimized.
|
While The "must diverge" aspect is also unusual, but it shows that this proposal is firmly about returning early in "error-like" cases. We already have this for the
I have an experiment that does 2~4 with Sufficiently Advanced Generics. 5 could be added easily. It also converts to/from For the 1 case, maybe |
This comment has been minimized.
This comment has been minimized.
olivren
commented
Oct 2, 2015
|
I find it disturbing that this new form of
Also, I don't understand why there should be a requirement about the divergence of the body, except for the fact that it would be a common use case for this construction. As an alternative, you can add that the same result can be achieved using this construct:
|
This comment has been minimized.
This comment has been minimized.
|
@olivren The |
This comment has been minimized.
This comment has been minimized.
|
I'll present an alternative syntax that might get the point across better: // Unlike in a regular let, this pattern must be refutable, not irrefutable.
let Some(a) = x else { return; }
// use `a` hereThe above would be equivalent to this RFC's: if !let Some(a) = x { return; }
// use `a` here |
This comment has been minimized.
This comment has been minimized.
olivren
commented
Oct 2, 2015
|
@tsion Oh I missed that |
This comment has been minimized.
This comment has been minimized.
|
I like @tsion's alternate syntax. FWIW, in clippy we have https://github.com/Manishearth/rust-clippy/blob/master/src/utils.rs#L266, which works pretty well I'm a bit uneasy that this introduces a branch without indentation. It's a bit jarring to read. |
This comment has been minimized.
This comment has been minimized.
|
I feel like it's the same as: if !some_important_condition() { return; }And the error handling block could be placed on the next line and intended as normal, etc. |
This comment has been minimized.
This comment has been minimized.
|
Yeah, but returns/breaks/continues are pretty well known across the board. This is ... strange, and cuts into our "strangeness budget as @Havvy mentions. |
This comment has been minimized.
This comment has been minimized.
|
I have to say I'm surprised at the references to the strangeness budget, because I find this to be a very natural extension of the language (at least with a sufficiently bikeshedded syntax). The essence of the idea is this: Then you need a block to handle the failure case, hence the (Note that this is a different sense of " |
This comment has been minimized.
This comment has been minimized.
|
The main "strange" bit is that it introduces nonlinear flow, in a novel way. return/break/continue are known to behave that way. They also have obvious points to jump to. fn foo() {
if something{
if !let ... {}
if !let ... {}
}
}if one of them fails, where does it jump to next? It's not immediately obvious. return/break/continue are known constructs, and everyone knows where they jump to. This is completely non-obvious on the other hand, especially for people coming from other languages. |
This comment has been minimized.
This comment has been minimized.
|
@Manishearth But the bodies of those |
This comment has been minimized.
This comment has been minimized.
|
Oh, right. In that case, I'm mostly okay with this. |
This comment has been minimized.
This comment has been minimized.
|
To be specific, for anyone else reading, |
This comment has been minimized.
This comment has been minimized.
|
(Or |
This comment has been minimized.
This comment has been minimized.
|
I like the @tsion’s example above and think the syntax could be expanded even further to subsume
|
kennytm
referenced this pull request
Nov 14, 2017
Closed
Allow pattern matching after guard clause #45978
This comment has been minimized.
This comment has been minimized.
|
Another possibility to throw out there, now that we can add keywords: unless let Statement {
source_info,
kind: StatementKind::Assign(lvalue, Rvalue::BinaryOp(_, lhs, rhs)),
} = bin_statement
{
continue;
};
... do stuff with the 4 bindings ...Which would desugar to if let Statement {
source_info,
kind: StatementKind::Assign(lvalue, Rvalue::BinaryOp(_, lhs, rhs)),
} = bin_statement
{
... do stuff with the 4 bindings ...
} else {
let _: ! = {
continue;
};
}Which demonstrates that we already have sufficient language support for checking that a block is divergent, and I think having to add Edit: Oh, glaebhoerl already suggested that two years ago in #373 (comment) |
This comment has been minimized.
This comment has been minimized.
comex
commented
Nov 21, 2017
|
Meh. My sentiment is the opposite of @glaebhoerl's from a few months back: these proposals are going to contortions to work around a problem that would be more elegantly solved by making …Or I could be wrong, since it's not like I've tried it out. But I don't think so. |
This comment has been minimized.
This comment has been minimized.
|
@comex The problem of using // example 1
let x: Result<i32, i32> = Ok(3);
let Ok(y) = x else { return };
println!("{:?}", y); // 3However, in // example 2
let x: Result<i32, i32> = Ok(3);
if !let Ok(y) = x {
println!("{:?}", x); // this should be acceptable right?
// (not diverging)
}
println!("{:?}", y); // `y` cannot be defined here. |
This comment has been minimized.
This comment has been minimized.
burdges
commented
Nov 23, 2017
•
|
We've had a long thread with many strong objections to the I suggested in the guard clause flow typing thread that we wait and watch the parser performance between Rust and Puffs. I think folks will care if Puffs sees performance benefits attributable to their usage of subtyping and refinement types and/or their assertion/proofs business. If we wind up wanting refinement types for performance reasons anyways, then they might produce syntax usable in the |
This comment has been minimized.
This comment has been minimized.
|
What does Puffs have to do with the gained expressivity of I long for that feature every day. |
This comment has been minimized.
This comment has been minimized.
|
How about let match Ok(x) = foo() {
Err(e) => return Err(e),
}
// x is in scope hereThis way you have access to the information that didn't match in the " |
This comment has been minimized.
This comment has been minimized.
hauleth
commented
Jan 12, 2018
|
@canndrew what would be advantage over let x = match foo() {
Ok(x) => x,
Err(e) => return Err(e)
}Because it seems negligible. |
This comment has been minimized.
This comment has been minimized.
|
@hauleth |
This comment has been minimized.
This comment has been minimized.
burdges
commented
Jan 12, 2018
|
I find I still think refine gives the most elegant answer:
And |
This comment has been minimized.
This comment has been minimized.
|
This RFC introduces something that is more expressive than Why would the What is the difference between the two following expressions in your examples? let y = refine bar { None => None };
let Some(y) = refine bar { None => None };The two being apparently allowed looks weird to me. |
This comment has been minimized.
This comment has been minimized.
burdges
commented
Jan 13, 2018
•
|
In the first,
|
This comment has been minimized.
This comment has been minimized.
|
@burdges This is highly complicated, without any precedent, requires rustc to create subsets of enum types and whatnot on the fly (because the non-diverging path may have This also still seems completely unrelated to |
This comment has been minimized.
This comment has been minimized.
burdges
commented
Jan 13, 2018
|
Yes it be complicated. Right now, it's an argument against doing any |
This comment has been minimized.
This comment has been minimized.
Pzixel
commented
Mar 6, 2018
•
|
I really want this RFC to be accepted in its original form. Then, when you write: if !let Some(x) = get_some() {
return Err("it's gone!");
}
println!("{}", x); // and other useful stuff
// more stuffwhich translates into if let Some(x) = get_some() {
println!("{}", x); // and other useful stuff
// more stuff
}
else {
return Err("it's gone!");
}Yes, it's a bit uncommon that instead of been declared in block scope |
This comment has been minimized.
This comment has been minimized.
darkwater
commented
Apr 17, 2018
|
I feel like the |
This comment has been minimized.
This comment has been minimized.
eonil
commented
May 13, 2018
•
|
IMO, Swift
They're specifically designed for check and forced early-exit logic. And IMO, new relatively unfamiliar keyword
using of IMO, just using
Series of Or just using nothing would be better for extreme brevity by sacrificing readability.
As it's clear that the name |
glaebhoerl
referenced this pull request
Jun 27, 2018
Open
Explicit refutable `let`: `let`..`else` #373
This comment has been minimized.
This comment has been minimized.
|
In a similar vein to this RFC I've seen proposals to allow
I've also seen people propose allowing
I think the lesson here is that we could unify and generalize all these proposals to allow a whole algebra of binding conditionals where A
With this system you could write stuff like
|
This comment has been minimized.
This comment has been minimized.
Pzixel
commented
Aug 6, 2018
•
|
This comment has been minimized.
This comment has been minimized.
nielsle
commented
Aug 6, 2018
•
|
@canndrew Interesting idea. AFAICS rust shouldn't allow matches on both branches of an if-statement. As an example the following code looks meaningful, but also slightly confusing. if !let Some(x) = foo || let Some(y) = bar || let y=2 {
// Do something with y
return
}
// Do something with xNitpick: In your example you cannot do stuff with |
This comment has been minimized.
This comment has been minimized.
Yeah, it's actually impossible with just
I'm not sure I follow. If I guess this shows that this syntax has the potential to be too confusing though, |
Centril
added
A-syntax
A-expressions
A-control-flow
labels
Nov 27, 2018
This comment has been minimized.
This comment has been minimized.
christopherswenson
commented
Mar 11, 2019
•
|
It seems to me like we need a real guard statement in addition to It would be very unintuitive (and would probably lead to much confusion) if
added While this is still useful even if it doesn’t add bindings to the outer scope, it doesn’t solve the nesting problem that guard statements address. I agree with @eonil that Swift’s guard style is the most appropriate:
I think it’s abundantly clear what’s going on here. Additionally, it should be possible to have a guard statement without a let:
|
This comment has been minimized.
This comment has been minimized.
Pzixel
commented
Mar 11, 2019
|
Used this feature yesterday in real C# code: private Task ValidateTransactionAsync(ILogger logger, RabbitMessage message)
{
if (!(message is ValidationRabbitMessage validationMessage))
{
logger.Error("Cannot validate message {@ValidationMessage}", message);
return Task.CompletedTask;
}
switch (validationMessage.Message)
{
case Request request:
return ValidateRequestAsync(logger, request, validationMessage.TransactionHash);
case RequestStatusUpdate requestStatusUpdate:
return ValidateRequestStatusUpdateAsync(logger, requestStatusUpdate, validationMessage.TransactionHash);
default:
throw new InvalidOperationException($"Unknown message type '{message.GetType().Name}'");
}
}Still waiting this feature. |
mbrubeck commentedOct 2, 2015
Rendered
Note: The initial version of this RFC used the syntax
if !let PAT = EXPR { BODY }. In the current version this has been changed tolet PAT = EXPR else { BODY }for reasons discussed below.