add a scope for if let guard temporaries and bindings
#143376
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
rustbot
added
S-waiting-on-review
|
Some changes occurred in match lowering cc @Nadrieril |
This ensures `if let` guard temporaries and bindings are dropped before the match arm's pattern's bindings.
|
All sounds reasonable to me. Let's see what perf thinks. @bors2 try |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
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`).
rustbot
added
the
S-waiting-on-perf
|
Queued 43cc1e1 with parent a413f77, future comparison URL. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes my concern with
if letguard 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 letguards. 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) too1, 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: #51114Tests 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!
Footnotes
e.g., new scopes are needed to keep failed guards inside
letchain 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 inletstatement patterns from dropping theletstatement's initializer, so it isn't a complete solution. I'm still not sure how to do that without rescheduling drops, changing howletstatements' scopes work, or restricting the functionality of guard patterns inletstatements (includinglet-else). ↩