Introduce `let...else` #87688

camsteffen · 2021-08-02T00:31:58Z

Tracking issue: #87335

The trickiest part for me was enforcing the diverging else block with clear diagnostics. Perhaps the obvious solution is to expand to let _: ! = .., but I decided against this because, when a "mismatched type" error is found in typeck, there is no way to trace where in the HIR the expected type originated, AFAICT. In order to pass down this information, I believe we should introduce Expectation::LetElseNever(HirId) or maybe add HirId to Expectation::HasType, but I left that as a future enhancement. For now, I simply assert that the block is ! with a custom ObligationCauseCode, and I think this is clear enough, at least to start. The downside here is that the error points at the entire block rather than the specific expression with the wrong type. I left a todo to this effect.

Overall, I believe this PR is feature-complete with regard to the RFC.

rust-highfive · 2021-08-02T00:32:00Z

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

Some changes occurred in src/tools/rustfmt.

cc @calebcartwright

rust-highfive · 2021-08-02T00:32:02Z

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

src/tools/rustfmt/src/items.rs

bors · 2021-08-02T18:55:28Z

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

cjgillot

Thanks @camsteffen for this implementation! I left a few comments on the lowering part: I am not convinced that you chose the simplest way to express this in HIR. I am interested in your reasonning.

compiler/rustc_ast_lowering/src/local.rs

compiler/rustc_ast_lowering/src/lib.rs

compiler/rustc_ast/src/ast.rs

compiler/rustc_ast_lowering/src/local.rs

cjgillot · 2021-08-03T07:17:51Z

Also, how does this feature articulate with the refactoring in #80357 ?

camsteffen · 2021-08-03T16:26:06Z

Also, how does this feature articulate with the refactoring in #80357 ?

It don't really see any conflict, surprisingly, because that PR touches every let-containing syntax except for let statements. @c410-f3r do you agree with that statement?

camsteffen · 2021-08-04T18:38:38Z

Rebased and...

Changed the lowered HIR structure to let val: $ty = $init; match val { $pat => { $trailing_stmts }, _ => $else_blk }
Added tests for type annotation and lint attributes
Corrected the rustfmt fix

c410-f3r · 2021-08-04T18:51:46Z

Also, how does this feature articulate with the refactoring in #80357 ?

It don't really see any conflict, surprisingly, because that PR touches every let-containing syntax except for let statements. @c410-f3r do you agree with that statement?

#80357 touches more than +2k lines of code and I have doubts about no potential conflicts. My free-time is mostly limited to weekends so a confirmation, if it is worth something, will have to wait.

It's been 8 months of endless rebases and ppl are waiting years for #53667. Personally, one more possible conflicting PR or one less possible conflicting PR doesn't make a difference any more

camsteffen · 2021-08-04T19:02:47Z

#80357 touches more than +2k lines of code and I have doubts about no potential conflicts. My free-time is mostly limited to weekends so a confirmation, if it is worth something, will have to wait.

No rush. I'm not wondering if there are any potential merge conflicts. Just wondering if there is a fundamental conflict with how the two PR's change the AST/HIR. And I don't think there is from my own cursory scan.

c410-f3r · 2021-08-08T13:26:12Z

After cherry-picking, it looked like that most conflicts were more related to missing MatchSource variants than logical stuff although I am not completely sure.

But as stated before with all personal intrinsic reasons, don't mind #80357.

compiler/rustc_parse/src/parser/stmt.rs

src/test/ui/let-else/let-else-brace-before-else.rs

compiler/rustc_infer/src/infer/error_reporting/mod.rs

compiler/rustc_ast_lowering/src/block.rs

camsteffen · 2021-08-15T14:22:49Z

@cjgillot Do you think this is ready to merge after addressing the comments (minor changes) or are you still reviewing?

cjgillot · 2021-08-15T19:22:02Z

@camsteffen This will be ready to merge once the comments have been addressed.

bors · 2021-08-16T03:36:22Z

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

