Proposal: stabilize if_let_rescope for Edition 2024

Tracked by #124085

Summary

if_let_rescope is an implementation of the #124085 through #107251. This change did not come with a RFC. This proposal will aim to describe the change to the language fully.

if let and the current unstabilized if let-chain has a caveat to the assignment of lifetime rules regarding temporary values generated from initializer expressions.

//@ edition: 2021
if let Some(value) = droppy().get() {
    // do something
} else {
    // `droppy()` is still alive here
    // but it is out of reach as well.
}

Instead, after stabilizing this, we have this effect.

//@ edition: 2024
#![feature(if_let_rescope)]
if let Some(value) = droppy().get() {
    // do something
} else {
    // `droppy()` is already dropped here
}

This will allow us to be more consistent with let $pat = $expr else { $diverge; } where temporaries from $expr are dropped before entering the $diverge branch in today's language.

Given that this is an Edition breaking change, a lint is developed and tested in the field with crater runs.

What is being proposed for stabilization

In #107251, a new kind of scope data IfThenRescope is introduced and gated behind the edition and this feature gate. Upon stabilization, this scope will work like a terminating scope for variables and temporary values generated from the let-bindings.

What is more significant is the case where if let is used as a non-trivial value-producing subexpression in a larger evaluation context, in which it will have semantic impact to lifetimes and object drop orders.

Here are some breaking changes that will be introduced by stabilization.

#![feature(if_let_rescope)]
// borrowck may reject this ...
run(if let Some(ref value) = droppy().borrow() {
    value
} else {
    something_else()
})
// ...

A marginal amount of crates are discovered by previous crater runs in which this change leads to rejection from the borrow checker, because droppy() does not live long enough.

The migration lint that is now implemented on master suggests a rewrite to restore the old semantics perfectly, but not automatically applicable due to the current limitation of machine applicable lints and our internal support for overlapping suggestion spans.

#![feature(if_let_rescope)]
// borrowck will not reject this ...
run(match droppy().borrow() {
    Some(ref value) => { value }
    _ => { something_else() }
})

A more subtle and silent breakage could be involving use of types with significant drop implementation and synchronization primitives such as Mutexes.

if let Some(value) = mutex.lock().unwrap().get() {
    // do something
} else {
    // somehow code expects the lock to be held here ...
    // or because the mutex was only improperly used as a semaphore or for signalling ...
}

Although the crater run did not find breakage in runtime behaviour through cargo test, we proceed with developing lints to detect those cases and rewrite them into matches. Here the suggestion notes are machine applicable because we can coalesce the overlapping spans in one pass.

In the latest crater run we found a marginal population of crates that are truly impacted by this lint, revealing corner cases with the lint which are now properly handled and tested.

Future interactions

Currently there is another possible use of pattern matching in control flows that complements if let chain is the is operator proposed in rust-lang/rfcs#3573. This change both runs along the same principle of no unnecessary binding and long lifetime beyond the applicable scope from a pattern-matching predicate. With this change, the implementation of is would be believably less error-prone due to the unexpectedly long lifetimes.

This leaves match being the only exception to this principle. In fact, our mitigation and migration strategy is based on it. It might remains as an acceptable exception and further be publicized and inadvertently advertised as such.

In general, this change is not expected to influence or hinder major language design.

@rustbot

@rustbot labels +T-lang +A-edition-2024

Note that I'm not very good at analysing crater run results (not just what broke, but also what happened to the crates that don't build for other reasons), but I think it would be important to have a crater analysis that includes the exact number of breakages and crates that didn't build (or otherwise what the analysis did not cover), and if feasible classification of the nature of breakages. Regarding the lint, as mentioned it's not machine-applicable due to overlapping suggestions not possible in the current fix application infra, but also as is usual with proc-macro detection, any crates that forwards user spans with no distinguishing syntax context will cause Span::from_expansion to return false (technically not-our-bug, but still).

@rfcbot

FCP for stabilization:

@rfcbot merge

@joshtriplett

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

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.

@rfcbot

@rfcbot reviewed

@rfcbot

@rfcbot reviewed

Copying my reasoning from the meeting notes for posterity:

This introduces a disparity between match and if-let, which I don't like, but it moves towards earlier drop, which I do like, and it also means that if and if let work consistently:

struct DropOrder;

impl DropOrder {
    fn test(&self) -> bool {
        false
    }
}

impl Drop for DropOrder {
    fn drop(&mut self) {
        eprintln!("drop");
    }
}

fn main() {
    if DropOrder.test() {
    // ^^^^^^^^^ dropped before entering else
        eprintln!("this body");
    } else {
        eprintln!("else body");
    }
}

You can see the behavior here, it prints "drop" first.

