-
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
RFC: let-expression #3159
RFC: let-expression #3159
Conversation
I'm against the now-merged For example:
In a general case, if it's not The But with the new syntax, For example, this allows the following readability nightmare: pub mod XXX {
pub enum YYY { Foo(i32), Bar(u32) }
pub struct Baz (i8);
}
use XXX::YYY::*;
use XXX::Baz;
fn nightmare () {
let (a, b, c) = (Bar(0), Foo(0), Baz(0));
// Here we go
(let Foo(x) = a) || (let Bar(x) = b) || (let Baz(x) = c) || { return; };
} The reader gotta process the entire expression and internalize that the first two bindings may fail, but then, wait, This is nowhere being any better for new learners. It's confusing and error-prone. |
@lebensterben I don't see your concern. The user can write this today: if let Baz(x) = c { } else { } and just get a warning for unreachable code, which is good in my opinion. Irrefutable patterns in let expressions are like |
## Consistency | ||
|
||
Currently we have `if`, `if let` and `if let && let` and we teach them as three different | ||
constructs (plus `let else` in future). But if we make `let` a `bool` expression, all will become the same and it would be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is something about this RFC that made me unsure about whether this change will be good, however I am pretty sure that it doesn't really matter if let pat = expr
becomes a bool
expression in this RFC. It can be pretty confusing with the PBs and NBs seemingly attached to a single bool
value. Now if we replace all the checking that is needed for bool
returned by let expressions with simply syntactic sugar, we get let else
as well as if let chain
. Is this RFC just about making let
a bool
expression, as well as a wrapper on top of the concepts from if-let-chains
and let-else
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reasoning here is, new learners when see if let
, expect that let
should behave like a bool expression (like anything else inside if
scrutinee) and this confusion will grow if we chain them with &&
in if-let-chain and ||
in let-else. By making them an actual bool
expression, there will be more things to learn (about PB and NB), but there would be no surprise for how let
statement jumped into if
and &&
because it is a bool
expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do new learners actually expect let
to be a bool expression? The issue you linked in the RFC did not have any mention of bools. And new learners will get confused by the concepts of PBs and NBs too. You clearly have a bias towards if-let-chain and let-else because you describe their syntax as confusing, but as "more things to learn" when you talk about your RFC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yah, the linked issue was renamed and labeled as a bug in error reporting, the error wasn't helpful to the end user and had nothing really to do with booleans at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting ()
around let in if let
has it's roots in expecting if
is a isolated structure and let
is an expression inside it (which should be bool
because it is inside if scrutinee). New learner can soon discover that if let
is another construct and is attached together, but situation is worse with introducing &&
and if-let-chain. I'm not inventing this and this thread in internals has a discussion about confusing let
with an expression. Now there is two approach:
- Try to prevent this confusion, detect errors that mentions let expression as compiler bug (like issue I linked), and restrict new RFCs that use
let
in this way (like if-let-chain) and memorize special cases as different constructs. (current approach) - Make
let
an actualbool
expression. This needs generalizing binding rules to cover if-let and if-let-chain as a subset, and it will probably becomes a complex and confusing rule, but there would be no corner case andif
will become a general thing capable of handling if-let and if-let-chain and more things. (the approach of this RFC and @Centril in that internal thread)
Second approach has costs. But cost of first approach (confusing let
with an expression) is also real and I'm not the only person saying it. And boolean doesn't need to be mentioned explicitly and mentioning let expression is enough, because bool
is the only type that makes sense in context of if-let and if-let-chains.
text/0000-let-expression.md
Outdated
|
||
// with let expression | ||
(let Some(x) = a) || (let Some(x) = b) || (let Foo(x) = bar) || { return; }; | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address this concern: aggressively mixing &&
s and ||
s is accepted by the compiler, but it can just be hard to read:
(((let Some(x) = a) && (let Some(y) = x.transform())) || { panic!("failed to get y") }) && ((let Some(a) = y.transform1()) || ((let result = y.transform2()) && ((let Ok(a) = result) || { return result; })) || ((let either = y.transform3()) && ((let Either::Left(left) = either) && (let a = transform_left(left))) || ((let Either::Right(right) = either) && (let a = transform_right(right)))));
beautified:
(
(
(let Some(x) = a) &&
(let Some(y) = x.transform()))
|| { panic!("failed to get y") }
) && (
(let Some(a) = y.transform1()) || (
(let result = y.transform2()) && (
(let Ok(a) = result) || { return result; }
)
) || (
(let either = y.transform3()) && (
(let Either::Left(left) = either) && (let a = transform_left(left))
) || (
(let Either::Right(right) = either) && (let a = transform_right(right))
)
)
);
(and it looks like I have mismatched parenthesis)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compiler won't accept your code as-is, by rules of bindings. But I added your concern with a weaker example here: https://github.com/HKalbasi/rfcs/blob/master/text/0000-let-expression.md#hard-to-read-let-expressions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That counterargument is pretty weak to me. The second example with "complexity in patterns" is definitely less complex. At least I know the names of variables that are bound immediately by just looking at the first line of let ...
, and I know exactly where each pattern matches on each expression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is less complex, but it is also doing less work. And my point is this complexity can be scaled to infinity and you can write a 10 lines, confusing pattern.
At least I know the names of variables that are bound immediately by just looking at the first line of let ...
It is valid, but discovering which variable is bound isn't hard at all. In fact, all bindings in top-level of expression will be bound. In my example, they are x
and y
and a
. In your example, also result
and either
are in top level, but your example doesn't compile.
I know exactly where each pattern matches on each expression.
Things won't be that easy if I add another |
in pattern:
let (
((Foo(x), Some(y), (Some(a), _, _) | (_, Ok(a), _) | (_, _, Some(a)), _)
| (_, (Bar(y) | Foo(y), Bar(a), (Some(x), _, _) | (_, Err(x), _) | (_, _, Some(x)))
And in let expression we see which expression is in front of which pattern and this help readability. (Maybe I didn't understand your point, if it is the case an example can help)
My argument is "patterns can similarly and equally make infinitely complex examples", maybe I should change RFC text with better examples.
Isn't the |
It is short circuited but since both side of |
Regarding "let as a bool expression", it's worth noting that this isn't necessarily the only or most obvious thing a let expression might evaluate to. Expressions like In Javascript Edit: Somehow I forgot Javascript also has a |
If I submitted something like (let Some(x) = a) || (let Some(x) = b) || (let Foo(x) = bar) || { return; }; to a code review; it would be rejected as unreadable. Phrases like "keep it simple" would be used. Even if I understand it today, am I going to understand it in six months? The fact we can introduce this feature is not a justification for including it. |
@joew60 What is unclear in your example that you may not understand it in six months?
Certainly. The justification for this RFC is that it unites features like if-let, if-let-chain and let-else, which are known to be useful, in a general and consistent manner, and then get some extra features for free (with no adding to grammar and concepts of the language). At the end there will be some complex usages (which I don't believe your example is among them) that code reviewers can reject. |
That's how I feel about most of the examples in the RFC. I think this RFC is too focused on what is theoretically possible instead of what is useful, readable, practical, and realistic. Cramming more logic into a single statement is not an improvement. Code that looks like minified JavaScript is bad code. I don't feel great about this RFC, but I might consider it more seriously if it had a more practical lens. |
text/0000-let-expression.md
Outdated
Which by a generalized let-else can become: | ||
```rust | ||
for w in block.stmts.windows(2) { | ||
(let StmtKind::Semi(first) = w[0].kind) | ||
&& (let StmtKind::Semi(second) = w[1].kind) | ||
&& !differing_macro_contexts(first.span, second.span) | ||
&& (let ExprKind::Assign(lhs0, rhs0, _) = first.kind) | ||
&& (let ExprKind::Assign(lhs1, rhs1, _) = second.kind) | ||
&& eq_expr_value(cx, lhs0, rhs1) | ||
&& eq_expr_value(cx, lhs1, rhs0) | ||
|| continue; | ||
// 30 lines of code with two less tab |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is more like a practical example of the existing if_let_chains
feature (if you must include a continue
for the number of indentations)
if let StmtKind::Semi(first) = w[0].kind
&& let StmtKind::Semi(second) = w[1].kind
&& !differing_macro_contexts(first.span, second.span)
...
&& eq_expr_value(cx, lhs1, rhs0)
{
} else {
continue;
}
this is also more consistent with the way people would write
if next_condition {
continue;
}
rather than
!next_condition || continue;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if-let-chain still need one indentation, because variables are bound inside of if body (like if-let) but in the example with ||
variables are bound after statement (like let-else). If you believe if-let can't meet the needs that let-else provide, The same statement is true for if-let-chain and this RFC.
this is also more consistent with the way people would write
I am agree and prefer explicit if
over ||
in normal cases. Without bindings, both behavior is the same (both in current rust and in this RFC) but because binding rules for them are different (for good reasons; for example accessing if-let variables after body is surprising) you can save one indentation by ||
one.
And it is worth noting that !next_condition || continue;
is completely valid and working in today rust. But !next_condition else { continue };
isn't and won't, so let-else syntax has a negative point here and this RFC is more consistent with rust.
Currently we have `if`, `if let` and `if let && let` and we teach them as three different | ||
constructs (plus `let else` in future). But if we make `let` a `bool` expression, all will become the same and it would be | ||
easier for new learners to get it. After this RFC you get only an unused parenthesis warning | ||
for `if (let Some(x) = y) { }`, not a hard error. And it will have the same behavior with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it's worth adding a helpful error message for this even if the RFC is not accepted. We could even consider making the compiler recover, but with a warning, though I'm not sure that's the way to go. I haven't checked what error message is given currently though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think recovering with a warning is good, especially in if-let-chains because people may want to sure that &&
isn't interpreted wrong so add a (unnecessary) parenthesis. But it will make demand for let expression even more, because users will say: What is this let
that I can put it in if
with parenthesis and &&
it inside if, but nothing else?
@camsteffen I added some practical examples in this section and I probably add some more when I find.
This is minified JS: (picked from this page assets) !function(){"use strict";function e(e){const t=[];for(const o of function(){try{return document.cookie.split(";")}catch(e){return[]}}()){const[n,r]=o.trim().split("=");e= Is that example really looks like minified JS? What are their similarities? Just because it is in one line, it looks like minified JS? You can put it in multiple lines: (let Some(x) = a)
|| (let Some(x) = b)
|| (let Foo(x) = bar)
|| { return; }; And the point of this is not count of lines. You can put current alternative in one line as well: let x = if let Some(x) = a { x } else if let Some(x) = b { x } else if let Foo(x) = bar { x } else { return; }; Or in multiple lines: let x = if let Some(x) = a {
x
} else if let Some(x) = b {
x
} else if let Foo(x) = bar {
x
} else {
return;
}; Either way, current solution has unnecessary noise, two times more Maybe you can explain your feelings and what you mean from minified JS by some examples. |
JS is a very flexible language and allows you to write very dense code. Perhaps this is a better fit for the goals of JS. But I don't want to dwell on that comparison. The point is that the code is hard to read. That's just my own subjective feeling from looking at the examples. |
@camsteffen minified JS is a machine generated and unreadable by design code, Which doesn't related to that examples I think. JS is flexible but you can minify rust as well. Anyway, it is not our discussion, lets move on.
Can you please explain your feeling in more details? These questions may help:
In each case, if it is applicable, please provide which is wrong, for example bindings are unclear, side effects are unclear, ... It isn't necessary and you can simply don't like anything you want. |
Using a top level To me this: (let Some(x) = a)
|| (let Some(x) = b)
|| (let Foo(x) = bar)
|| { return; }; is way harder/surprising to read than if let Some(x) = a || let Some(x) = b || let Foo(x) = bar {
return;
} |
@kennytm Hm, but the logic to fail compilation if the expression fails to diverge does seem to answer your question:
Assuming divergence and double-checking later does solve that issue. The answer for the example you gave is thus that (some) z is always in scope, and m is always a I'm still in agreement that it is hard to explain, and that this is a bad idea, but your example does have a specified result :). |
On Mon, Aug 23, 2021 at 01:37:59PM -0700, HKalbasi wrote:
@fee1-dead
> || is widely understood as logical OR, And there is something about that: else precedes behavior. "if this condition is met, do this, or else do that".
Logical OR is `|`.
`|` is bitwise OR, not logical OR. `||` is logical OR.
I'm not the one who invented or-else for calling `||`, it exists because logical OR doesn't explain all aspects of `||`.
Logical OR does explain all aspects of `||`: if the first expression is
false, the whole expression can't possibly be true, so there's no point
in evaluating the second expression.
More importantly: it feels like you have a very specific conception of
how people "should" read `||`, and you're looking to educate people to
accept your interpretation. In general, we try to make it our very last
resort to present something unusual and just ask people to memorize and
go with it. Any time we can leverage people's existing understanding,
intuition, experience, or similar, we will often prefer to do so.
In that regard: nobody is arguing that it's not *possible* to use `||`
in the way you're proposing. However, I believe (and it appears that
others do as well) that `else` will align better with people's
exising intuition and experience.
Likewise, making a let binding behave like a boolean expression is
*possible*. I don't think we *should*, however.
> but about whether it will be straight-forward and easy to learn for beginners.
I can see people may become surprised if they are not familiar with short circuiting behavior of `||`.
It's possible to be familiar with the short-circuiting behavior of
`||`, and still find it awkward to use for control flow. It feels more
like the territory of C `*x++ = *y++`, or Perl one-liners; it doesn't
feel like Rust, at least to me. It feels like a construct that puts *more*
priority on having a single language construct and making it work for
everything, and *less* priority on making code easy to read and
understand.
There will be a sugar in language (until possible generalizations) `let pat = expr xxxx { diverge; }` and it really doesn't matter what is `xxxx`.
It does matter, though. The syntax we use becomes part of people's
models and intuitions, and `else` there *feels* like it forms part of a
consistent languge model. `||` doesn't feel like as good of a fit.
(There are things that would fit even *worse*, such as `=>` or `!`, but
I feel that `else` fits *better* than `||` in this context.)
else without if isn't in any language
Python has for-else and try-else, Ruby has begin-rescue-else, and
several languages have unless-else. There's a common meaning for `else`
in all of those constructs, in addition to if-else.
|
if (let Some(z) = z) else { mystery_function_1(&m); };
(let Some(z) = z) else { mystery_function_2(&m); };
(let Some(z) = z) else { mystery_function_3(&m); };
(let Some(z) = z) else { mystery_function_4(&m); };
(let Some(z) = z) else { mystery_function_5(&m); };
m.push(z); How you see problem (and what is the problem you see) with this RFC but not with let-else?
It is not exception of RHS, but exception of diverging values. You can imagine diverging branch add a hidden z which will be in the scope for the result of function (which we won't see after diverge) |
@HKalbasi What if trait Q { type A; fn a(&self) -> Self::A; }
impl Q for Vec<i32> { type A = !; }
impl Q for Vec<Option<i32>> { type A = bool; }
impl Q for Vec<Option<Option<i32>>> { type A = !; }
...
fn mystery_function_3<T: Q>(q: &T) -> T::A {
q.a()
}
In let-else we know for sure, before type-checking, that all There is no such guarantee in this RFC, |
Edit: Deleted as I realized I was getting side-tracked. Dropping out. |
This example seems wrong, unless I’m misunderstanding something
Wouldn’t the “translation” be behaving differently if |
@joshtriplett Thanks for your detailed comment. I can answer to some parts but I want to focus on my main point.
All of my point is: if that amount of better-ness is more important than consistency between let-else like construct They are hard to compare, because they are different type of costs. But I like to see your reasoning here. To make things easier to compare, what will you decide in these scenarios? :
These aren't impossible, due the fact that people (like centril I mentioned above) was aware of let-expression. If your answers is consistent and you always choose |
It is valid as long as BOOL_VALUE binds |
@steffahn good catch, seems it is a common pitfall. I will update the example. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
In the lang meeting today, Josh pointed out that rust-lang/rust#53667 has a checkbox for thinking about syntax -- and thus things like this -- as part of stabilization for that. So I think I agree with looking at how if-let-chaining goes further before considering this more. |
@scottmcm It is ok to put thinking on this kind of thing on the time of stabilization for if-let-chain and let-else, but let-else seems is intentionally not mentioned. From meeting note:
So it should be a blocker until we declare that we want And as a general comment about |
@HKalbasi No, it shouldn't be a blocker for let-else, because that would imply that |
@joshtriplett If |
@HKalbasi You seem to be taking it as a critical point that we can't have |
The examples in this RFC are very unconvincing. There are barely less symbols and barely less rightward drift in the proposed versions compared to the current macro-based ones. I won't bash at the problems already mentioned, but I can't help but note that the problems have more idiomatic solutions. This example let Some(x) = foo() && let Some(y) = bar(x) && f(y) can be rewritten much more cleanly and idiomatically using try operator: let x = foo()?;
let y = bar(x)?;
f(y) The only problem is that it doesn't work locally since the try operator always returns from the containing function. We may want to postpone returning, and the function may not even return an try {
let x = foo()?;
let y = bar(x)?;
f(y)
} Done! Very clean, very readable, doesn't require to mess with the enclosing function. In current stable Rust one can use option-returning closures instead of try blocks, which are less ergonomic, but still a net win if you're doing a lot of matching: let opt_chain = {
let x = foo()?;
let y = bar(x)?;
let z = quux(y)?;
Some(f(z))
};
if let Some(t) = opt_chain() {
// do stuff
} The if let Some(t) = foo().or_else(|| bar()).or_else(|| quux()) {
// do stuff
} Sure, it's less readable than composition via macro_rules! option_or {
($e: expr $(, $es: expr)* $(,)?) => {
$e $( .or_else(|| $es) )*
};
}
if let Some(true) = option_or!(foo(), bar(), quux()) {
//do stuff
} Finally, there is stuff about bindings for arbitrary sum types. Again, this case is easily solved by a macro, together with the techniques above: macro_rules! option_or {
($e: expr $(, $es: expr)* $(,)?) => {
$e $( .or_else(|| $es) )*
};
}
macro_rules! bind {
( $($ident: ident),+ , $pat: pat = $expr: expr) => {
#[allow(unused_parens)]
if let $pat = $expr { Some(( $($ident),+ )) } else { None }
};
}
enum Foo {
T(bool),
H { y: u32, g: bool },
F
}
struct Bar { f1: u32, f2: bool }
let x = bind!(x, Foo::T(x) = foo())?;
if let Some((n, b)) = option_or!(
bind!(y, g, Foo::H { y, g } = baz(x)),
bind!(f1, f2, Bar { f1, f2 } = bar(x)),
) {
// do stuff with `n: u32, b: bool`
} One could probably write an even more nice looking The techniques above compose seamlessly, don't require any modification to the base language or parsing rules, and don't introduce any potential ambiguities with respect to binding scope and order. They are a bit less readable, but imho not significantly so, and not enough to outweigh the benefits. These patterns also don't look like something which should be very common in most of the projects (syntax analysis and compilers are a different case, of course). The main downside is that the compiler can't give good error messages for macros, unlike built-in let-expressions. However, this is outweighed by the fact that someone would first need to implement all that extra analysis and syntax-awareness. The macros are also quite simple and well-scoped, which should limit any possible confusion. |
My first comment ever in any Rust discussion! As someone who works with Rust and Swift daily I am interested in the outcome of this discussion. I am mostly interested in two use cases I have encountered in the code.
guard let value = result else {
return 0
}
// value is in scope. This is described as something that helps with the indentation but I find that it actually simplifies how I reson about that code. In stable Rust I don't know how to express this without nesting. So two options I can think of are either:
if !let EnumKind(value) == result {
return 0;
}
// value is in scope. I find the latter more familiar.
while let next = lines.peek(),
let nextTag = next.split(separator: "=", maxSplits: 1, ommitingEmpty: false).first,
tagSet.contains(nextTag) {
result += lines.next()
} Notice the use of comma as the separator. It makes it obvious what order the bindings happen at and that conjunction is the only option. The reson this construct is useful is only because you can use the binding from one expression in the following expressions. In Rust without while let Some(true) = lines.peek().map(|next| match next.split_once("=") {
Some((next_tag, _)) => tag_set.contains(next_tag),
_ => false,
}) {
result.push_str(lines.next().unwrap());
} My experience with large Javascript codebases is that we want to steer clear of this. Now in Rust with while let Some(next) = lines.peek() &&
let Some((next_tag, _)) = next.split_once("=") &&
tag_set.contains(next_tag) {
result.push_str(next);
lines.next();
} We're introducing if a || b && let Some(value) = value { } Which doesn't compile. But these do: if (a || b) && let Some(value) = value { }
if a || b && c { } I find this janky and confusing. I see two directions that are easy to defend:
Maybe the next step is to try a prototype of this RFC and then decide between let expressions which I see as the logical continuation of |
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. |
Rendered