search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
95.22k stars 12.28k forks source link

Tracking issue for eRFC 2497, "if- and while-let-chains, take 2" #53667

Open Centril opened 5 years ago

Centril commented 5 years ago

This is a tracking issue for the eRFC "if- and while-let-chains, take 2" (rust-lang/rfcs#2497). For the tracking issue for the immediate edition changes, see #53668.

Steps:

Unresolved questions:

Collected issues:

Implementation history:

Unresolved problems

Centril commented 5 years ago

I'm working on implementing this in a 3+ PR step fashion based on discussions with @oli-obk.

oli-obk commented 5 years ago

for the record, the discussed step list is

  1. remove if from HIR and make lowering emit a match
  2. remove if let from AST and instead have a let expression
  3. "do the rest of the work" (might get split up further).
alexreg commented 5 years ago

I can't imagine 3 would be a particularly large step (enough to merit splitting), but 3 steps sounds fair enough.

estk commented 5 years ago

@Centril I'm really jazzed about this feature, lmk if there is any work I can help with.

Centril commented 5 years ago

@estk :) The work is pretty non-parallelizable at the moment; However, once https://github.com/rust-lang/rust/pull/60861 lands you may want to take a look at https://github.com/rust-lang/rust/issues/60336.

Manishearth commented 5 years ago

Question worth discussing: This introduces let as something like an expression in the AST.

