regression: let-else syntax restriction (right curly brace not allowed)

[Mark-Simulacrum]

https://crater-reports.s3.amazonaws.com/beta-1.77-3/beta-2024-02-18/gh/Rayzeq.UntimedExplosion/log.txt

I suspect this was an intentional change but we should track down the PR and make sure it's mentioned in relnotes.

[INFO] [stdout] error: right curly brace `}` before `else` in a `let...else` statement not allowed
[INFO] [stdout]    --> src/game.rs:349:13
[INFO] [stdout]     |
[INFO] [stdout] 349 |             } else { break; };
[INFO] [stdout]     |             ^
[INFO] [stdout]     |
[INFO] [stdout] help: use parentheses instead of braces for this macro
[INFO] [stdout]     |
[INFO] [stdout] 341 ~             let Some(msg) = select! ( else { break; };
[INFO] [stdout] 342 ~             let Some(msg) = select! ) else { break; };
[INFO] [stdout]     |
[INFO] [stdout] 
[INFO] [stdout] 
[INFO] [stdout] error: right curly brace `}` before `else` in a `let...else` statement not allowed
[INFO] [stdout]    --> src/lobby.rs:282:13
[INFO] [stdout]     |
[INFO] [stdout] 282 |             } else { break; };
[INFO] [stdout]     |             ^
[INFO] [stdout]     |
[INFO] [stdout] help: use parentheses instead of braces for this macro
[INFO] [stdout]     |
[INFO] [stdout] 274 ~             let Some(msg) = select! ( else { break; };
[INFO] [stdout] 275 ~             let Some(msg) = select! ) else { break; };
[INFO] [stdout]     |

[ehuss]

I'm pretty sure this is from #119062.

[Mark-Simulacrum]

Thanks! It sounds like the FCP there was made with "no breakage found" (#119062 (comment)), so nominating for lang to see if we want to revisit the decision. (My sense is no, given the single GitHub repo that's affected based on beta crater).

[Mark-Simulacrum]

Dropping compiler while lang decides what to do.

[compiler-errors]

Looks like it was committed in the last month. Pretty hilarious and unfortunate timing, but I think that we should accept this breakage given it's a single crate -- however, let's double check with T-lang.

[dtolnay]

It was committed on December 18 (Rayzeq/UntimedExplosion@feed6fa), 25 hours after my analysis of crates.io in #118880 (comment). (The crate is not published to crates.io so I wouldn't have seen it anyway.)

Another unfortunate coincidence: the function it's in has the following comment, so the author is already having quite an experience with toolchains:

// WARNING: EventStream is broken with rust 1.74.X, stay on 1.73.X until this is fixed

https://github.com/Rayzeq/UntimedExplosion/blob/0296c09abc51d105080ecd6b817123d9522fb372/src/lobby.rs#L212

[joshtriplett]

Starting an FCP to confirm we want to go ahead with this.

I sent the author of that repo a PR to fix it. :)

@rfcbot merge

[rfcbot]

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

• @joshtriplett
• @nikomatsakis
• @pnkfelix
• @scottmcm
• @tmandry

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!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

[traviscross]

@rustbot labels -I-lang-nominated

We discussed this in the meeting today. The mood was distinctly in favor of going ahead with this intended breakage, and as above, we're confirming that consensus via FCP. We'll unnnominate.

[joshtriplett]

Nominating for synchronous discussion because we don't seem to have managed to get checkboxes asynchronously. :)

[traviscross]

@rustbot labels -I-lang-nominated

We discussed this in the meeting today 2024-07-17, to make people aware, but with no more discussion of substance.

[tmandry]

My personal opinion is that this restriction should eventually be relaxed or removed. There's no reason you shouldn't be able to write, for example,

let Some(foo) = unsafe { (*foo).method() } else {
    panic!()
};

and the inconsistency with other forms in the language doesn't make sense to me.

I had no problem with this fix which made the let-else restriction more internally consistent (if I'm remembering correctly) when there was no breakage. The breakage might have made me think twice, but it's too late at this point. Since only one crate was broken and this has since hit stable, I don't think there's anything for us to do now.

[traviscross]

@rfcbot cancel

As @tmandry pointed out, there's actually nothing to do here. Time has finalized the consensus of the earlier meeting in a way the FCP did not. We could always relax the restriction in the way proposed, and we'd probably FCP that.