rust-lang / rust

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

let-else does not support `else if` #111910

Open lcnr opened 1 year ago

lcnr commented 1 year ago
fn main() {
    let Some(x) = Some(1)
    else if true {
        return;
    } else {
        return;
    }
}

results in the following error but should compile imo:

error: conditional `else if` is not supported for `let...else`
 --> src/main.rs:3:10
  |
3 |     else if true {
  |          ^^ expected `{`
  |
help: try placing this code inside a block
  |
3 ~     else { if true {
4 |         return;
5 |     } else {
6 |         return;
7 ~     } }
  |

the improved diagnostic has been added in #89974 which refers to https://github.com/rust-lang/rust/issues/87335#issuecomment-944888160 by @joshtriplett:

let ... else if definitely isn't something the RFC specifies, and I don't think we should. A better error message might make sense, though ("cannot use else if with let ... else").

I personally disagree with "and I don't think we should". For me this not working was fairly surprising and conflicts with my intuition of let else as a shorthand for

let Some(var) = expr else { return; }

let var = if let Some(var) = expr {
    var
} else {
    return
}

or as stated in the RFC

This is a counterpart to if let expressions, and the pattern matching works identically, except that the value from the pattern match is assigned to the surrounding scope rather than the block's scope.

I would like to add support for this myself or mentor someone adding this. I am not sure what process is required here. If not desired, please FCP close this.

fee1-dead commented 1 year ago

I'd be willing to work on this if this is desired.

est31 commented 1 year ago

FTR this is the real world code that my comment in the let else tracking issue was referring to (the link bitrotted):

https://github.com/rust-lang/rust/blob/72d66064e77281536588189a916af28a1819b313/compiler/rustc_codegen_ssa/src/back/link.rs#L189-L198

Nowadays it's an if let but one could imagine code afterwards that one doesn't want to be affected by rightward drift.

IMO it's a good idea to have it, but back when let else was still unstable it was definitely better to have shipped without it to keep the feature minimal.

It also has low conflict potential with other extension proposals for let else. The worst I could think of is confusion potential with extensions that look (but not are) very close:

let Foo::BarA(foo) = hi
else if let Foo::BarB(foo) { return; } // We could theoretically allow dropping the expr here and just match on the `hi` expr
else if let Foo::BarC(foo, foo2) { return; }
else if true { // Is this tail if confusing? It does *not* try to do something with the `hi` expr
    panic!()
} else {
    return;
};

or the match equivalent:

let Foo::BarA(foo) = hi
else match {
    Foo::BarC(foo) => panic!(),
    Foo::BarD(foo, foo2) => panic!(),
    _ => panic!(),
};
c410-f3r commented 1 year ago

Hum, it is unrelated but I would like to chime-in to also mention the possibility of using let_else with let_chains.

scottmcm commented 1 year ago

(Not speaking for the team)

Personally, I'm not inclined to do this. We generally don't have "remove one level of extra indentation" features, and (more importantly) I think that if we're going to extend this, it would be something that more directly integrates with the pattern behaviour. Like there was the previous discussion of something like let Ok(x) = ... else match { Err(e) => ... };, for example, which is intimately connected with the core let-else feature, and not something that would work already by just putting it inside the else braces.

(I consider normal else if to be linearly less nesting because of chaining, but a let-else-if isn't chainable, so doesn't get that extra mountain-range-avoidance bonus.)

lcnr commented 1 year ago

to me this is not about "remove one level of extra indentation" but much rather that else if and else are part of the same language construct. else without else if fundamentally feels inconsistent to me. Other features, like else match however introduce a new construct, as currently else match is not supported anywhere.

scottmcm commented 1 year ago

I guess I don't see it that way as there's no if in the first place. To me, else if is going with if, not because of the else, I guess.

(For example, for-else in Python doesn't support elif, as far as I know, since elif is part of if, not part of else.)

joshtriplett commented 11 months ago

I'm similarly not inclined to do this; else if doesn't seem like an inherent part of let-else, and it doesn't feel like it adds new functionality (since you must diverge). If this added or unlocked functionality you couldn't get otherwise, that'd make it more appealing. But as it stands, it seems like a minor syntax tweak.

lcnr commented 11 months ago

this is a discussion about syntactic sugar, it can never unlock functionality you couldn't get otherwise. let-else itself is a minor syntax tweak.

Looking at https://github.com/rust-lang/rust/issues/111910#issuecomment-1561890790 as an example where this might be useful:

        let Some((path, _)) = &used_crate_source.rlib
        else if used_crate_source.rmeta.is_some() {
            return Err(format!(
                "could not find rlib for: `{}`, found rmeta (metadata) file",
                name
            ));
        } else {
            return Err(format!("could not find rlib for: `{}`", name));
        };

vs

        let Some((path, _)) = &used_crate_source.rlib else {
            if used_crate_source.rmeta.is_some() {
                return Err(format!(
                    "could not find rlib for: `{}`, found rmeta (metadata) file",
                    name
                ));
            } else {
                return Err(format!("could not find rlib for: `{}`", name));
            }
        };

I think the formatting required for else if is worse than the formatting with else { ... } here. having let PAT = EXPR else if COND { on one line feels fairly hard to read, even if it could fit on one line.

I still think having the option to use else if if the else is forced to be on a separate line anyways is nice, but :shrug: experimenting with the formatting of it does make the argument less convincing.

Going to drop the nomination because this issue doesn't feel important enough to stay nominated for months.