Proposal: stabilize if_let_rescope for Edition 2024

[dingxiangfei2009]

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.

[dingxiangfei2009]

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

[jieyouxu]

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).

[joshtriplett]

FCP for stabilization:

@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
• @traviscross

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]

@rfcbot reviewed

[nikomatsakis]

@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.

[tmandry]

@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.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[joshtriplett]

Also, when we document this, we should point out that while it's possible to have surprises with Mutex<()> getting dropped early, this change avoids surprises with it being dropped late, such as surprising self-deadlocks in the else branch.

[traviscross]

@rustbot labels -I-lang-nominated

We discussed this in the meeting today... and it's now in FCP.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[lcnr]

sorry for the long comment

---

Why does this change need to break the following snippet? https://play.rust-lang.org/?version=nightly&mode=debug&edition=2024&gist=08a43c178a203ce952e9c1aee733646b

fn main() {
    use_value(if let Some(value) = droppy().borrow() {
        value
    } else {
        return;
    })
}

looking at the generated MIR we get the following

fn main() -> () {
    let mut _0: ();
    let mut _1: std::option::Option<&Droppy>;
    let mut _2: &Droppy;
    let _3: Droppy;
    let mut _4: isize;
    scope 1 {
        debug value => _5;
        let _5: &Droppy;
    }

    bb0: {
        _3 = droppy() -> [return: bb1, unwind continue];
    }

    bb1: {
        _2 = &_3;
        _1 = Droppy::borrow(move _2) -> [return: bb2, unwind: bb7];
    }

    bb2: {
        _4 = discriminant(_1);
        switchInt(move _4) -> [1: bb3, 0: bb4, otherwise: bb9];
    }

    bb3: {
        _5 = copy ((_1 as Some).0: &u32);
        drop(_3) -> [return: bb5, unwind continue];
    }

    bb4: {
        drop(_3) -> [return: bb6, unwind continue];
    }

    bb5: {
        _0 = use_value::<&Droppy>(copy _5) -> [return: bb6, unwind continue];
    }

    bb6: {
        return;
    }

    bb7 (cleanup): {
        drop(_3) -> [return: bb8, unwind terminate(cleanup)];
    }

    bb8 (cleanup): {
        resume;
    }

    bb9: {
        unreachable;
    }
}

where in edition 2021 this instead uses the following, only dropping the temporaries from the if let pattern after the call to use_value

    bb2: {
        _4 = discriminant(_1);
        switchInt(move _4) -> [1: bb3, 0: bb4, otherwise: bb9];
    }

    bb3: {
        _5 = copy ((_1 as Some).0: &Droppy);
        _0 = use_value::<&Droppy>(copy _5) -> [return: bb5, unwind: bb7];
    }

    bb4: {
        drop(_3) -> [return: bb6, unwind continue];
    }

    bb5: {
        drop(_3) -> [return: bb6, unwind continue];
    }

What I would have expected is that this feature does not change the drop location of if let pattern at all in the if-branch. Is it not possible to keep the drop locations from the 2021 edition, but adding another drop at the start of the else branch. The drop of the pattern at the end of the statement would then be a noop if the else-branch has been taken due to drop-flags.

[lcnr]

e.g. looking at a minimized version of https://github.com/rust-lang/rust/blob/master/tests/ui/feature-gates/feature-gate-if-let-rescope.rs: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=68208167c29c3eb875339a6e5ed90e15