camsteffen · 2021-08-20T21:53:52Z

Is now based on #88187.

After rebasing over #80357, it seemed right to change the desugared form (again) to use the new HIR form of if let. So now the desugared form is let value: type = init; if let pat = value { trailing_stmts } else { diverging_block }.

cjgillot · 2021-08-31T20:11:50Z

@bors r+ rollup=never

bors · 2021-08-31T20:11:51Z

📌 Commit 3ff1d6b has been approved by cjgillot

bors · 2021-09-01T01:02:45Z

⌛ Testing commit 3ff1d6b with merge c2a4088...

bors · 2021-09-01T03:43:30Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing c2a4088 to master...

pnkfelix · 2021-09-08T15:02:27Z

During weekly performance triage, this was flagged as causing small regressions (approximately 1%) in instruction counts on several of the *-doc tests, found via human eye.

Might be worth double-checking to see if that slowdown was expected.

Mark-Simulacrum · 2021-10-04T16:59:12Z

I've so far failed to arrive at any conclusive evidence with this change -- going to keep investigating, though.

The regression seems real -- cachegrind etc do reproduce it -- but it traces to functions like decode read_str and such, which seems rather unhelpful. I'm working now on building a debuginfo-enabled compiler for both before/after to try and get a better sense of the causes here.

Mark-Simulacrum · 2021-10-05T18:08:54Z

This is beginning to look fairly similar to #88881. There's around 20-30k additional instructions executed (for await-call-tree-doc), mostly in Span decoding -- but Span decoding is a very complex piece of code.

I've tried a number of things to try and figure out what the underlying cause for this is, but my current belief is that we're just seeing the effects of different optimizer decisions -- no actual difference in behavior -- but it is both unclear why that is or what's causing it. I've looked at the disassembly before/after and LLVM IR (manually dumped out of rustc with bootstrap adjustments locally), but none of that really yielded any clear indicators of what we can do to fix this.

I'll post further updates as I continue my investigation...

Mark-Simulacrum · 2021-10-11T18:53:40Z

It looks like the Span decoding function has worse codegen with this PR. As best as I can tell, this isn't readily "fixable" and seems not directly connected to this PR, but adjustments to the LocalKind Encode impl do seem to fix it (I can't explain why changes to encoding affect decoding). I can reproduce the regression locally with and without PGO, so it doesn't seem related to that either.

AFAICT, there are some pretty weird choices made by LLVM in the assembly for this function -- I've uploaded disassembly here. For example, we can see this diff near the top of the function:

-                       mov     rsi, qword ptr [r13 + 8]
-                       mov     rdi, qword ptr [r13 + 16]
-                       cmp     rdi, rsi
+                       mov     r15, qword ptr [rbp + 8]
+                       mov     rdi, qword ptr [rbp + 16]
+                       mov     rsi, rdi
+                       sub     rsi, r15

If we ignore the register renaming, the meaningful difference is the cmp is replaced with a mov and sub. However, there's no real reason for LLVM to do that. rsi is not used again before being overwritten I think, so this is just wasting a mov for no real reason. It seems like the majority of the changes to this function are similar "weird" small losses, not anything terribly material. There's also some extra stack spilling (note that the stack size increases by 32 bytes).

Replacing the LocalKind Encode impl with empty stubs or otherwise adjusting it seems to fix the codegen here. The LocalKind Encode impl is not invoked by any typical compilation (tested via inserting a panic!). I'm fairly certain now that the indirect effects on Span decoding are due to shuffling of CGU partitions for the rustc_metadata crate

I instrumented the CGU partitioning (-Zprint-mono-items=lazy) on the minimal change which replaces the Encode impl for LocalKind with an empty stub, and locally it looks like the only two crate which have changed are rustc_interface and rustc_metadata.

For rustc_metadata, if we ignore changes in which CGU gets which mono item, the only additions are the extra emit_enum/emit_enum_variant, etc. along with the closures for those:

