Skip to content

Stabilize if let guards (feature(if_let_guard)) #141295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Kivooeo
Copy link
Contributor

@Kivooeo Kivooeo commented May 20, 2025

Summary

This proposes the stabilization of if let guards (tracking issue: #51114, RFC: rust-lang/rfcs#2294). This feature allows if let expressions to be used directly within match arm guards, enabling conditional pattern matching within guard clauses.

What is being stabilized

The ability to use if let expressions within match arm guards.

Example:

enum Command {
    Run(String),
    Stop,
    Pause,
}

fn process_command(cmd: Command, state: &mut String) {
    match cmd {
        Command::Run(name) if let Some(first_char) = name.chars().next() && first_char.is_ascii_alphabetic() => {
            // Both `name` and `first_char` are available here
            println!("Running command: {} (starts with '{}')", name, first_char);
            state.push_str(&format!("Running {}", name));
        }
        Command::Run(name) => {
            println!("Cannot run command '{}'. Invalid name.", name);
        }
        Command::Stop if state.contains("running") => {
            println!("Stopping current process.");
            state.clear();
        }
        _ => {
            println!("Unhandled command or state.");
        }
    }
}

Motivation

The primary motivation for if let guards is to reduce nesting and improve readability when conditional logic depends on pattern matching. Without this feature, such logic requires nested if let statements within match arms:

// Without if let guards
match value {
    Some(x) => {
        if let Ok(y) = compute(x) {
            // Both `x` and `y` are available here
            println!("{}, {}", x, y);
        }
    }
    _ => {}
}

// With if let guards
match value {
    Some(x) if let Ok(y) = compute(x) => {
        // Both `x` and `y` are available here
        println!("{}, {}", x, y);
    }
    _ => {}
}

Implementation and Testing

The feature has been implemented and tested comprehensively across different scenarios:

Core Functionality Tests

Scoping and variable binding:

  • scope.rs - Verifies that bindings created in if let guards are properly scoped and available in match arms
  • shadowing.rs - Tests that variable shadowing works correctly within guards
  • scoping-consistency.rs - Ensures temporaries in guards remain valid for the duration of their match arms

Type system integration:

  • type-inference.rs - Confirms type inference works correctly in if let guards
  • typeck.rs - Verifies type mismatches are caught appropriately

Pattern matching semantics:

Error Handling and Diagnostics

  • warns.rs - Tests warnings for irrefutable patterns and unreachable code in guards
  • parens.rs - Ensures parentheses around let expressions are properly rejected
  • macro-expanded.rs - Verifies macro expansions that produce invalid constructs are caught
  • guard-mutability-2.rs - Tests mutability and ownership violations in guards
  • ast-validate-guards.rs - Validates AST-level syntax restrictions

Drop Order and Temporaries

Key insight: Unlike let_chains in regular if expressions, if let guards do not have drop order inconsistencies because:

  1. Match guards are clearly scoped to their arms
  2. There is no "else block" equivalent that could cause temporal confusion

Edition Compatibility

This feature stabilizes on all editions, unlike let_chains which was limited to edition 2024. This is safe because:

  1. if let guards don't suffer from the drop order issues that affected let_chains in regular if expressions
  2. The scoping is unambiguous - guards are clearly tied to their match arms
  3. Extensive testing confirms identical behavior across all editions

Interactions with Future Features

The lang team has reviewed potential interactions with planned "guard patterns" and determined that stabilizing if let guards now does not create obstacles for future work. The scoping and evaluation semantics established here align with what guard patterns will need.

Unresolved Issues

All blocking issues have been resolved:


Related:

@rustbot
Copy link
Collaborator

rustbot commented May 20, 2025

r? @SparrowLii

rustbot has assigned @SparrowLii.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 20, 2025

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

@Kivooeo Kivooeo force-pushed the if-let-guard-stable branch from 6fe74d9 to 5ee8970 Compare May 20, 2025 17:08
@rustbot
Copy link
Collaborator

rustbot commented May 20, 2025

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@Kivooeo Kivooeo force-pushed the if-let-guard-stable branch from eb0e4b4 to 0358002 Compare May 20, 2025 17:13
@rust-log-analyzer

This comment has been minimized.

@Kivooeo Kivooeo force-pushed the if-let-guard-stable branch from 92a5204 to ab138ce Compare May 20, 2025 17:35
@rust-log-analyzer

This comment has been minimized.

@Kivooeo Kivooeo force-pushed the if-let-guard-stable branch from 5ceca48 to a20c4f6 Compare May 20, 2025 17:57
@rust-log-analyzer

This comment has been minimized.

@Kivooeo Kivooeo force-pushed the if-let-guard-stable branch 2 times, most recently from 1dd9974 to 5796073 Compare May 20, 2025 18:56
@traviscross traviscross added T-lang Relevant to the language team needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-documentation Status: Waiting on approved PRs to documentation before merging and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 20, 2025
@traviscross
Copy link
Contributor

@traviscross
Copy link
Contributor

@SparrowLii
Copy link
Member

SparrowLii commented May 21, 2025

This needs a fcp so I'd like to roll this to someone more familiar with this feature
r? compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 21, 2025
@rustbot rustbot assigned oli-obk and unassigned SparrowLii May 21, 2025
@traviscross traviscross added I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels May 21, 2025
@oli-obk
Copy link
Contributor

oli-obk commented May 21, 2025

r? @est31

@traviscross
Copy link
Contributor

traviscross commented Jun 13, 2025

Great catch. That does violate my expectations. Thanks @dianne for bringing that to our attention.

@rfcbot concern drop-order-bug-found-by-dianne

Here are some tests about this:

Playground link

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Jun 13, 2025
@traviscross traviscross removed the finished-final-comment-period The final comment period is finished for this PR / Issue. label Jun 13, 2025
@traviscross
Copy link
Contributor

@rustbot author

@rustbot rustbot removed the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 13, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 13, 2025
@traviscross traviscross added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Jun 13, 2025
@traviscross
Copy link
Contributor

traviscross commented Jun 13, 2025

Given the near miss -- despite the care we took to avoid it -- it now feels like, for this and these sort of things, maybe we should expect proof that the testing is exhaustive against the code that's expected to be equivalent, e.g. by some automated means to generate tests for all possible cases.

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jun 13, 2025

@dianne, this is a valuable catch, there is nothing to sorry about, it's better to find this before we stabilize the feature, rather than after

about your test, the reason i was confident about this part because we had this test compare-if-let-guard which shows that there is not differences between if let inside match's arm and if let guard, but, it doesnt show exactly the problem with dropping match bindings (which i was not knowing about)

i analyzed the MIR and can confirm the problem

    match Some(1) {
        Some(_x) if let Some(_y) = Some(2) => {},
        _ => {}
    }

for some reasons compiler creates guard bindings before pattern bindings

    bb2: {
        _5 = copy ((_6 as Some).0: i32); // _6 = Option::<i32>::Some(const 2_i32);
        _3 = copy ((_1 as Some).0: i32); // _1 = Option::<i32>::Some(const 1_i32);
        goto -> bb3;
    }

this means pattern bindings would be dropped first (reverse order), causing the dropck issues you identified

@dianne
Copy link
Contributor

dianne commented Jun 13, 2025

I believe creating guard bindings before pattern bindings is intentional: it's needed for https://doc.rust-lang.org/reference/expressions/match-expr.html#r-expr.match.guard.value. I think the drops will need manual rescheduling to untwist them1; some amount of manual rescheduling is already done for or-patterns, so there's precedent. Ideally I'd like to implement this by specifying the drop order explicitly, but that's soft-blocked on #1421632 (and it might just be easier anyway to use the drop order we get from lowering the guard, but shuffled to happen after the arm's bindings are dropped).

Footnotes

  1. Unless we delay doing by-value bindings for the guard until after by-value bindings for the pattern, but that would require some refactors and also would restrict moving out of a let guard's bindings later in the guard. I don't think that'd be worth avoiding rescheduling drops.

  2. let guards with or-patterns in their pattern have a strange drop order that's currently hard to specify, which makes it harder to schedule drops for the arm body all at once.

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jun 13, 2025

I'm not sure if the team has already seen #142163, @traviscross, but I wanted to bring it to your attention. It's a fairly significant issue, and if it wasn't brought up at the last meeting a couple of days ago, I think it would be worth putting on the agenda soon (though I'm not entirely familiar with how these meetings are planned)