I would expect the generated MIR in edition 2024 (diff from current edition 2021 output) to be the following:

  fn main() -> () {
      let mut _0: ();                      // return place in scope 0 at src/main.rs:17:10: 17:10
      let mut _1: A;                       // in scope 0 at src/main.rs:18:9: 18:14
      let _2: ();                          // in scope 0 at src/main.rs:19:5: 22:6
      let mut _3: std::option::Option<B<'_, A>>; // in scope 0 at src/main.rs:19:19: 19:24
      let mut _4: &mut A;                  // in scope 0 at src/main.rs:19:19: 19:20
      let mut _5: isize;                   // in scope 0 at src/main.rs:19:12: 19:16
      let _6: std::option::Option<B<'_, A>>; // in scope 0 at src/main.rs:21:9: 21:14
      let mut _7: &mut A;                  // in scope 0 at src/main.rs:21:9: 21:10
      scope 1 {
          debug a => _1;                   // in scope 1 at src/main.rs:18:9: 18:14
          scope 2 {
          }
      }

      bb0: {
          StorageLive(_1);                 // scope 0 at src/main.rs:18:9: 18:14
          _1 = A;                          // scope 0 at src/main.rs:18:17: 18:18
          FakeRead(ForLet(None), _1);      // scope 0 at src/main.rs:18:9: 18:14
          StorageLive(_2);                 // scope 1 at src/main.rs:19:5: 22:6
          StorageLive(_3);                 // scope 2 at src/main.rs:19:19: 19:24
          StorageLive(_4);                 // scope 2 at src/main.rs:19:19: 19:20
          _4 = &mut _1;                    // scope 2 at src/main.rs:19:19: 19:20
          _3 = A::f(move _4) -> [return: bb1, unwind: bb10]; // scope 2 at src/main.rs:19:19: 19:24
                                          // mir::ConstOperand
                                          // + span: src/main.rs:19:21: 19:22
                                          // + const_: Const { ty: for<'a> fn(&'a mut A) -> Option<B<'a, A>> {A::f}, val: Value(A::f) }
      }

      bb1: {
          StorageDead(_4);                 // scope 2 at src/main.rs:19:23: 19:24
          PlaceMention(_3);                // scope 2 at src/main.rs:19:19: 19:24
          _5 = discriminant(_3);           // scope 2 at src/main.rs:19:19: 19:24
          switchInt(move _5) -> [0: bb2, otherwise: bb4]; // scope 2 at src/main.rs:19:12: 19:16
      }

      bb2: {
          falseEdge -> [real: bb3, imaginary: bb4]; // scope 2 at src/main.rs:19:12: 19:16
      }

      bb3: {
          _2 = const ();                   // scope 2 at src/main.rs:19:25: 20:6
          goto -> bb7;                     // scope 1 at src/main.rs:19:5: 22:6
      }
      
      bb4 {
+         drop(_3) -> [return: bb4Cont, unwind: bbWhoCares];
+     }
+
+      bb4Cont: {         
          StorageLive(_6);                 // scope 1 at src/main.rs:21:9: 21:14
          StorageLive(_7);                 // scope 1 at src/main.rs:21:9: 21:10
          _7 = &mut _1;                    // scope 1 at src/main.rs:21:9: 21:10
          _6 = A::f(move _7) -> [return: bb5, unwind: bb9]; // scope 1 at src/main.rs:21:9: 21:14
                                          // mir::ConstOperand
                                          // + span: src/main.rs:21:11: 21:12
                                          // + const_: Const { ty: for<'a> fn(&'a mut A) -> Option<B<'a, A>> {A::f}, val: Value(A::f) }
      }

      bb5: {
          StorageDead(_7);                 // scope 1 at src/main.rs:21:13: 21:14
          drop(_6) -> [return: bb6, unwind: bb9]; // scope 1 at src/main.rs:21:14: 21:15
      }

      bb6: {
          StorageDead(_6);                 // scope 1 at src/main.rs:21:14: 21:15
          _2 = const ();                   // scope 1 at src/main.rs:20:12: 22:6
          goto -> bb7;                     // scope 1 at src/main.rs:19:5: 22:6
      }

      bb7: {
+         // In case we dropped `_3` in `bb4` we'd drop it here again, however, that
+         // should be fine as `Drop` before drop elaboration is conditional on whether
+         // the variable has already been dropped. This would cause `if let` to always
+         // require drop flags tracking which may cause perf issues, but I would like to
+         // at least see some testing for that
          drop(_3) -> [return: bb8, unwind: bb10]; // scope 1 at src/main.rs:22:6: 22:7
      }

      bb8: {
          StorageDead(_3);                 // scope 1 at src/main.rs:22:6: 22:7
          StorageDead(_2);                 // scope 1 at src/main.rs:22:6: 22:7
          _0 = const ();                   // scope 0 at src/main.rs:17:11: 23:2
          StorageDead(_1);                 // scope 0 at src/main.rs:23:1: 23:2
          return;                          // scope 0 at src/main.rs:23:2: 23:2
      }

      bb9 (cleanup): {
          drop(_3) -> [return: bb10, unwind terminate(cleanup)]; // scope 1 at src/main.rs:22:6: 22:7
      }

      bb10 (cleanup): {
          resume;                          // scope 0 at src/main.rs:17:1: 23:2
      }
  }