@@ -3895,0 +3896 @@
+MONO_ITEM fn <rmeta::encoder::EncodeContext as rustc_serialize::Encoder>::emit_enum::<[closure@rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for >
@@ -4311,0 +4313,3 @@
+MONO_ITEM fn <rmeta::encoder::EncodeContext as rustc_serialize::Encoder>::emit_enum_variant::<[closure@rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeConte>
+MONO_ITEM fn <rmeta::encoder::EncodeContext as rustc_serialize::Encoder>::emit_enum_variant::<[closure@rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeConte>
+MONO_ITEM fn <rmeta::encoder::EncodeContext as rustc_serialize::Encoder>::emit_enum_variant::<[closure@rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeConte>
@@ -5977,0 +5982,3 @@
+MONO_ITEM fn <rmeta::encoder::EncodeContext as rustc_serialize::Encoder>::emit_enum_variant_arg::<[closure@rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeC>
+MONO_ITEM fn <rmeta::encoder::EncodeContext as rustc_serialize::Encoder>::emit_enum_variant_arg::<[closure@rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeC>
+MONO_ITEM fn <rmeta::encoder::EncodeContext as rustc_serialize::Encoder>::emit_enum_variant_arg::<[closure@rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeC>
@@ -7817 +7823,0 @@
-MONO_ITEM fn <rustc_ast::LocalKind as rustc_serialize::Encodable<rmeta::encoder::EncodeContext>>::encode
@@ -19645,0 +19652,8 @@
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode::{closure#0}
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode::{closure#0}::{closure#0}
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode::{closure#0}::{closure#1}
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode::{closure#0}::{closure#1}::{closure#0}
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode::{closure#0}::{closure#2}
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode::{closure#0}::{closure#2}::{closure#0}
+MONO_ITEM fn rustc_ast::ast::_DERIVE_rustc_serialize_Encodable_E_FOR_LocalKind::<impl rustc_serialize::Encodable<rmeta::encoder::EncodeContext> for rustc_ast::LocalKind>::encode::{closure#0}::{closure#2}::{closure#1}

This is pretty expected, since all of these are getting instantiated with rmeta::encoder::EncodeContext for the first time (it's defined in rustc_metadata). This all mostly comes about from a pretty deep callstack, rooted in attribute encoding (since those contain arbitrary expressions and paths, we get essentially all AST types serialize and deserialize impl's codegened).

The partitioning has changed for CGUs 0, 3, and 4.

   8622 after-with-empty-encode/rustc-metadata/rustc_metadata.f82b8cc0-cgu.0
   2824 after-with-empty-encode/rustc-metadata/rustc_metadata.f82b8cc0-cgu.3
   6021 after-with-empty-encode/rustc-metadata/rustc_metadata.f82b8cc0-cgu.4
   8634 after-mono/rustc-metadata/rustc_metadata.f82b8cc0-cgu.0
   4466 after-mono/rustc-metadata/rustc_metadata.f82b8cc0-cgu.3
   4377 after-mono/rustc-metadata/rustc_metadata.f82b8cc0-cgu.4

Generally it seems like CGU 3 is now much larger -- presumably around 1,000 items moved from being put into CGU 4 into CGU 3.

This included a move of fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_enum_variant_arg::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> st... from CGU 4 to 3. I believe (though have no hard evidence for it) that this likely caused transitive effects making the codegen worse. Notably other bits of that impl didn't move, so probably inlining is suffering here. (The below is a grep for rustc_span::Span as rustc_serialize::Decodable<.*>>::decode in rustc_metadata CGUs).

--- empty-encode-span	2021-10-11 14:42:56.626182306 -0400
+++ full-encode-span	2021-10-11 14:42:36.389748828 -0400
@@ -2,9 +2,9 @@
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.15:fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_enum_variant_arg::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}> Internal
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.15:fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_struct_field::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}> Internal
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.3:fn <for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode} as std::ops::FnOnce<(&mut rmeta::decoder::DecodeContext,)>>::call_once - shim(for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}) Internal
+rustc-metadata/rustc_metadata.f82b8cc0-cgu.3:fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_enum_variant_arg::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}> Internal
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.3:fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_struct_field::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}> Internal
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.4:fn <for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode} as std::ops::FnOnce<(&mut rmeta::decoder::DecodeContext,)>>::call_once - shim(for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}) Internal
-rustc-metadata/rustc_metadata.f82b8cc0-cgu.4:fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_enum_variant_arg::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}> Internal
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.4:fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_struct_field::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}> Internal
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.7:fn <for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode} as std::ops::FnOnce<(&mut rmeta::decoder::DecodeContext,)>>::call_once - shim(for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}) Internal
 rustc-metadata/rustc_metadata.f82b8cc0-cgu.7:fn <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::read_struct_field::<rustc_span::Span, for<'r> fn(&'r mut rmeta::decoder::DecodeContext) -> std::result::Result<rustc_span::Span, <rmeta::decoder::DecodeContext as rustc_serialize::Decoder>::Error> {<rustc_span::Span as rustc_serialize::Decodable<rmeta::decoder::DecodeContext>>::decode}> Internal

Overall I think this means we probably can't really do much about the hit here -- it's just more CGU partitioning sometimes leading to suboptimal results. It's possible that adding some targeted #[inline] annotations or otherwise shifting around code might help with this, but generally that seems brittle and not particularly warranted. I'm going to mark this regression as triaged, because I don't think we can "fix" it at this time. Hopefully we'll eventually have a nicer partitioning algorithm that avoids this kind of trouble.

In the meantime, it's possible we can help by avoiding so much transitive codegen being necessary in rustc_metadata for all the serialize impls for rustc_ast and such. But that's a hard project and not one with obvious solutions, so I think we shouldn't block triaging this PR on it.

@rustbot label +perf-regression-triaged

…plett Stabilize `let else` :tada: **Stabilizes the `let else` feature, added by [RFC 3137](rust-lang/rfcs#3137 🎉 Reference PR: rust-lang/reference#1156 closes rust-lang#87335 (`let else` tracking issue) FCP: rust-lang#93628 (comment) ---------- ## Stabilization report ### Summary The feature allows refutable patterns in `let` statements if the expression is followed by a diverging `else`: ```Rust fn get_count_item(s: &str) -> (u64, &str) { let mut it = s.split(' '); let (Some(count_str), Some(item)) = (it.next(), it.next()) else { panic!("Can't segment count item pair: '{s}'"); }; let Ok(count) = u64::from_str(count_str) else { panic!("Can't parse integer: '{count_str}'"); }; (count, item) } assert_eq!(get_count_item("3 chairs"), (3, "chairs")); ``` ### Differences from the RFC / Desugaring Outside of desugaring I'm not aware of any differences between the implementation and the RFC. The chosen desugaring has been changed from the RFC's [original](https://rust-lang.github.io/rfcs/3137-let-else.html#reference-level-explanations). You can read a detailed discussion of the implementation history of it in `@cormacrelf` 's [summary](rust-lang#93628 (comment)) in this thread, as well as the [followup](rust-lang#93628 (comment)). Since that followup, further changes have happened to the desugaring, in rust-lang#98574, rust-lang#99518, rust-lang#99954. The later changes were mostly about the drop order: On match, temporaries drop in the same order as they would for a `let` declaration. On mismatch, temporaries drop before the `else` block. ### Test cases In chronological order as they were merged. Added by df9a2e0 (rust-lang#87688): * [`ui/pattern/usefulness/top-level-alternation.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/pattern/usefulness/top-level-alternation.rs) to ensure the unreachable pattern lint visits patterns inside `let else`. Added by 5b95df4 (rust-lang#87688): * [`ui/let-else/let-else-bool-binop-init.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-bool-binop-init.rs) to ensure that no lazy boolean expressions (using `&&` or `||`) are allowed in the expression, as the RFC mandates. * [`ui/let-else/let-else-brace-before-else.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-brace-before-else.rs) to ensure that no `}` directly preceding the `else` is allowed in the expression, as the RFC mandates. * [`ui/let-else/let-else-check.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-check.rs) to ensure that `#[allow(...)]` attributes added to the entire `let` statement apply for the `else` block. * [`ui/let-else/let-else-irrefutable.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-irrefutable.rs) to ensure that the `irrefutable_let_patterns` lint fires. * [`ui/let-else/let-else-missing-semicolon.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-missing-semicolon.rs) to ensure the presence of semicolons at the end of the `let` statement. * [`ui/let-else/let-else-non-diverging.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-non-diverging.rs) to ensure the `else` block diverges. * [`ui/let-else/let-else-run-pass.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-run-pass.rs) to ensure the feature works in some simple test case settings. * [`ui/let-else/let-else-scope.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-scope.rs) to ensure the bindings created by the outer `let` expression are not available in the `else` block of it. Added by bf7c32a (rust-lang#89965): * [`ui/let-else/issue-89960.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/issue-89960.rs) as a regression test for the ICE-on-error bug rust-lang#89960 . Later in 102b912 this got removed in favour of more comprehensive tests. Added by 8565419 (rust-lang#89974): * [`ui/let-else/let-else-if.rs`](https://github.com/rust-lang/rust/blob/1.58.1/src/test/ui/let-else/let-else-if.rs) to test for the improved error message that points out that `let else if` is not possible. Added by 9b45713: * [`ui/let-else/let-else-allow-unused.rs`](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-allow-unused.rs) as a regression test for rust-lang#89807, to ensure that `#[allow(...)]` attributes added to the entire `let` statement apply for bindings created by the `let else` pattern. Added by 61bcd8d (rust-lang#89841): * [`ui/let-else/let-else-non-copy.rs`](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-non-copy.rs) to ensure that a copy is performed out of non-copy wrapper types. This mirrors `if let` behaviour. The test case bases on rustc internal changes originally meant for rust-lang#89933 but then removed from the PR due to the error prior to the improvements of rust-lang#89841. * [`ui/let-else/let-else-source-expr-nomove-pass.rs `](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-source-expr-nomove-pass.rs) to ensure that while there is a move of the binding in the successful case, the `else` case can still access the non-matching value. This mirrors `if let` behaviour. Added by 102b912 (rust-lang#89841): * [`ui/let-else/let-else-ref-bindings.rs`](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-ref-bindings.rs) and [`ui/let-else/let-else-ref-bindings-pass.rs `](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-ref-bindings-pass.rs) to check `ref` and `ref mut` keywords in the pattern work correctly and error when needed. Added by 2715c5f (rust-lang#89841): * Match ergonomic tests adapted from the `rfc2005` test suite. Added by fec8a50 (rust-lang#89841): * [`ui/let-else/let-else-deref-coercion-annotated.rs`](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-deref-coercion-annotated.rs) and [`ui/let-else/let-else-deref-coercion.rs`](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-deref-coercion.rs) to check deref coercions. #### Added since this stabilization report was originally written (2022-02-09) Added by 76ea566 (rust-lang#94211): * [`ui/let-else/let-else-destructuring.rs`](https://github.com/rust-lang/rust/blob/1.63.0/src/test/ui/let-else/let-else-destructuring.rs) to give a nice error message if an user tries to do an assignment with a (possibly refutable) pattern and an `else` block, like asked for in rust-lang#93995. Added by e7730dc (rust-lang#94208): * [`ui/let-else/let-else-allow-in-expr.rs`](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-allow-in-expr.rs) to test whether `#[allow(unused_variables)]` works in the expr, as well as its non presence, as well as putting it on the entire `let else` *affects* the expr, too. This was adding a missing test as pointed out by the stabilization report. * Expansion of `ui/let-else/let-else-allow-unused.rs` and `ui/let-else/let-else-check.rs` to ensure that non-presence of `#[allow(unused)]` does issue the unused lint. This was adding a missing test case as pointed out by the stabilization report. Added by 5bd7106 (rust-lang#94208): * [`ui/let-else/let-else-slicing-error.rs`](https://github.com/rust-lang/rust/blob/1.61.0/src/test/ui/let-else/let-else-slicing-error.rs), a regression test for rust-lang#92069, which got fixed without addition of a regression test. This resolves a missing test as pointed out by the stabilization report. Added by 5374688 (rust-lang#98574): * [`src/test/ui/async-await/async-await-let-else.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/async-await/async-await-let-else.rs) to test the interaction of async/await with `let else` Added by 6c529de (rust-lang#98574): * [`src/test/ui/let-else/let-else-temporary-lifetime.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/let-else-temporary-lifetime.rs) as a (partial) regression test for rust-lang#98672 Added by 9b56640 (rust-lang#99518): * [`src/test/ui/let-else/let-else-temp-borrowck.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/let-else-temporary-lifetime.rs) as a regression test for rust-lang#93951 * Extension of `src/test/ui/let-else/let-else-temporary-lifetime.rs` to include a partial regression test for rust-lang#98672 (especially regarding `else` drop order) Added by baf9a7c (rust-lang#99518): * Extension of `src/test/ui/let-else/let-else-temporary-lifetime.rs` to include a partial regression test for rust-lang#93951, similar to `let-else-temp-borrowck.rs` Added by 60be2de (rust-lang#99518): * Extension of `src/test/ui/let-else/let-else-temporary-lifetime.rs` to include a program that can now be compiled thanks to borrow checker implications of rust-lang#99518 Added by 47a7a91 (rust-lang#100132): * [`src/test/ui/let-else/issue-100103.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/issue-100103.rs), as a regression test for rust-lang#100103, to ensure that there is no ICE when doing `Err(...)?` inside else blocks. Added by e3c5bd6 (rust-lang#100443): * [`src/test/ui/let-else/let-else-then-diverge.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/let-else-then-diverge.rs), to verify that there is no unreachable code error with the current desugaring. Added by 9818526 (rust-lang#100443): * [`src/test/ui/let-else/issue-94176.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/issue-94176.rs), to make sure that a correct span is emitted for a missing trailing expression error. Regression test for rust-lang#94176. Added by e182d12 (rust-lang#100434): * [src/test/ui/unpretty/pretty-let-else.rs](https://github.com/rust-lang/rust/blob/master/src/test/ui/unpretty/pretty-let-else.rs), as a regression test to ensure pretty printing works for `let else` (this bug surfaced in many different ways) Added by e262856 (rust-lang#99954): * [`src/test/ui/let-else/let-else-temporary-lifetime.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/let-else-temporary-lifetime.rs) extended to contain & borrows as well, as this was identified as an earlier issue with the desugaring: rust-lang#98672 (comment) Added by 2d8460e (rust-lang#99291): * [`src/test/ui/let-else/let-else-drop-order.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/let-else-drop-order.rs) a matrix based test for various drop order behaviour of `let else`. Especially, it verifies equality of `let` and `let else` drop orders, [resolving](rust-lang#93628 (comment)) a [stabilization blocker](rust-lang#93628 (comment)). Added by 1b87ce0 (rust-lang#101410): * Edit to `src/test/ui/let-else/let-else-temporary-lifetime.rs` to add the `-Zvalidate-mir` flag, as a regression test for rust-lang#99228 Added by af591eb (rust-lang#101410): * [`src/test/ui/let-else/issue-99975.rs`](https://github.com/rust-lang/rust/blob/master/src/test/ui/let-else/issue-99975.rs) as a regression test for the ICE rust-lang#99975. Added by this PR: * `ui/let-else/let-else.rs`, a simple run-pass check, similar to `ui/let-else/let-else-run-pass.rs`. ### Things not currently tested * ~~The `#[allow(...)]` tests check whether allow works, but they don't check whether the non-presence of allow causes a lint to fire.~~ → *test added by e7730dc* * ~~There is no `#[allow(...)]` test for the expression, as there are tests for the pattern and the else block.~~ → *test added by e7730dc* * ~~`let-else-brace-before-else.rs` forbids the `let ... = {} else {}` pattern and there is a rustfix to obtain `let ... = ({}) else {}`. I'm not sure whether the `.fixed` files are checked by the tooling that they compile. But if there is no such check, it would be neat to make sure that `let ... = ({}) else {}` compiles.~~ → *test added by e7730dc* * ~~rust-lang#92069 got closed as fixed, but no regression test was added. Not sure it's worth to add one.~~ → *test added by 5bd7106* * ~~consistency between `let else` and `if let` regarding lifetimes and drop order: rust-lang#93628 (comment) → *test added by 2d8460e* Edit: they are all tested now. ### Possible future work / Refutable destructuring assignments [RFC 2909](https://rust-lang.github.io/rfcs/2909-destructuring-assignment.html) specifies destructuring assignment, allowing statements like `FooBar { a, b, c } = foo();`. As it was stabilized, destructuring assignment only allows *irrefutable* patterns, which before the advent of `let else` were the only patterns that `let` supported. So the combination of `let else` and destructuring assignments gives reason to think about extensions of the destructuring assignments feature that allow refutable patterns, discussed in rust-lang#93995. A naive mapping of `let else` to destructuring assignments in the form of `Some(v) = foo() else { ... };` might not be the ideal way. `let else` needs a diverging `else` clause as it introduces new bindings, while assignments have a default behaviour to fall back to if the pattern does not match, in the form of not performing the assignment. Thus, there is no good case to require divergence, or even an `else` clause at all, beyond the need for having *some* introducer syntax so that it is clear to readers that the assignment is not a given (enums and structs look similar). There are better candidates for introducer syntax however than an empty `else {}` clause, like `maybe` which could be added as a keyword on an edition boundary: ```Rust let mut v = 0; maybe Some(v) = foo(&v); maybe Some(v) = foo(&v) else { bar() }; ``` Further design discussion is left to an RFC, or the linked issue.

rust-highfive assigned davidtwco Aug 2, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 2, 2021

This comment has been minimized.

Sign in to view

calebcartwright reviewed Aug 2, 2021

View reviewed changes

src/tools/rustfmt/src/items.rs Show resolved Hide resolved

cjgillot self-assigned this Aug 2, 2021

cjgillot requested changes Aug 2, 2021

View reviewed changes

camsteffen force-pushed the let-else branch from 62e73a6 to 5c5f92c Compare August 4, 2021 18:34

This comment has been minimized.

Sign in to view

camsteffen force-pushed the let-else branch from 5c5f92c to ed25e6e Compare August 4, 2021 19:35

camsteffen mentioned this pull request Aug 4, 2021

Refactor lower_stmts #87773

Closed

Fishrock123 reviewed Aug 9, 2021

View reviewed changes

compiler/rustc_parse/src/parser/stmt.rs Show resolved Hide resolved

Fishrock123 reviewed Aug 9, 2021

View reviewed changes

src/test/ui/let-else/let-else-brace-before-else.rs Show resolved Hide resolved

compiler/rustc_infer/src/infer/error_reporting/mod.rs Outdated Show resolved Hide resolved

compiler/rustc_infer/src/infer/error_reporting/mod.rs Show resolved Hide resolved

davidtwco removed their assignment Aug 12, 2021

cjgillot reviewed Aug 14, 2021

View reviewed changes

compiler/rustc_ast_lowering/src/block.rs Outdated Show resolved Hide resolved

camsteffen force-pushed the let-else branch from ed25e6e to 133a0a3 Compare August 20, 2021 21:48

camsteffen force-pushed the let-else branch from 133a0a3 to 27e5182 Compare August 20, 2021 21:56

camsteffen added 4 commits August 30, 2021 20:18

Calculate LetSource later

dc028f6

Handle irrufutable or unreachable let-else

df9a2e0

Add let-else tests

5b95df4

Fix tests

3ff1d6b

camsteffen force-pushed the let-else branch from 331761f to 3ff1d6b Compare August 31, 2021 01:18

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 31, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 1, 2021

bors merged commit c2a4088 into rust-lang:master Sep 1, 2021

rustbot added this to the 1.56.0 milestone Sep 1, 2021

This was referenced Sep 1, 2021

Add expansion to while desugar spans #88175

Merged

Lower only one HIR owner at a time #87234

Merged

fix(rustc_lint): better detect when parens are necessary #88547

Merged

camsteffen deleted the let-else branch September 1, 2021 12:50

Fishrock123 mentioned this pull request Sep 1, 2021

Tracking Issue for RFC 3137: let-else statements #87335

Closed

7 tasks

pnkfelix added the perf-regression Performance regression. label Sep 8, 2021

pr2502 mentioned this pull request Oct 6, 2021

Feature request: parse new let_else syntax rust-lang/rust-analyzer#10469

Closed

rustbot added the perf-regression-triaged The performance regression has been triaged. label Oct 11, 2021

fee1-dead mentioned this pull request Dec 14, 2021

Eliminate ConstnessAnd again #91549

Merged

cormacrelf mentioned this pull request Feb 15, 2022

Stabilize let else #93628

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `let...else` #87688

Introduce `let...else` #87688

camsteffen commented Aug 2, 2021 •

edited

Loading

rust-highfive commented Aug 2, 2021

rust-highfive commented Aug 2, 2021

This comment has been minimized.

bors commented Aug 2, 2021

cjgillot left a comment

cjgillot commented Aug 3, 2021

camsteffen commented Aug 3, 2021

camsteffen commented Aug 4, 2021 •

edited

Loading

c410-f3r commented Aug 4, 2021 •

edited

Loading

This comment has been minimized.

camsteffen commented Aug 4, 2021

c410-f3r commented Aug 8, 2021 •

edited

Loading

camsteffen commented Aug 15, 2021

cjgillot commented Aug 15, 2021

bors commented Aug 16, 2021

camsteffen commented Aug 20, 2021

cjgillot commented Aug 31, 2021

bors commented Aug 31, 2021

bors commented Sep 1, 2021

bors commented Sep 1, 2021

pnkfelix commented Sep 8, 2021

Mark-Simulacrum commented Oct 4, 2021

Mark-Simulacrum commented Oct 5, 2021

Mark-Simulacrum commented Oct 11, 2021

Introduce let...else #87688

Introduce let...else #87688

Conversation

camsteffen commented Aug 2, 2021 • edited Loading

rust-highfive commented Aug 2, 2021

rust-highfive commented Aug 2, 2021

This comment has been minimized.

bors commented Aug 2, 2021

cjgillot left a comment

Choose a reason for hiding this comment

cjgillot commented Aug 3, 2021

camsteffen commented Aug 3, 2021

camsteffen commented Aug 4, 2021 • edited Loading

c410-f3r commented Aug 4, 2021 • edited Loading

This comment has been minimized.

camsteffen commented Aug 4, 2021

c410-f3r commented Aug 8, 2021 • edited Loading

camsteffen commented Aug 15, 2021

cjgillot commented Aug 15, 2021

bors commented Aug 16, 2021

camsteffen commented Aug 20, 2021

cjgillot commented Aug 31, 2021

bors commented Aug 31, 2021

bors commented Sep 1, 2021

bors commented Sep 1, 2021

pnkfelix commented Sep 8, 2021

Mark-Simulacrum commented Oct 4, 2021

Mark-Simulacrum commented Oct 5, 2021

Mark-Simulacrum commented Oct 11, 2021

Introduce `let...else` #87688

Introduce `let...else` #87688

camsteffen commented Aug 2, 2021 •

edited

Loading

camsteffen commented Aug 4, 2021 •

edited

Loading

c410-f3r commented Aug 4, 2021 •

edited

Loading

c410-f3r commented Aug 8, 2021 •

edited

Loading