Consider disallowing let-else expression from ending in `}`, as the RFC said

[dtolnay]

This is a followup to #118880 / #118859.

The compiler's implementation of let-else does not implement restrictions described by the accepted let-else RFC, so some programs that the RFC says are invalid syntax are accepted by stable Rust compilers since 1.65.0.

#118880 (comment) has the relevant examples of such code.

[dtolnay]

Detailed context copied from #118880 (comment):

---

A design aspect of the let-else RFC that was important enough to feature in the 6-sentence summary is that the EXPRESSION in the syntax let PATTERN: TYPE = EXPRESSION else DIVERGING_BLOCK; must not end in }.

This restriction ended up in the RFC based on multiple rounds of feedback.

• The feature originally envisioned by the author did not have this restriction. It only had the essential restriction that EXPRESSION must not be if … {…} (+else if) because in that case the new syntax would collide with stable syntax that already had a different meaning.

• Based on a suggestion in Zulip by bstrie shortly before the publication of the RFC, the syntax was made more restrictive to say EXPRESSION must not be what the Reference calls ExpressionWithBlock. The change was made in rust-lang/rfcs@3438586 and means the expression must not be any of {…}, unsafe {…}, loop {…}, while … {…}, for … in … {…}, if … {…} (+else if +else), or match … {…}.

• The wording to disallow the above expressions was made more precise in response to Niko's non-blocking concern. The wording improvement was made in rust-lang/rfcs@89c5b6e without changing the intent of the restriction.

• In response to steffahn's insightful comment and a related older unresolved review by nagisa and followup, followed by several +1 comments in GitHub and Zulip, and digama0's review, the syntax was made more restrictive again to disallow EXPRESSION from being anything having the last token }. This change was rust-lang/rfcs@c0c2fcc. It disallows async {…}, try {…}, const {…}, Struct {…}, |…| {…}, &{…}, break {…}, return {…} and many others. This restriction was not implemented by the compiler. Later fixed by More expressions correctly are marked to end with curly braces #118880 in Rust 1.76.

• With further iteration from Zulip, the wording was clarified in rust-lang/rfcs@b6b87d6 to emphasize that any expression ending in } before macro expansion is not allowed. mac! {…} is one such example. This restriction was not implemented by the compiler and the cases in More expressions correctly are marked to end with curly braces #118880 (comment) all violate it.

The compiler's implementation of let-else did not implement the restriction described by the accepted RFC, so some programs that the RFC says are invalid syntax are accepted by stable Rust compilers since 1.65.0.

To understand why disallowing only ExpressionWithBlock is not sufficient, see the code samples containing -if in steffahn's comment.

Separately from the ambiguities explained in that comment, steffahn also was I think the first to note a human value in having } else { always mean an else corresponding to an if, as opposed to an else corresponding to a let. This is voiced in steffahn's comment linked above, as well as subsequent discussion on Zulip arriving at agreement about the final strengthenings ("on the grounds that it makes it easy to distinguish let from let-else by looking for an else not preceded by }").

rust-lang/rfcs#3137 (comment) has a summary of the consensus from Zulip and the state of the RFC after the second strengthening of the restriction.

rust-lang/rfcs#3137 (comment) expresses the author's fondness in favor of the second strengthened restriction, "to be clear to human readers", which I also agree with, and is further reinforced by a lang team consensus expressed by Josh in Zulip: "Based on the lang team consensus, the thing we wanted to avoid was let PAT = some_construct_with { braces } else { diverge };"

At the time, only one single exception to the must-not-end-in-} form of the restriction was still being considered, and that was very explicitly the specific case of {…}, i.e. let … = {…} else {…}; with nothing between the = and first {. "So, nothing that ends in } and doesn't start with { unless it's wrapped in parentheses or braces." Ultimately there was not consensus about this exception, there was strong opposition in Zulip and it was never written into the RFC or implemented in the compiler.

The issue of } before else did not get listed as an unresolved question in the final draft of the RFC which got merged. This is because the topic was sufficiently resolved by the discussion described above and the lang team consensus via Zulip, and all parties were in agreement about this iteration of the restriction. So there was no further discussion of it in the tracking issue nor during stabilization aside from one brief comment reinforcing that the restriction is intentional and future possibilities are left open.