[dingxiangfei2009]

Thank you for the suggestion @lcnr !

I admit that making the double drop would work. It would perfectly retain the current Edition 2021 if let semantics and I at one point considered this option very appealing, too.

On the other hand, however, the feature gate if_let_rescope is proposed to apply a rather conscious change in design which is to make the temporary lifetime more defined and contained by syntatical constructs. I have to admit that extending the temporary lifetime as in the example provides great convenience. It comes with a cognitive cost for readers to reason about, when exactly a temporary value is terminated, especially if it has an effectful destructor holding a resource, such as MutexGuard. When values withholding resources are generated, if let can be hazardous to work with because the resources will be held back for longer without clear benefits.

compute(if let Some(&value) = mutex.lock().unwrap().get(&key) {
    // here we take a copy and our business with the lock is technically finished
    value
} else {
    // we can drop the lock guard here in the failure path ...
    return;
}); // ... but in the happy path, throughout the span of `compute` invocation, the lock is held for no extra benefit

If indeed a borrow checker error is raised, this at least provides an opportunity to users that a resource is held beyond its use for the decision-making here and it encourages a better, clearer structure of the program as an alternative.

if let Some(value) = mutex.lock().unwrap().get(&key) {
    // value: &V
    compute(value);
} else {
    return;
}

With Edition 2021 and prior, one may attempt to say that to answer this question, it sufficed to "just find the deepest enclosing block and point at its end," or "look out for the semicolon at the end of the statement." In practice, this idiom does not work reliably. One example is when if let is involved in a macro expansion. Since macros can be used at nested expression location, where temporary values involved in if let are terminated is not a property that can be easily determined from local definition or information. On the contrary, the option to promise that the temporary values will be dropped consistently after evaluating the consequent block or when pattern matching fails, is a rule that is local and is easy to apply with a quick glance at code, and is also easy to teach. With this change, there is hope to bring more clarity to how temporary lifetimes are assigned and reduce the surprises.

[nikomatsakis]

We need to reach a decision ASAP (really need to already have reached a decision), so I've been thinking over @lcnr's alternative proposal to drop temporaries at the end of the "then" arm (when the pattern matches) and before the "else" arm (when the pattern does not match). I certainly see the appeal and it can be more ergonomic. However, overall it doesn't seem like the right call to me, as it is not consistent with any existing forms and errs on the side of introducing more runtime bugs. Let me explain.

Background: design axioms

At the end of the day, there is no "correct" set of temporary rules. What seems obvious in one example ("clearly the temporary should live longer...") because rather unobvious in another ("what? the lock is being held for the entire if let block?"). In fact, just last week I had a rather mysterious deadlock that wound up being the result of precisely the scenario that @dingxiangfei2009 describes: I had an if let and I expected it would release the lock once I was done using the result, but in fact it was not dropped until the end of the if let, resulting in a recursive lock failure.

Unfortunately we never got as far as deciding on an official set of design axioms around temporaries -- I still think that's worth doing -- but my personal take is that the axioms should be...

• Syntax driven and predictable: users should not have to know the precise types involved to know when values will be dropped.
• Shorter is safer: picking a shorter temporary lifetime means users get borrow check errors if you are wrong instead of runtime failures.
• Do what I mean: we should try to drop temporaries at "the right time" whenever we can.