As for the issue itself, I currently see two possible approaches:

  1. Document the current behavior and treat it as non-blocking — as far as I understand, this behavior isn’t exclusive to if let guards but is part of a more general issue. Also, from what I can tell, it's not something that can be used to break soundness — for example, the borrow checker already prevents misuses in the if let context. If we fix this issue after stabilization, as I understand it, this probably won’t break backwards compatibility (please correct me or share your opinion on this point, @dianne)

  2. Wait until the issue is resolved, and proceed with stabilization only after that

I’m not saying this isn’t a real issue — it definitely is — but it seems to be more of an external problem rather than one specific to this feature. I'm not sure what the best path forward for this stabilization PR is, so I just wanted to share the two directions which are the most obvious to me

@dianne
Copy link
Contributor

dianne commented Jun 13, 2025

as far as I understand, this behavior isn’t exclusive to if let guards but is part of a more general issue.

That's also my understanding. The only thing that makes if let guards special is that their scrutinee/bindings are created/declared in the match arm's scope. Thus, explicitly specifying the drop order for a match arm as a whole requires explicitly specifying that drop order (which makes it less convenient). That's not the only way to resolve the issue with let guards, though, so I don't think it would need to block let guard stabilization. I think the easiest solution to let guard drop order currently would be to use the drop order we cget from lowering guards, but move them to happen before the arm's pattern's drops, keeping the ordering the same as it is now besides that. This wouldn't require solving the or-patterns issue.

