-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Stabilize let chains in the 2024 edition #132833
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
Conversation
Process-wise, we normally do the FCP on the stabilization PR, and I think that'd make the most sense here. Let us know if you need us on lang to weigh in on anything specifically, e.g. any of the open questions, to make such a stabilization PR possible. cc @rust-lang/lang for visibility. And cc @rust-lang/style given the open item on the style guide. @rustbot labels +I-style-nominated Putting on the edition hat, since this would be an edition item in some sense, we should talk about this in our next edition call. @rustbot labels +I-edition-nominated |
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
@traviscross understood, I've converted it to a pull request using the |
Beautiful, thanks. Let's nominate this for lang discussion too then. @rustbot labels +I-lang-nominated |
This comment has been minimized.
This comment has been minimized.
79e1519
to
ee7488a
Compare
@rustbot labels -I-edition-nominated We talked this one through in the edition call. Since there's no breakage or migration here, we're happy for this one to land in Rust 2024 either ahead of or after the release of Rust 2024 itself as long as there is an edition guide chapter for it. |
aa24aa5
to
d57626e
Compare
c373f03
to
767073c
Compare
This comment has been minimized.
This comment has been minimized.
The edition gate is a bit stricter than the drop behaviour, which is fine. The cases we want to avoid are the opposite: not gated but 2021 drop behaviour.
07dccf5
to
b4bd8b5
Compare
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.
Am I also understanding this correctly that this PR will also stablize the following on all editions?
fn main() {
let x: Option<Option<i32>> = None;
while let Some(x) = x && let Some(_) = x {}
}
Has there been any discussion to stabilize that? My understanding is that only 2024 was discussed and FCP'd, but maybe I missed something. If only 2024 was discussed then we probably can't stabilize any form of let chains with edition < 2024.
/// Whether let chains are allowed on all editions, or it's edition dependent. | ||
/// In case of edition dependence, specify the currently present edition. | ||
pub enum LetChainsPolicy { | ||
AlwaysAllowed, | ||
Edition(Edition), | ||
} |
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.
/// Whether let chains are allowed on all editions, or it's edition dependent. | |
/// In case of edition dependence, specify the currently present edition. | |
pub enum LetChainsPolicy { | |
AlwaysAllowed, | |
Edition(Edition), | |
} | |
/// Whether let chains are allowed on all editions, or it's edition dependent. | |
/// In case of edition dependence, specify the currently present edition. | |
pub enum LetChainsContext { | |
AlwaysAllowed, | |
EditionDependent { current_edition: Edition }, | |
} |
When you write it as LetChainsPolicy
it feels like the edition that you are passing in is some sort of condition that must be passed, as compared to the current edition. I believe this change would help clarify things better.
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.
changed
b4bd8b5
to
d69f6e8
Compare
No you are right, only 2024 has been discussed. The reason why |
d69f6e8
to
5258cb7
Compare
@@ -3425,14 +3429,9 @@ impl<'a> Parser<'a> { | |||
let if_span = self.prev_token.span; | |||
let mut cond = self.parse_match_guard_condition()?; | |||
|
|||
CondChecker::new(self).visit_expr(&mut cond); | |||
CondChecker::new(self, LetChainsPolicy::AlwaysAllowed).visit_expr(&mut cond); |
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.
Would be nice to leave a note here that this is allowed since it is already gated behind if_let_guard
, so it is still unstable and not being stabilized. But that can be done as a followup.
@bors r+ |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 9bfa31f (parent) -> 8bf5a8d (this PR) Test differencesShow 16 test diffsStage 1
Stage 2
Additionally, 2 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 8bf5a8d12feea10dfada53fb2d119283b0e0107c --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (8bf5a8d): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary -1.1%, secondary 2.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -0.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 775.125s -> 776.303s (0.15%) |
Stabilization report
This proposes the stabilization of
let_chains
(tracking issue, RFC 2497) in the 2024 edition of Rust.What is being stabilized
The ability to
&&
-chainlet
statements insideif
andwhile
is being stabilized, allowing intermixture with boolean expressions. The patterns inside thelet
sub-expressions can be irrefutable or refutable.The feature will only be stabilized for the 2024 edition and future editions. Users of past editions will get an error with a hint to update the edition.
closes #53667
Why 2024 edition?
Rust generally tries to ship new features to all editions. So even the oldest editions receive the newest features. However, sometimes a feature requires a breaking change so much that offering the feature without the breaking change makes no sense. This occurs rarely, but has happened in the 2018 edition already with
async
andawait
syntax. It required an edition boundary in order forasync
/await
to become keywords, and the entire feature foots on those keywords.In the instance of let chains, the issue is the drop order of
if let
chains. If we wantif let
chains to be compatible withif let
, drop order makes it hard for us to generate correct MIR. It would be strange to have different behaviour forif let ... {}
andif true && let ... {}
. So it's better to stay consistent withif let
.In edition 2024, drop order changes have been introduced to make
if let
temporaries be lived more shortly. These changes also affectedif let
chains. These changes make sense even if you don't take theif let
chains MIR generation problem into account. But if we want to use them as the solution to the MIR generation problem, we need to restrict let chains to edition 2024 and beyond: for let chains, it's not just a change towards more sensible behaviour, but one required for correct function.Introduction considerations
As edition 2024 is very new, this stabilization PR only makes it possible to use let chains on 2024 without that feature gate, it doesn't mark that feature gate as stable/removed. I would propose to continue offering the
let_chains
feature (behind a feature gate) for a limited time (maybe 3 months after stabilization?) on older editions to allow nightly users to adopt edition 2024 at their own pace. After that, the feature gate shall be marked as stabilized, not removed, and replaced by an error on editions 2021 and below.Implementation history
let_chains
in Rust 1.64 #94927let_else
does not interact withlet_chains
#94974let_chains
] Forbidlet
inside parentheses #95008let
s in certain places #97295let_chains
blocker #98633;
for;
within let-chains #117743{
in let-chains #117770let
or==
on typo'd let-chain #118191irrefutable_let_patterns
on leading patterns ifelse if
let-chains #129394let_chains
references reference#1179Adoption history
In the compiler
Outside of the compiler
Tests
Intentional restrictions
partially-macro-expanded.rs
,macro-expanded.rs
: it is possible to use macros to expand to both the pattern and the expression inside a let chain, but not to the entirelet pat = expr
operand.parens.rs
:if (let pat = expr)
is not allowed in chainsensure-that-let-else-does-not-interact-with-let-chains.rs
:let...else
doesn't support chaining.Overlap with match guards
move-guard-if-let-chain.rs
: test for theuse moved value
error working well in match guards. could maybe be extended with let chains that have more than onelet
shadowing.rs
: shadowing in if let guards works as expectedast-validate-guards.rs
: let chains in match guards require the match guards feature gateSimple cases from the early days
PR #88642 has added some tests with very simple usages of
let else
, mostly as regression tests to early bugs.then-else-blocks.rs
ast-lowering-does-not-wrap-let-chains.rs
issue-90722.rs
issue-92145.rs
Drop order/MIR scoping tests
issue-100276.rs
: let expressions on RHS aren't terminating scopesdrop_order.rs
: exhaustive temporary drop order test for various Rust constructs, including let chainsscope.rs
: match guard scoping testdrop-scope.rs
: another match guard scoping test, ensuring that temporaries in if-let guards live for the armdrop_order_if_let_rescope.rs
: if let rescoping on edition 2024, including chainsmir_let_chains_drop_order.rs
: comprehensive drop order test for let chains, distinguishes editions 2021 and 2024.issue-99938.rs
,issue-99852.rs
both bad MIR ICEs fixed by #102394Linting
irrefutable-lets.rs
: trailing and leading irrefutable let patterns get linted for, others don't. The lint is turned off forelse if
.issue-121070-let-range.rs
: regression test for false positive of the unused parens lint, precedence requires the()
s hereParser: intentional restrictions
disallowed-positions.rs
:let
in expression context is rejected everywhere except at the top levelinvalid-let-in-a-valid-let-context.rs
: nestedlet
is not allowed (let's are no legal expressions just because they are allowed inif
andwhile
).Parser: recovery
issue-103381.rs
: Graceful recovery of incorrect chaining ofif
andif let
semi-in-let-chain.rs
: Ensure that stray;
s in let chains give nice errors (if_chain!
users might be accustomed to;
s)deli-ident-issue-1.rs
,brace-in-let-chain.rs
: Ensure that stray unclosed{
s in let chains give nice errors and hintsMisc
conflicting_bindings.rs
: the conflicting bindings check also works in let chains. Personally, I'd extend it to chains with multiple let's as well.let-chains-attr.rs
: attributes work on let chainsTangential tests with
#![feature(let_chains)]
if-let.rs
: MC/DC coverage tests for let chainslogical_or_in_conditional.rs
: not really about let chains, more about dropping/scoping behaviour of||
stringify.rs
: exhaustive test of thestringify
macroexpanded-interpolation.rs
,expanded-exhaustive.rs
: Exhaustive test of-Zunpretty
diverges-not.rs
: Never type, mostly tangential to let chainsPossible future work
if let Pat(bindings) = expr {}
to be written asif expr is Pat(bindings) {}
(RFC 3573).if let
chains are a natural extension of the already existingif let
syntax, and I'd argue orthogonal towardsis
syntax.let
-chains andis
are not mutually exclusive lang-team#297let ... else
statements. There is no proposed RFC for this however, nor is it implemented on nightly.if
keyword as well, but on stable Rust, they don't supportlet
. The functionality is available via an unstable feature (if_let_guard
tracking issue). Stabilization of let chains affects this feature in so far as match guards containing let chains now only need theif_let_guard
feature gate be present instead of also thelet_chains
feature (NOTE: this PR doesn't implement this simplification, it's left for future work).Open questions / blockers
let
(I don't think this is a blocker): #117977move-guard-if-let-chain.rs
andconflicting_bindings.rs
to have chains with multiple let's: done in 133093let_else
. I think we can live withlet pat = expr
not evaluating asexpr
for macro_rules macros, especially given thatlet pat = expr
is not a legal expression anywhere except insideif
andwhile
.let_chains
again reference#1740