-
Notifications
You must be signed in to change notification settings - Fork 572
specify if let guards with updated scoping rules
#1957
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
base: master
Are you sure you want to change the base?
Conversation
7eb684c to
9add291
Compare
| or a `match` guard. | ||
| * The body expression for a match arm. | ||
| * The non-pattern matching condition expression of an `if` or `while` expression or a non-pattern-matching `match` guard condition operand. | ||
| * The pattern-matching guard, if present, and body expression for a `match` arm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To address #1823 (comment) (cc @theemathas), the temporary lifetime of an if let guard scrutinee is always extended, like for if let expressions. There aren't lifetime extension rules like there are for let statements. This rule should hopefully cover that (similar to how the analogous rule for "The pattern-matching condition(s) and consequent body of if" does below). There's an example further down too showing that the scrutinee lives until the end of the arm. Do you think this is sufficient, or would this benefit from additional clarification?
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, are you saying that both the guard and the body expression together count as a single temporary scope (and not two)? I find that unintuitive, but I suppose that works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a single temporary scope for the arm, which encompasses the guard and body expression, so that if let scrutinees' temporaries live through the body, yes; if let scrutinees aren't temporary destruction scopes by default. Maybe it would be clearer to phrase the temporary scope as being as for the whole arm, with a clarifying note that it includes the guard. Something like "* The arm of a match expression. This includes the arm's guard, if present."
Personally, I'd like stricter scoping rules for if let, but this is consistent with if let/while let expressions. Making their scrutinees be temporary drop scopes by default with lifetime extension rules (like let statements have) would be a larger change requiring an edition break.
|
Is this waiting on anything in particular? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
It looks like my removal of the "Match Guards and Pattern Binding" section was undone? I've re-removed it. I wanted to leave commits before my own mostly untouched, but I can clean up the commit history to remove anything that I'm not preserving in this PR if that'd help keep what's actually meant to be here clearer. |
…ved ingore from some blocks
This applies some editorial rework, mainly to match the style of `if` conditions.
- Removes specification of `if let` guards' bindings "only" being valid if the guard succeeds. The arm body is only executed if the guard succeeds, so the bindings are always usable within the arm body when it's executed. - Makes terminology slightly more consistent (replaces some remaining uses of "`if let` guard" with "`let` pattern") - Other small wording and punctuation tweaks.
b090107 to
ab50634
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
Sorry about that @dianne! I had double-checked the reflog when I was fixing that, and I'm not sure how it got missed. |
traviscross
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
In our PEG-style grammar, the order matters, and in this case, we need `MatchGuardChain` to come after `Expression`. Thanks-to: Eric Huss <eric@huss.org>
c56a2ee to
20e729c
Compare
This is based on #1823 by @Kivooeo and @ehuss, rebased to resolve conflicts. Per rust-lang/rust#141295 (comment), I'm opening this as a separate PR to contribute an updated specification of
if letguards' drop-scoping rules. The new spec is based on the compiler's implementation1, accounting for the changes in rust-lang/rust#143376 and additional corner-cases in examples. I've also done some minor edits to the sections on lexical scope and match expressions, but for the most part they're the same as in #1823.Tracking issue: rust-lang/rust#51114
Stabilization: rust-lang/rust#141295
Footnotes
As with the rules for other syntactic constructs (e.g.
letstatements andifexpressions), this is slightly simplified. Due to implementation details, the compiler's notion of drop scopes is more fine-grained than is necessary for the language spec. ↩