@dtolnay: If there's a specific rule that you would propose here for the handling of this kind of thing, please describe that and why it would be better and then renominate this issue for further discussion.

The rule I like is that EXPRESSION must not end in }, which was the RFC final draft.

To the extent that the compiler is going to accept some EXPRESSION which end in } (which is not my preference), I think those cases should be approved by the lang team since so far no exceptions to must-not-end-in-} have been through team approval.

---

[dtolnay]

I am moving the lang-nominated label over from the merged PR #118880 to this issue.

@rustbot label +I-lang-nominated

[dtolnay]

#118880 (comment) has stats on the prevalence of the incorrectly accepted syntax on crates.io. Currently there are 0 occurrences of let … = mac! {…} else {…}, or any other form that would be broken by implementing the } else restriction, such as let … = … + &mac!{…} else {…}.

[Fishrock123]

I think this would be preferable from a user-consistency point of view as I wrote into the RFC, that was the intention and I was not aware it wasn’t implemented as such until recently.

I would like to point out this wording under the discussion of this issue in the RFC

This restriction can be made by checking if the expression ends in } after parsing but before macro expansion.

Specifically that the examples I have seen where this restriction is not upheld is in relation to macros, and the RFC was specific in that the intention was for this check to happen before macro expansion (or in a way which considers a trailing } from a macro pre-expansion, if before is not feasible from an implementation standpoint).

[dtolnay]

Thanks @Fishrock123! My interpretation of that part of the RFC is (these might serve as useful unit tests)—

macro_rules! m {
    () => {
        const { false }
    };
}

fn main() {
    // VALID SYNTAX despite containing `} else` after macro expansion
    let false = m!() else { return; };
}

macro_rules! m {
    () => {
        false
    };
}

fn main() {
    // INVALID SYNTAX due to `} else` before macro expansion
    let false = m! {} else { return; };
}

"Before macro expansion" isn't a completely clear way to describe the restriction because macro expansion isn't one atomic step in compilation. It is interleaved with parsing — syntax trees transit back and force between macro expansion and parsing. The parsing steps must enforce the } else restriction even when taking place after some macro expansion has occurred.

macro_rules! m {
    ($($tt:tt)*) => {
        let false = $($tt)* else { return; };
    };
}

fn main() {
    // INVALID SYNTAX during macro expansion
    m!(const { false });
}

macro_rules! m {
    ($e:expr) => {
        let false = $e else { return; };
    };
}

fn main() {
    // ???
    m!(unsafe { false });
}

[Fishrock123]

@dtolnay Yes you correctly understand how the RFC intended it. The first with m!() else should be valid but m! {} else should be invalid. Completely for legibility to a Rust user and not for any particular implementation purpose.

I suppose it could become confusing when spectating macro expansions for debugging if the expanded output had } else { diverge }. Maybe folks have more opinions on that than myself. I would be amicable towards either argument.

"Before macro expansion" isn't a completely clear way to describe the restriction because macro expansion isn't one atomic step in compilation. It is interleaved with parsing — syntax trees transit back and force between macro expansion and parsing.

Fair enough; a more clear description was not pointed out to me, and sadly I have not had time to learn the internals of rustc.

[scottmcm]

Thank you for the detailed history, @dtolnay ! That's really helpful.

[traviscross]

@rustbot labels -I-lang-nominated

In the T-lang triage meeting today, on the basis of the outstanding history and analysis provided by @dtolnay, the consensus was that this is indeed a bug fix, and one that is safe to do. The result of this is a proposed FCP merge over in PR #119062.

We appreciate @dtolnay for raising this issue and pushing it forward.