There might also be reason to specify and stabilize let guards, as-is, with guards' bindings and scrutinees being dropped after the arm's pattern's. I've been trying to think in terms of what's needed for guard patterns to find reasons to keep/untwist the drop order, but so far I haven't come up with anything in those terms beyond "implementation convenience". Even for hypothetical if let guard patterns, all I've found so far is that they're more complicated than I previously had thought ^^;

If we fix this issue after stabilization, as I understand it, this probably won’t break backwards compatibility

Fixing #142163 is a breaking change regardless, but I'd consider it mostly orthogonal to let guards. let guards do increase its surface area in a way, by adding more ways to write patterns, but in many cases they'd be written as patterns regardless of whether the language has let guards. I have been in a situation where let guards would have let me write things as patterns that I otherwise wouldn't have been able to, but I also didn't depend on drop order or or-patterns. I'm not sure it's possible to get a perfect picture of how the impact of fixing #142163 changes based on stabilizing let guards, but I think it's minimal. I just happened to find a lot of drop order inconsistencies in a short timeframe because they affect the implementation of guard patterns.

@traviscross
Copy link
Contributor

I'm curious to hear from @est31 here as well. Even if it's part of a more general issue, it'd be better if possible if we could do something here to avoid increasing the surface area of it, and I'm curious to hear from everyone what the options might be there.

@traviscross
Copy link
Contributor

Separately, I'm curious to hear if there might be some structured way that we could test this to assure ourselves that we really have covered all of the possible drop order cases before the next attempt at stabilization.

@bors
Copy link
Collaborator

bors commented Jun 17, 2025