Do we plan to make it a true expression? As in, will $e:expr match let Pat(..) = foo in macros? This does not mean that it will work everywhere (as @centril correctly points out, $t:ty matches impl Trait, but it's only allowed in certain positions in the AST), it just means that macros will be able to match on it. They may error further down the pipeline if it gets used outside of an if.

This is technically a breaking behavior change; a macro with a fallthrough to let $p:pat = $e:expr will suddenly start behaving differently.

eddyb commented 5 years ago

@Manishearth I thought that you can't probe whether something parses as an AST fragment in macro_rules macros, only tokens (this includes $i:ident, $l:lit, etc.). That is, I expected the first expr/pat/stmt/block/item/etc. that is reached to be final. But I'm wrong, and this is either behavior introduced in the past couple years, or I was just confused.

petrochenkov commented 5 years ago

this is either behavior introduced in the past couple years

This is https://github.com/rust-lang/rust/pull/42913 if I'm not mistaken, almost 2 years ago. One token lookahead is used to reject alternatives that "obviously don't fit".

matklad commented 4 years ago

(not sure this is the right place for this, but I don't think we have anything else opened for "pattern matching expressions" discussion).

I've just noticed that Java is getting pattern matching along the lines proposed in https://github.com/rust-lang/rfcs/issues/929#issuecomment-285602496. That is, control-flow sensitive scoping and patterns on the right are now almost Java-level proven and trusted language design choices.

dzmitry-lahoda commented 3 years ago

any actual syntax for this in nightly rust? just cannot type it to work.

! Compiling of exercises/conversions/from_into.rs failed! Please try again. Here's the output:
error: `let` expressions are not supported here
  --> exercises/conversions/from_into.rs:44:33
   |
44 |         if (parts.len() == 2 && let Some(n) = name && let Some(Ok(a))= age) {
   |                                 ^^^^^^^^^^^^^^^^^^
   |
   = note: only supported directly in conditions of `if`- and `while`-expressions
   = note: as well as when nested within `&&` and parenthesis in those conditions

not nice 

if parts.len() == 2 { if let Some(n) = name { if let Some(Ok(a)) = age { 
            return Person{name: n.to_owned().to_string(),  age : a,}; 
        }}}
jearmstrong21 commented 3 years ago

Any progress updates? Although I love rust this is by far the jankiest part of the language

c410-f3r commented 3 years ago

I am trying to tackle this feature one bit at a time in my spare time. No promises for a concrete release date though

HTGAzureX1212 commented 3 years ago

Currently if I had to match against two different Option<T>s I'd have to:

if let (Some(smth), Some(another_smth)) = (smth_option(), another_smth_option()) {
    /* more code */
}

IMHO this works, but not an ideal way to me. I like the idea of this RFC. Looking forward to this in Rust! 👍

c410-f3r commented 3 years ago

Just to make it official

@rustbot claim

joshtriplett commented 3 years ago

@c410-f3r How goes the development on this? (No rush, just checking in.)

c410-f3r commented 3 years ago

@joshtriplett hir::ExprKind::If (#79328) was merged and hir::ExprKind::Let (#80357) will probably be merged until the end of this month. After that, I will start to actually implement the feature. Everything so far was very challenging/laborious and I am not sure if the worst is yet to come. Personally, my hopes for a solid implementation and stabilization are on the Q3~Q4 of this year.

One thing that I would like to emphasize is the help that @matthewjasper has been given me. Without him, I wouldn't be able to do half of what has already been done. Thanks @matthewjasper!

h3har commented 3 years ago

It was not clear to me reading the RFC (https://github.com/rust-lang/rfcs/blob/master/text/2497-if-let-chains.md) whether the proposed precedence rules (= having higher precedence than &&) applies to regular let statements too, or just in an if or while context.

E.g., will

let boolean_value = returns_boolean() && also_returns_boolean();

be affected and now require parentheses to compile correctly? I would assume yes...

teohhanhui commented 3 years ago

To be precise, the changes in this RFC entail that || has the lowest precedence at the top level of if STUFF { .. }. The operator && has then the 2nd lowest precedence and binds more tightly than ||. If the user wants to disambiguate, they can write (EXPR && EXPR) or { EXPR && EXPR } explicitly. The same applies to while expressions.

It does mention if and while explicitly.

foodornt commented 2 years ago

it's been 3 years, what happened to this RFC 😢

HKalbasi commented 2 years ago

I wrote a RFC draft about let expression. I like to hear comments of people involved here before submitting it officially. You can reach me in zulip or issues of my RFC fork (this is better because it is public)

HKalbasi commented 2 years ago

Now I submitted it in this PR. Thanks for your work on if let chain and I hope this RFC is a positive step in this direction and for the rust in general.

camsteffen commented 2 years ago

I opened rust-dev-tools/fmt-rfcs#169 to discuss a formatting question.

plazmoid commented 2 years ago

Why || is forbidden in let chains? I didn't find anything about it in RFC

c410-f3r commented 2 years ago

Why || is forbidden in let chains? I didn't find anything about it in RFC

https://github.com/rust-lang/rfcs/blob/master/text/2497-if-let-chains.md#keeping-the-door-open-for-if-let-or-expressions

"Not supported" is probably a better term

c410-f3r commented 2 years ago

A stabilization attempt is available at https://github.com/rust-lang/rust/pull/94927

zertyz commented 2 years ago

The 1.60 stable compiler lead me here for a condition which I, intuitively, wrote like this (a):

if let Some(x) = x && let Some(y) = f(x) {
    (1)
} else {
    (2)
}

... it said error[E0658]: 'let' expressions in this position are unstable

It seems the only alternative, by now, is to write it like this (b):

if let Some(x) = x {
    if let Some(y) = f(x) {
        (1)
    } else {
        (2)
    }
} else {
    (2)
}

not sure about the full process of going forward with this tracking issue... but, after 4 years it seems the effort is pretty much stalled... :(

I just want to vote for this to be added to the language, as the (b) solution leads to poor code...

... or... am I doing it wrong, somehow?

WaffleLapkin commented 2 years ago

@zertyz there is a stabilization attempt @ https://github.com/rust-lang/rust/pull/94927 that targets Rust 1.62 (see the comment just before yours), so I wouldn't call this "stalled".

januwA commented 2 years ago

Does this expression work in rust 1.62

if let Some(email) = data.user.email && !email.is_empty() {
    q = q.filter(users::email.eq(email));
}

I now get an error in 1.60

error[E0658]: `let` expressions in this position are unstable
jplatte commented 2 years ago

@januwA No, the stabilization PR #94927 has not been merged yet, and since 1.61 will become stable and 1.62 beta very soon, it doesn't seem likely for this feature to become stable in 1.62.

c410-f3r commented 2 years ago

~2 years, 4 months, 3 weeks and 1 day of long nights, obstacles and headaches.~

~Hope stabilization won't be reverted but regardless, thanks to everyone who helped make this feature a reality.~

EDIT: Reverted in https://github.com/rust-lang/rust/pull/100538

mousetail commented 1 year ago

~2 years, 4 months, 3 weeks and 1 day of long nights, obstacles and headaches.~

~Hope stabilization won't be reverted but regardless, thanks to everyone who helped make this feature a reality.~

EDIT: Reverted in #100538

Celebrated too soon :(

Mark-Simulacrum commented 1 year ago

Reopening this tracking issue, since it's not currently stable.

I'd also like to flag https://github.com/rust-lang/rust/pull/94927#issuecomment-1165156328 which leads me to believe that perhaps stabilization was actually premature for that reason as well? It's not clear to me that we ever had a follow-up to #97295 which actually adjusted our restrictions further, and this error leads me to believe that let expressions would have been stable in more places than I expected us to want. @c410-f3r -- can you elaborate on whether there was a follow-up to #97295 that resolved @compiler-errors concern?

Maybe that was https://github.com/rust-lang/rust/pull/98633, but given the error I mentioned in that random MIR test it seems like it may have been (still) incomplete?

c410-f3r commented 1 year ago

98633 was merged after #97295 and then the underlying solution was refined in #99731 and #100011.

issue-92893.stderr now correctly denies undesired let expressions in the parser as well as other let locations like if let Some(elem) = _opt && [1, 2, 3][let _ = ()] = 1. Therefore, it seems that everything related to https://github.com/rust-lang/rust/pull/94927#issuecomment-1099605024 is fixed.

@Mark-Simulacrum Can you close https://github.com/rust-lang/rust/issues/60707 and https://github.com/rust-lang/rust/issues/60571? They are no longer relevant.

Mark-Simulacrum commented 1 year ago

Ah, I missed that we're emitting a "spurious" feature gate error even though the syntax itself is denied in a separate error in that case. Seems OK.

camsteffen commented 1 year ago

Edit: Oh man I feel silly. Had an extra if.

I just tried writing if <BOOL> && if let ... and it fails pretty terribly. Shouldn't this be supported? If not, there should at least be better diagnostics.

fn foo(b: bool, x: Option<u32>) {
    if b && if let Some(x) = x {}
}
error: expected `{`, found `}`
 --> src/lib.rs:3:1
  |
3 | }
  | ^ expected `{`
  |
note: the `if` expression is missing a block after this condition
 --> src/lib.rs:2:8
  |
2 |     if b && if let Some(x) = x {}
  |        ^^^^^^^^^^^^^^^^^^^^^^^^^^
tronta commented 1 year ago

maybe you should try

fn foo(b: bool, x: Option<u32>) {
    if b && let Some(x) = x {}
}
joshtriplett commented 1 year ago

@camsteffen I think it'd be worth improving the diagnostics for that case; perhaps we could detect it and give a rustfix-able suggestion for "delete the if"?

camsteffen commented 1 year ago

Yes I was thinking that. Seems tricky though since the parser will be in a nested if expression.

alercah commented 1 year ago

This isn't specific to if let: if true && if true {false} gives the same error.

It would probably have been nice to disallow this long ago by requiring parens for any nested block expression, as is done for struct expressions, but that ship is long sailed. Definitely should be considered for improved diagnostics and maybe a lint, but it shouldn't affect this RFC.

lunabunn commented 1 year ago

What is the status on this? Is it ready for a FCP?

rami3l commented 1 year ago

@lunabunn According to https://github.com/rust-lang/rust-clippy/issues/9354, the feature was once stabilized, but reverted due to the issue https://github.com/rust-lang/rust/issues/100513. However the latter has been closed, so I guess we should be back on the right track now.

Lee-Janggun commented 1 year ago

There were a few ICEs related to this feature, including the following I could find.

After the remaining open issues are closed, another FCP may be possible (hopefully ready for 1.65).

est31 commented 1 year ago

Now with #103034 merged, only #99938 is open of your list @Lee-Janggun , which should be fairly easy to close.

I've also filed:

It should ideally be resolved before stabilization. Also, while #100513 is closed, it should probably be reopened because the drop order isn't perfect yet (and there is no lang team approval what a perfect drop order even is).

solomatov commented 1 year ago

What are the plans to stabilize it. Is there any chance it gets into 1.66?

jhpratt commented 1 year ago

Considering 1.66 beta branched on Friday, there is zero chance it'll land then.

est31 commented 1 year ago

As @jhpratt pointed out, it won't stabilize for 1.66 now.

The implementation state has recently been greatly improved by #103034 and #102998. There has also been lang team feedback on the open questions about the drop order.

There is one weird quirk remaining, which is around bool expressions. My PR #103293 tries to address it. The quirk also occurs in non-let if chains, it would still be great to have it fixed before let chains stabilize as it makes the bool conditionals interact with the let conditionals, which they otherwise don't do: the bool conditionals drop in a totally different pattern from the let conditionals, and at a totally different time. This separation makes sense as bool conditionals don't leave behind any bindings while let conditionals do. But with the && twist present, adding a let _ = () && conditional in front of a bunch of bool conditionals changes the drop order within the group of bool conditionals. That seems worse to me than what non-let if chains are doing.

There is still one big-ish bug in #103476. We might settle on it being an acceptable quirk or even, like #103784, expected behaviour, but ideally it would be investigated further before let chains stabilize.

Outside of that I don't know of any blockers.

est31 commented 1 year ago

As a small new years update on the let-chains feature. Some new developments:

real-felix commented 1 year ago

I am not sure it has been discussed before, and I know it's a syntax which shouldn't be used, but isn't that a bit confusing?

let condition = false;
let some_other_condition = false;

if let false = condition && some_other_condition {
    println!("step 1"); // Does not enter here
}

if let false = (condition && some_other_condition) {
    println!("step 2"); // Enters here
}

The first notation is ambiguous, and maybe not everyone expect the precedence to go that way. At least, there should be some kind of warning, like “You better write it that way: if !condition && some_other_condition { … } if that's what you meant”. That warning may even be shown whenever one pattern matches a bool, regardless of the context.

kamulos commented 1 year ago

Wouldn't that be the same as the following:

    let condition = false;
    let some_other_condition = false;

    let a = condition && some_other_condition == false;
    let b = (condition && some_other_condition) == false;

    println!("{a}, {b}");
jtran commented 1 year ago

Interesting. The above comments are saying that the = operator in an if let pat = expr can be thought of as similar to == in terms of precedence level, not like = in assignment statements. That is surprising to me.

I suspect that it was designed this way since it's very common to write code that looks like:

if let Some(x) = foo() && bar(x) {

If the precedence were the other way, the above code would require parentheses.

The relevant section of the RFC is here. It also came up in this thread https://github.com/rust-lang/rust/issues/53667#issuecomment-860336898.