In short, this is moving us closer to the world I want, which is one in which match is able to drop more eagerly.

@rfcbot

@rfcbot reviewed

I'm confident in the semantics proposed, given the above reasoning, and that we can make the edition experience good enough for our users, though it might require some dialing in before we ship.

For our reference, here's a worked-through example of the behavior compared with the various refactorings discussed:

Playground link

Probably what is most interesting to me, in that work-through, is comparing the behavior of if-let with let-else. That is, compare...

if let Ok(_v1) = temporary(p).as_ref() {
    a()
} else {
    b()
}

...with:

'top: {
    let Ok(_v1) = temporary(p).as_ref() else {
        break 'top b()
    };
    a()
}

In the former, even with if_let_rescope, we drop the temporary after running a(), while in the latter it's the other way around.

I know Niko has in the past argued for dropping the temporary immediately after considering the scrutinee. The let-else case "speaks to me" in terms of why that might be reasonable to consider (eventually).

What we're doing here, with the if_let_rescope rule, is at least probably a clean a step in that direction.

Rollup merge of rust-lang#131984 - dingxiangfei2009:stabilize-if-let-…

@rustbot

Offline, @traviscross pointed out to me that part of the reasoning here is to unblock let-chains -- i.e., you can view if let Some(x) = y { A } else { B } as if (let Some(x) = y) { A } else { B }. I find that persuasive but all of this makes me think we should revisit this in lang-team triage tomorrow.

@rustbot labels +I-lang-nominated

Unrolled build for rust-lang#131984

still lang-nominated, reopening

I think let chains are motivation enough for moving off the 2021 semantics in 2024. While making the change it would be great to simplify the mental model if we can, but arguably both 2024.1 and 2024.2 simplify the mental model in different ways.

The argument that this can lead to more predictable code is a strong one for me. I would also argue that if-let behaving like if makes for a better mnemonic device than if-let behaving like match, which is a minor point but it is something. A general rule that says let-else behaves like let, if-let behaves like if, and match is its own thing (for now) is simple enough to remember.

On the other side we have the argument that 2024.2 more closely matches what a user means in some cases ("DWIM"), and the fact that 2021.1 is breaking the existing behavior and thus breaking more code.

On the "DWIM" side, looking at the examples of code that would be affected by the 2024.1 change so far doesn't motivate me to change the rules to 2024.2. An if-let nested inside of a function call isn't a case I think we should rotate on very much. Maybe other patterns would make me change my mind but so far I haven't been able to come up with anything very compelling.

On the breaking change front, data would be helpful, but I still feel unable to evaluate. I haven't had time to dig in, but just looking at the crater triage comments I'm having trouble understanding what we learned. It seems like the results are still a bit ambiguous.

So far I prefer 2024.1 unless I see evidence that we're breaking a significant amount of "idiomatic" code in an annoying and unnecessary way.

+1. I'm happy with what's now in Rust 2024, and also with the design axioms Niko outlined.

@rustbot

@rustbot labels -I-lang-nominated

We discussed this in lang triage today. Through that discussion, we confirmed that we're happy with the Rust2024.1 behavior that has now landed in Rust 2024 (nightly).

In doing this, we were motivated by the design axioms Niko noted above. We were motivated by how this change moves toward better alignment with if, and in that way facilitates a more useful meaning for if-let chains.

We discussed how we may, or may not, want to eventually fully align with if and drop the temporaries before the then branch. If we later did want to do that, this would have been a clean step in that direction, so we're not doing any harm here in adopting this.

We acknowledged in our discussion that this is a tough call in many respects.

Do not trigger `if_let_mutex` starting from Edition 2024 (#13695)

Navigation Menu

Repository navigation

Proposal: stabilize `if_let_rescope` for Edition 2024 #131154

Summary

What is being proposed for stabilization

Future interactions

21 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Proposal: stabilize if_let_rescope for Edition 2024 #131154

Description

Summary

What is being proposed for stabilization

Future interactions

Activity

dingxiangfei2009 commented on Oct 2, 2024

jieyouxu commented on Oct 2, 2024

joshtriplett commented on Oct 2, 2024

rfcbot commented on Oct 2, 2024

traviscross commented on Oct 2, 2024

nikomatsakis commented on Oct 2, 2024

tmandry commented on Oct 2, 2024

21 remaining items

traviscross commented on Oct 29, 2024

traviscross commented on Oct 29, 2024

traviscross commented on Oct 29, 2024

lcnr commented on Oct 29, 2024

tmandry commented on Oct 30, 2024

traviscross commented on Oct 30, 2024

traviscross commented on Oct 30, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

Proposal: stabilize `if_let_rescope` for Edition 2024 #131154