☔ The latest upstream changes (presumably #137944) made this pull request unmergeable. Please resolve the merge conflicts.

@est31
Copy link
Member

est31 commented Jun 17, 2025

Thanks @dianne for the great catch. I'd say it's definitely not the behaviour I expected, and we should change it, and that's a blocker for stabilization. I wouldn't put all of #142163 into the path though, that should be its own issue.

Drop order has a bunch of weird edge cases, like the thing where panicking in an array or vec constructor reverses the drop order of the array's items, and it's guaranteed to be stable via RFC 1857. Maybe we can change some weirdnesses pointed out in #142163 as well. The let chains stabilization has caused general weirdness fixes #103293 and #103108, but those were in the blocker path for stabilization.

In #140981, I have tried to use the edge cases known from if let chains to test if let guards. Those work indeed really well, but I suppose with all that attention on chains I missed the more basic, non-chain drop order issue found by @dianne. I guess it slipped through because if let chains don't have that extra scrutinee.

I'm curious to hear if there might be some structured way that we could test this to assure ourselves that we really have covered all of the possible drop order cases

The surface area is huge. There have been attempts at doing exhaustive drop order checks, like #100526, #99291, and most recently #133605. RFC 1857's title is very generic but it only considers a small slice of the surface area as well (structs, vecs, etc). One issue of exhaustive checks is that they only consider the current language, and not all of its possible extensions.

That doesn't mean we can't try to make a test for if let chains as exhaustive as possible. We just need to go through all the places that potentially create temporaries, and add ManuallyDrop(_, n) around them.

The relevant piece here wasn't that we didn't have a drop order test with a scrutinee and a guard, it was that the existing tests didn't make the scrutinee temporary ManuallyDrop.

There is still unexplored territory though, i.e. do we drop before or after code execution reaches the next arm's if let (we probably drop after, and we should probably drop before).

@dianne
Copy link
Contributor

dianne commented Jun 18, 2025

There is still unexplored territory though, i.e. do we drop before or after code execution reaches the next arm's if let (we probably drop after, and we should probably drop before).

Thankfully, let guard temporaries are already dropped before the next arm, though we're likely missing a test for it. As I understand it, their temporary scope (and the scope of their bindings) is the scope of the arm they're on, so they're dropped when leaving the arm, whether that's through getting to the end of the arm's body, from a guard failing, or from breaking out early. This is also consistent between match arms and or-patterns: conceptually, failing a guard leaves the arm's scope, then trying another or-pattern alternative re-enters it1. Here's an example (playground):

assert_drop_order(1..=6, |o| {
    let mut x = 1;
    // The match's scrutinee is kept until the end of the match.
    match o.log(6) {
        // Failing a guard breaks out of the arm's scope, dropping
        // the `let` guard's scrutinee. To test the guard for the
        // next or-pattern alternative, we re-enter the arm's scope.
        _ | _ | _ if let _ = o.log(x)
            && { x += 1; false } => unreachable!(),
        // If the guard succeeds, we stay in the arm's scope so the
        // `let` guard's scrutinee is kept around. As before, we
        // drop it when leaving the arm's scope.
        _ if let _ = o.log(5)
            && true => { o.log(4); }
        _ => unreachable!(),
    }
});

Footnotes

  1. The way it's handled when lowering to MIR is a bit messier. When a guard fails, we perform drops as though leaving the arm's scope (by way of in_if_then_scope breaking to the match's scope), but we don't fully leave the arm's scope until we finish handling all of its or-pattern alternatives, in order to share the arm body between them. Instead, we manually manage the drops scheduled in the arm's scope.

@est31
Copy link
Member

est31 commented Jun 18, 2025

Thankfully, let guard temporaries are already dropped before the next arm

fwiw, I didn't mean relative drop order of temporaries created by two arms, but drop order of temporaries of the first arm relative to the execution of the next arm. An example shows this (playground):

fn main() {
    assert_drop_order(1..=6, |o| {
        // The match's scrutinee is kept until the end of the match.
        match o.log(6) {
            _ if let _ = o.log(1)
                && { false } => unreachable!(),
            _ if let _ = o.log(3)
                && { o.push(2); false } => unreachable!(),
            _ if let _ = o.log(5)
                && true => { o.log(4); }
            _ => unreachable!(),
        }
    });
}
@est31
Copy link
Member

est31 commented Jun 18, 2025

Also a bit strange that in these instances, the drop of the scrutinee is after the guard temporaries, while in other examples you put above it is before the drop of the guard temporaries.... not sure what is goin on here.

@dianne
Copy link
Contributor

dianne commented Jun 18, 2025

I think that's because in these last two examples, the scrutinee is left alone, but in my earlier examples, I moved from the scrutinee into pattern bindings. The drop for the scrutinee is scheduled first in the match's scope, so it happens last (after guard temporaries), but drops for by-value pattern bindings are scheduled last in the arm's scope, so they happen first (before guard temporaries, which are also in the arm's scope).

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jul 2, 2025

Hi all,

Unfortunately I’ll be away for at least a year due to military service, so I won’t be able to keep working on stabilizing this feature in the near future.

The implementation itself is solid, and there aren’t really open questions about how it should work. However, there are still some important things left to do before stabilization:

  • We definitely need tests for the drop order cases @dianne pointed out above.

  • The bug with drop order where guard bindings are created before pattern bindings. As @dianne explained, that’s intentional for guard value evaluation, but it leads to the problematic drop order where pattern bindings are dropped first

  • Documentation. But I believe it’s best to write it only after fixing the drop order bug. Otherwise we risk documenting something we intend to change.

  • Implementation-wise, there’s no blocker, the problem is mainly the drop order correctness and documenting it.

We don’t have a specific PR yet for fixing the drop order for if let guards, but @dianne has expressed possible interest in implementing that fix in the future.

Basically everything else is ready. Once the drop order fix lands, the remaining work is mostly writing exhaustive tests and documentation. And re-run FCP process AFAIU.

@est31 – would you possibly be interested in taking this over, or picking it up later? No worries at all if not — in the worst case, I’ll be able (hopefully, in my place it's pretty dangerous I would say) to continue this when I’m back. If there are any further changes or adjustments needed, please feel free to push directly to this branch. I’m absolutely fine with maintainers or reviewers taking over and finalizing any remaining details.

Thanks so much to everyone who has helped so far!

@est31
Copy link
Member

est31 commented Jul 2, 2025

@Kivooeo thanks for your help and pushing this forward. I hope to see you back in the community once you are back out of the military service. Stay safe.

@Kivooeo
Copy link
Contributor Author

Kivooeo commented Jul 2, 2025

(little off-topic but i have to say it) Thank you for the kind words! I also hope I’ll be able to return to working on Rust project. I still have some ambitious plans for reorganizing the tests that I wanted to finish this summer, but unfortunately I’ll have to postpone them until next year.

Regarding my previous message: I think it can serve as a good new starting point and help anyone interested in stabilizing this feature understand where things stand right now. I tried to summarize everything that’s currently relevant and that might help going forward.

Thanks again to everyone — I’ll definitely be back to work on Rust!

rust-bors bot added a commit that referenced this pull request Jul 3, 2025
add a scope for `if let` guard temporaries and bindings

This fixes my concern with `if let` guard drop order, namely that the guard's bindings and temporaries were being dropped after their arm's pattern's bindings, instead of before (#141295 (comment)). The guard's bindings and temporaries now live in a new scope, which extends until (but not past) the end of the arm, guaranteeing they're dropped before the arm's pattern's bindings. So far, this is the only way I've thought of to achieve this without explicitly rescheduling guards' drops to move them after the arm's pattern's.

I'm not sure this should be merged as-is. It's a little hacky and it introduces a new scope for *all* match arms rather than just those with `if let` guards. However, since I'm looking for feedback on the approach, I figured this is a relatively simple way to present it. As I mention in a FIXME comment, something like this will be needed for guard patterns (#129967) too[^1], so I think the final version should maybe only add these scopes as needed. That'll be better for perf too.

Tracking issue for `if_let_guard`: #51114

Tests are adapted from examples by `@traviscross,` `@est31,` and myself on #141295. cc, as I'd like your input on this.

I'm not entirely sure who to request for scoping changes, but let's start with r? `@Nadrieril` since this relates to guard patterns, we talked about it recently, and rustbot's going to ping you anyway. Feel free to reassign!

[^1]: e.g., new scopes are needed to keep failed guards inside `let` chain patterns from dropping existing bindings/temporaries; something like this could give a way of doing that without needing to reschedule drops. Unfortunately it may not help keep failed guards in `let` statement patterns from dropping the `let` statement's initializer, so it isn't a complete solution. I'm still not sure how to do that without rescheduling drops, changing how `let` statements' scopes work, or restricting the functionality of guard patterns in `let` statements (including `let`-`else`).
@Kivooeo Kivooeo force-pushed the if-let-guard-stable branch from 87bedfc to 59125c0 Compare July 3, 2025 20:06
@rustbot
Copy link
Collaborator

rustbot commented Jul 3, 2025

There are changes to the tidy tool.

cc @jieyouxu

@Kivooeo Kivooeo force-pushed the if-let-guard-stable branch from 59125c0 to e7dd467 Compare July 3, 2025 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-meta Area: Issues & PRs about the rust-lang/rust repository itself A-rustc-dev-guide Area: rustc-dev-guide A-testsuite Area: The testsuite used to check the correctness of rustc A-tidy Area: The tidy tool disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. S-blocked Status: Blocked on something else such as an RFC or other implementation work. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. S-waiting-on-documentation Status: Waiting on approved PRs to documentation before merging S-waiting-on-team Status: Awaiting decision from the relevant subteam (see the T-<team> label). T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-lang Relevant to the language team to-announce Announce this issue on triage meeting WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)