Let me walk through these and their rationale and implications. Why predictable? Without this tenet, we could make lifetime rules predicated on lifetime inference or some such, but we decided that was a bad idea early on because (a) the borrow check doesn't have all the info it needs to make an informed decision, such as references from unsafe code; and (b) users should not have to model the borrow check results to know when a destructor will run.

Why is shorter safer? Shorter lifetimes produce borrow check errors, which is annoying, but longer lifetimes produce deadlocks and panics at runtime, which is worse. This is a pretty common source of bugs—take a look at [Understanding Memory and Thread Safety Practices and Issues in Real-World Rust Programs], which found that 30 out of 38 of the deadlocks they found were caused by double locking, with all their examples showing cases of temporary lifetimes. "Rust's complex [temporary] lifetime rules together with its implicit unlock mechanism make it harder for programmers to write blocking-bug-free code." (the word "temporary" is inserted by me, but what other parts of lifetime rules are complicated?)

Finally, yes, we would like to be convenient where possible. In my original 2014 blog post I laid out two patterns

• foo.lock().do_something(), which would drop the lock at the end of the statement;
• let x = Foo { x: &value() }, which will obviously want value to live as long as x.

Both of those patterns are pretty ingrained in Rust code by now.

Analyzing this case in terms of the axioms

The question at hand is, given an if-let like this, which creates a temporary value so that it can call as_ref (which expects an &self)....

if let Some(r) = temporary().as_ref() { A } else { B }

...when should we drop this temporary() relative to executing A and B. Today (Rust2021) we drop it after both. The proposal in the PR, which I'll call it "Rust2024.1", is to drop it before both. The proposed alternative, which I'll call "Rust2024.2", is to drop after A but before B.

So how should we choose between those three options, based on the axioms? It seems clear that "erring on the side of shorter" matches Rust2024.1 best but "DWIM" matches Rust 2024.2 best. (Rust2021 fails on both of those axioms.)

But what about the first axiom, predictability? That's a bit more complicated. In terms of predictability, if-let is in an awkward spot, as it has to contend with two precedents. To see what I mean, consider the following forms that one might reasonably expect to be "equivalent":

if let Some(r) = temporary().as_ref() { A } else { B }
if temporary().as_ref().is_some() { A } else { B } // assume that `r` is dead in the above
{ let Some(r) = temporary else { B }; A }
{ match temporary().as_ref() { Some(r) => A, None => B } }

Here is how drops occur in the three variants:

Variant	if-let	if	let-else	match
Rust2021	after, after	before, before	before, before	after, after
Rust2024.1	before, before	before, before	before, before	after, after
Rust2024.2	after, before	before, before	before, before	after, after

Based on this, I argue that both Rust2021 and Rust2024.1 are more predictable than Rust 2024.2, which introduces a new combo.

Are we really predictable?

TBH, our temporary rules are syntax driven, but I don't thnk they are all that predictable. This change helps, but maybe there is a better solution we can pursue going forward. Also, after change, match becomes even more of an outlier; imo it is prioritizing DWIM over the first two axioms and should change. But the impact was too large in practice and so we decided to stay the course for now. I think that was the right call, it just doesn't apply here because the impact is narrower.

Looking forward, I feel like we need to do something to improve the temporary rules, and I wonder if "DWIM" is just too much in tension with predictability. I kind of like the idea of introducing some kind of explicit way to say "introduce a let binding for this above the current statement", like x.let, such that instead of let x = Foo { x: &vec![] } one might do let x = Foo { x: vec![].let } or something like that, but I'd have to try it for a while to see how it really feels.

[lcnr]

My reasoning for continuing to allow this was two-fold:

• this seems to be the only breakage caused by this change and is separate from the actual desired impact, i.e. if we did not change the eager drop for else, I don't think we'd have moved the drop earlier for the if-branch.
  • imo this change between editions is fairly confusing and feels like a regression to me. t
  • it might be possible to land the 2024.2 behavior without an edition, avoiding a very likely unexpected inconsistency
• the 2024.2 behavior more closely matches match, which matches my thoughts about if let. I think a similar change would be to eagerly drop the scrutinee when taking a wildcard branch. This prioritizes the third axiom even more

This is a pretty common source of bugs—take a look at [Understanding Memory and Thread Safety Practices and Issues in Real-World Rust Programs], which found that 30 out of 38 of the deadlocks they found were caused by double locking, with all their examples showing cases of temporary lifetimes.

This leads me to believe we should have a proper rustc lint for these cases¹ by relying on a #[diagnostics::lint_as_temporary(reason = "...")] attribute. I think the value of "shorter is safer" is singificantly lower if we lint in case the longer-lived temporaries may matter.

The stabilization PR for the 2024.1 approach is ready for merge in #131984. I am still not totally convinced by the current approach. Given that that's a lang question, I won't block that PR though

Footnotes

1. i.e. whenever we would like to shorten the lifetime of temporaries for the sake of the "shorter is safer" axiom, go with the longer lifetime + lint instead ↩

[nikomatsakis]

Hmm, @traviscross pointed out that my comment was mistaken in some of the particulars, which I find mildly disturbing. Side note but the challenge in communicating precisely here goes to show the potential value of a spec, this may be a useful case study.

In any case, I misunderstood the behavior. The actual definition is not what I thought. My example cannot correctly capture the full details. A better example are these four "rough equivalents":

// if-let
test(if let Some(x) = maybe_droppy().as_ref() { A } else { B }, C);

// if (assume the `x` from `Some(x)` above is not used in A or B, since otherwise you'd get errors)
test(if maybe_droppy().is_some() { A } else { B }, C);

// let-else (errors if `x` is live in Rust 2021, actually)
let Some(x) = maybe_droppy().as_ref() else { B };
test(A, C);

// match
test(match maybe_droppy().as_ref() { Some(_) => A, None => B }, C);

The key point here is that we have A and B but then we have other content C that is within the "temporary scope" of the call to test. Now I believe we have the following table:

Variant	if-let	if	let-else	match
Rust2021	after, after, after	before, before, before	before, before, before	after, after, after
Rust2024.1	after, before, before	before, before, before	before, before, before	after, after, after
Rust2024.2	after, before, before or after (*)	before, before, before	before, before, before	after, after, after

(*) it will be dropped before if take the else branch, after otherwise

Reviewing the table like this, I am feeling uncertain (much as I hate to admit it) as to what is best. There was at some point a rule that @m-ou-se, @dingxiangfei2009, and I were considering that had to do with thinking about the answer to the questions:

• is it guaranteed that a reference to the temporary is in scope?
• is it possible for a reference to the temporary to be in scope?
• is it impossible for a reference to the temporary to be in scope?

I feel like we can refine the axioms to take this into account: e.g., if it is impossible (as in the else branch of if let), it feels good to drop the value (and yet we don't do this consistently for e.g. the None arm of a match, which is not great). When it is guaranteed that a temporary is in scope (as in let x = &temp()) it seems obviously good to extend the lifetime (though indeed x may not be actively used).

The trouble lies in the "possible" cases, which often have to do with match arms -- basically we have to know the types involved to know whether if let Some(x) = maybe_droppy().as_ref() has an in-scope reference to the temporary returned by maybe_droppy; indeed, we have to know the types involved to even know if there is a temporary at all. Therefore the first axiom ("syntactically driven") is not entirely achievable, for better or worse.

Question for @traviscross and @dingxiangfei2009 -- how "optional" is this change? I feel very strongly that we need to change the drop rules for tail expressions; this seems like a "nice to have".

[nikomatsakis]

Also, @traviscross can you verify my comment there :)

[nikomatsakis]

So, I was wrong in some of the particulars, but I still do not prefer Rust2024.2, and my reasoning is that it seems like the least predictable option (violates axiom 1).

It seems to me that the real impact of this change is to align the temporary rules for if let with if statements rather than match statements. Whether that's good or not is the question to be decided.

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.

[traviscross]

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

Playground link

[traviscross]

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.

[traviscross]

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

[lcnr]

still lang-nominated, reopening

[tmandry]

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.

[traviscross]

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

[traviscross]

@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.