From 27592844d3ce0d92bb441795ece642fcaa3d4429 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maja=20K=C4=85dzio=C5=82ka?= Date: Wed, 26 Mar 2025 03:26:09 +0100 Subject: [PATCH 1/4] ExprUseVisitor: properly report discriminant reads This solves the "can't find the upvar" ICEs that resulted from `maybe_read_scrutinee` being unfit for purpose. --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 102 +++++++++-- tests/crashes/137467-1.rs | 17 -- tests/crashes/137467-2.rs | 18 -- tests/crashes/137467-3.rs | 8 - .../2229_closure_analysis/capture-enums.rs | 2 + .../capture-enums.stderr | 26 ++- .../match/patterns-capture-analysis.rs | 5 + .../match/patterns-capture-analysis.stderr | 79 +++++--- .../only-inhabited-variant-stable.rs | 23 +++ .../only-inhabited-variant-stable.stderr | 20 +++ ...habited-variant.exhaustive_patterns.stderr | 20 +++ .../only-inhabited-variant.normal.stderr | 20 +++ ...tivariant.rs => only-inhabited-variant.rs} | 8 +- tests/ui/closures/or-patterns-issue-137467.rs | 170 ++++++++++++++++++ 14 files changed, 426 insertions(+), 92 deletions(-) delete mode 100644 tests/crashes/137467-1.rs delete mode 100644 tests/crashes/137467-2.rs delete mode 100644 tests/crashes/137467-3.rs create mode 100644 tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs create mode 100644 tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr create mode 100644 tests/ui/closures/2229_closure_analysis/only-inhabited-variant.exhaustive_patterns.stderr create mode 100644 tests/ui/closures/2229_closure_analysis/only-inhabited-variant.normal.stderr rename tests/ui/closures/2229_closure_analysis/{run_pass/multivariant.rs => only-inhabited-variant.rs} (55%) create mode 100644 tests/ui/closures/or-patterns-issue-137467.rs diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index f5e0f01e4c573..7999b0f05e88e 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -942,6 +942,19 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx } /// The core driver for walking a pattern + /// + /// This should mirror how pattern-matching gets lowered to MIR, as + /// otherwise lowering will ICE when trying to resolve the upvars. + /// + /// However, it is okay to approximate it here by doing *more* accesses than + /// the actual MIR builder will, which is useful when some checks are too + /// cumbersome to perform here. For example, if after typeck it becomes + /// clear that only one variant of an enum is inhabited, and therefore a + /// read of the discriminant is not necessary, `walk_pat` will have + /// over-approximated the necessary upvar capture granularity. + /// + /// Do note that discrepancies like these do still create obscure corners + /// in the semantics of the language, and should be avoided if possible. #[instrument(skip(self), level = "debug")] fn walk_pat( &self, @@ -951,6 +964,11 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx ) -> Result<(), Cx::Error> { let tcx = self.cx.tcx(); self.cat_pattern(discr_place.clone(), pat, &mut |place, pat| { + debug!("walk_pat: pat.kind={:?}", pat.kind); + let read_discriminant = || { + self.delegate.borrow_mut().borrow(place, discr_place.hir_id, BorrowKind::Immutable); + }; + match pat.kind { PatKind::Binding(_, canonical_id, ..) => { debug!("walk_pat: binding place={:?} pat={:?}", place, pat); @@ -973,11 +991,7 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx // binding when lowering pattern guards to ensure that the guard does not // modify the scrutinee. if has_guard { - self.delegate.borrow_mut().borrow( - place, - discr_place.hir_id, - BorrowKind::Immutable, - ); + read_discriminant(); } // It is also a borrow or copy/move of the value being matched. @@ -1011,13 +1025,71 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx PatKind::Never => { // A `!` pattern always counts as an immutable read of the discriminant, // even in an irrefutable pattern. - self.delegate.borrow_mut().borrow( - place, - discr_place.hir_id, - BorrowKind::Immutable, - ); + read_discriminant(); + } + PatKind::Expr(PatExpr { kind: PatExprKind::Path(qpath), hir_id, span }) => { + // A `Path` pattern is just a name like `Foo`. This is either a + // named constant or else it refers to an ADT variant + + let res = self.cx.typeck_results().qpath_res(qpath, *hir_id); + match res { + Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => { + // Named constants have to be equated with the value + // being matched, so that's a read of the value being matched. + // + // FIXME: Does the MIR code skip this read when matching on a ZST? + // If so, we can also skip it here. + read_discriminant(); + } + _ => { + // Otherwise, this is a struct/enum variant, and so it's + // only a read if we need to read the discriminant. + if self.is_multivariant_adt(place.place.ty(), *span) { + read_discriminant(); + } + } + } + } + PatKind::Expr(_) | PatKind::Range(..) => { + // When matching against a literal or range, we need to + // borrow the place to compare it against the pattern. + // + // FIXME: What if the type being matched only has one + // possible value? + // FIXME: What if the range is the full range of the type + // and doesn't actually require a discriminant read? + read_discriminant(); + } + PatKind::Struct(..) | PatKind::TupleStruct(..) => { + if self.is_multivariant_adt(place.place.ty(), pat.span) { + read_discriminant(); + } + } + PatKind::Slice(lhs, wild, rhs) => { + // We don't need to test the length if the pattern is `[..]` + if matches!((lhs, wild, rhs), (&[], Some(_), &[])) + // Arrays have a statically known size, so + // there is no need to read their length + || place.place.ty().peel_refs().is_array() + { + // No read necessary + } else { + read_discriminant(); + } + } + PatKind::Or(_) + | PatKind::Box(_) + | PatKind::Ref(..) + | PatKind::Guard(..) + | PatKind::Tuple(..) + | PatKind::Wild + | PatKind::Missing + | PatKind::Err(_) => { + // If the PatKind is Or, Box, Ref, Guard, or Tuple, the relevant accesses + // are made later as these patterns contains subpatterns. + // If the PatKind is Missing, Wild or Err, any relevant accesses are made when processing + // the other patterns that are part of the match } - _ => {} } Ok(()) @@ -1880,6 +1952,14 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx self.cat_deref(hir_id, base) } + /// Checks whether a type has multiple variants, and therefore, whether a + /// read of the discriminant might be necessary. Note that the actual MIR + /// builder code does a more specific check, filtering out variants that + /// happen to be uninhabited. + /// + /// Here, we cannot perform such an accurate checks, because querying + /// whether a type is inhabited requires that it has been fully inferred, + /// which cannot be guaranteed at this point. fn is_multivariant_adt(&self, ty: Ty<'tcx>, span: Span) -> bool { if let ty::Adt(def, _) = self.cx.try_structurally_resolve_type(span, ty).kind() { // Note that if a non-exhaustive SingleVariant is defined in another crate, we need diff --git a/tests/crashes/137467-1.rs b/tests/crashes/137467-1.rs deleted file mode 100644 index b6bff2bdc4e86..0000000000000 --- a/tests/crashes/137467-1.rs +++ /dev/null @@ -1,17 +0,0 @@ -//@ known-bug: #137467 -//@ edition: 2021 -enum Camera { - Normal { base_transform: i32 }, - Volume { transform: i32 }, -} - -fn draw_ui(camera: &mut Camera) { - || { - let (Camera::Normal { - base_transform: _transform, - } - | Camera::Volume { - transform: _transform, - }) = camera; - }; -} diff --git a/tests/crashes/137467-2.rs b/tests/crashes/137467-2.rs deleted file mode 100644 index a70ea92b22dc2..0000000000000 --- a/tests/crashes/137467-2.rs +++ /dev/null @@ -1,18 +0,0 @@ -//@ known-bug: #137467 -//@ edition: 2021 - -enum Camera { - Normal { base_transform: i32 }, - Volume { transform: i32 }, -} - -fn draw_ui(camera: &mut Camera) { - || { - let (Camera::Normal { - base_transform: _, - } - | Camera::Volume { - transform: _, - }) = camera; - }; -} diff --git a/tests/crashes/137467-3.rs b/tests/crashes/137467-3.rs deleted file mode 100644 index cb81a9a912e7c..0000000000000 --- a/tests/crashes/137467-3.rs +++ /dev/null @@ -1,8 +0,0 @@ -//@ known-bug: #137467 -//@ edition: 2021 - -fn meow(x: (u32, u32, u32)) { - let f = || { - let ((0, a, _) | (_, _, a)) = x; - }; -} diff --git a/tests/ui/closures/2229_closure_analysis/capture-enums.rs b/tests/ui/closures/2229_closure_analysis/capture-enums.rs index d9c06a68c95b9..4c600ccdaa438 100644 --- a/tests/ui/closures/2229_closure_analysis/capture-enums.rs +++ b/tests/ui/closures/2229_closure_analysis/capture-enums.rs @@ -22,6 +22,7 @@ fn multi_variant_enum() { //~| ERROR Min Capture analysis includes: if let Info::Point(_, _, str) = point { //~^ NOTE: Capturing point[] -> Immutable + //~| NOTE: Capturing point[] -> Immutable //~| NOTE: Capturing point[(2, 0)] -> ByValue //~| NOTE: Min Capture point[] -> ByValue println!("{}", str); @@ -29,6 +30,7 @@ fn multi_variant_enum() { if let Info::Meta(_, v) = meta { //~^ NOTE: Capturing meta[] -> Immutable + //~| NOTE: Capturing meta[] -> Immutable //~| NOTE: Capturing meta[(1, 1)] -> ByValue //~| NOTE: Min Capture meta[] -> ByValue println!("{:?}", v); diff --git a/tests/ui/closures/2229_closure_analysis/capture-enums.stderr b/tests/ui/closures/2229_closure_analysis/capture-enums.stderr index 89a879cec468b..b62384ffe12e0 100644 --- a/tests/ui/closures/2229_closure_analysis/capture-enums.stderr +++ b/tests/ui/closures/2229_closure_analysis/capture-enums.stderr @@ -9,7 +9,7 @@ LL | let c = #[rustc_capture_analysis] = note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date error[E0658]: attributes on expressions are experimental - --> $DIR/capture-enums.rs:48:13 + --> $DIR/capture-enums.rs:50:13 | LL | let c = #[rustc_capture_analysis] | ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -34,18 +34,28 @@ note: Capturing point[] -> Immutable | LL | if let Info::Point(_, _, str) = point { | ^^^^^ +note: Capturing point[] -> Immutable + --> $DIR/capture-enums.rs:23:41 + | +LL | if let Info::Point(_, _, str) = point { + | ^^^^^ note: Capturing point[(2, 0)] -> ByValue --> $DIR/capture-enums.rs:23:41 | LL | if let Info::Point(_, _, str) = point { | ^^^^^ note: Capturing meta[] -> Immutable - --> $DIR/capture-enums.rs:30:35 + --> $DIR/capture-enums.rs:31:35 + | +LL | if let Info::Meta(_, v) = meta { + | ^^^^ +note: Capturing meta[] -> Immutable + --> $DIR/capture-enums.rs:31:35 | LL | if let Info::Meta(_, v) = meta { | ^^^^ note: Capturing meta[(1, 1)] -> ByValue - --> $DIR/capture-enums.rs:30:35 + --> $DIR/capture-enums.rs:31:35 | LL | if let Info::Meta(_, v) = meta { | ^^^^ @@ -67,13 +77,13 @@ note: Min Capture point[] -> ByValue LL | if let Info::Point(_, _, str) = point { | ^^^^^ note: Min Capture meta[] -> ByValue - --> $DIR/capture-enums.rs:30:35 + --> $DIR/capture-enums.rs:31:35 | LL | if let Info::Meta(_, v) = meta { | ^^^^ error: First Pass analysis includes: - --> $DIR/capture-enums.rs:52:5 + --> $DIR/capture-enums.rs:54:5 | LL | / || { LL | | @@ -85,13 +95,13 @@ LL | | }; | |_____^ | note: Capturing point[(2, 0)] -> ByValue - --> $DIR/capture-enums.rs:55:47 + --> $DIR/capture-enums.rs:57:47 | LL | let SingleVariant::Point(_, _, str) = point; | ^^^^^ error: Min Capture analysis includes: - --> $DIR/capture-enums.rs:52:5 + --> $DIR/capture-enums.rs:54:5 | LL | / || { LL | | @@ -103,7 +113,7 @@ LL | | }; | |_____^ | note: Min Capture point[(2, 0)] -> ByValue - --> $DIR/capture-enums.rs:55:47 + --> $DIR/capture-enums.rs:57:47 | LL | let SingleVariant::Point(_, _, str) = point; | ^^^^^ diff --git a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs index 40330af4088c2..a9d2777d93f13 100644 --- a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs +++ b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs @@ -14,6 +14,7 @@ fn test_1_should_capture() { //~| ERROR Min Capture analysis includes: match variant { //~^ NOTE: Capturing variant[] -> Immutable + //~| NOTE: Capturing variant[] -> Immutable //~| NOTE: Min Capture variant[] -> Immutable Some(_) => {} _ => {} @@ -132,6 +133,7 @@ fn test_5_should_capture_multi_variant() { //~| ERROR Min Capture analysis includes: match variant { //~^ NOTE: Capturing variant[] -> Immutable + //~| NOTE: Capturing variant[] -> Immutable //~| NOTE: Min Capture variant[] -> Immutable MVariant::A => {} _ => {} @@ -150,6 +152,7 @@ fn test_7_should_capture_slice_len() { //~| ERROR Min Capture analysis includes: match slice { //~^ NOTE: Capturing slice[] -> Immutable + //~| NOTE: Capturing slice[Deref] -> Immutable //~| NOTE: Min Capture slice[] -> Immutable [_,_,_] => {}, _ => {} @@ -162,6 +165,7 @@ fn test_7_should_capture_slice_len() { //~| ERROR Min Capture analysis includes: match slice { //~^ NOTE: Capturing slice[] -> Immutable + //~| NOTE: Capturing slice[Deref] -> Immutable //~| NOTE: Min Capture slice[] -> Immutable [] => {}, _ => {} @@ -174,6 +178,7 @@ fn test_7_should_capture_slice_len() { //~| ERROR Min Capture analysis includes: match slice { //~^ NOTE: Capturing slice[] -> Immutable + //~| NOTE: Capturing slice[Deref] -> Immutable //~| NOTE: Min Capture slice[] -> Immutable [_, .. ,_] => {}, _ => {} diff --git a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr index e7e5e7f7fa1bf..4b9d6fad0e504 100644 --- a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr +++ b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr @@ -14,6 +14,11 @@ note: Capturing variant[] -> Immutable | LL | match variant { | ^^^^^^^ +note: Capturing variant[] -> Immutable + --> $DIR/patterns-capture-analysis.rs:15:15 + | +LL | match variant { + | ^^^^^^^ error: Min Capture analysis includes: --> $DIR/patterns-capture-analysis.rs:12:5 @@ -33,7 +38,7 @@ LL | match variant { | ^^^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:30:5 + --> $DIR/patterns-capture-analysis.rs:31:5 | LL | / || { LL | | @@ -44,7 +49,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:49:5 + --> $DIR/patterns-capture-analysis.rs:50:5 | LL | / || { LL | | @@ -55,7 +60,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:63:5 + --> $DIR/patterns-capture-analysis.rs:64:5 | LL | / || { LL | | @@ -66,18 +71,18 @@ LL | | }; | |_____^ | note: Capturing variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:66:15 + --> $DIR/patterns-capture-analysis.rs:67:15 | LL | match variant { | ^^^^^^^ note: Capturing variant[(0, 0)] -> Immutable - --> $DIR/patterns-capture-analysis.rs:66:15 + --> $DIR/patterns-capture-analysis.rs:67:15 | LL | match variant { | ^^^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:63:5 + --> $DIR/patterns-capture-analysis.rs:64:5 | LL | / || { LL | | @@ -88,13 +93,13 @@ LL | | }; | |_____^ | note: Min Capture variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:66:15 + --> $DIR/patterns-capture-analysis.rs:67:15 | LL | match variant { | ^^^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:83:5 + --> $DIR/patterns-capture-analysis.rs:84:5 | LL | / || { LL | | @@ -105,7 +110,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:95:5 + --> $DIR/patterns-capture-analysis.rs:96:5 | LL | / || { LL | | @@ -116,7 +121,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:108:5 + --> $DIR/patterns-capture-analysis.rs:109:5 | LL | / || { LL | | @@ -127,7 +132,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:130:5 + --> $DIR/patterns-capture-analysis.rs:131:5 | LL | / || { LL | | @@ -138,13 +143,18 @@ LL | | }; | |_____^ | note: Capturing variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:133:15 + --> $DIR/patterns-capture-analysis.rs:134:15 + | +LL | match variant { + | ^^^^^^^ +note: Capturing variant[] -> Immutable + --> $DIR/patterns-capture-analysis.rs:134:15 | LL | match variant { | ^^^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:130:5 + --> $DIR/patterns-capture-analysis.rs:131:5 | LL | / || { LL | | @@ -155,13 +165,13 @@ LL | | }; | |_____^ | note: Min Capture variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:133:15 + --> $DIR/patterns-capture-analysis.rs:134:15 | LL | match variant { | ^^^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:148:5 + --> $DIR/patterns-capture-analysis.rs:150:5 | LL | / || { LL | | @@ -172,13 +182,18 @@ LL | | }; | |_____^ | note: Capturing slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:151:15 + --> $DIR/patterns-capture-analysis.rs:153:15 + | +LL | match slice { + | ^^^^^ +note: Capturing slice[Deref] -> Immutable + --> $DIR/patterns-capture-analysis.rs:153:15 | LL | match slice { | ^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:148:5 + --> $DIR/patterns-capture-analysis.rs:150:5 | LL | / || { LL | | @@ -189,13 +204,13 @@ LL | | }; | |_____^ | note: Min Capture slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:151:15 + --> $DIR/patterns-capture-analysis.rs:153:15 | LL | match slice { | ^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:160:5 + --> $DIR/patterns-capture-analysis.rs:163:5 | LL | / || { LL | | @@ -206,13 +221,18 @@ LL | | }; | |_____^ | note: Capturing slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:163:15 + --> $DIR/patterns-capture-analysis.rs:166:15 + | +LL | match slice { + | ^^^^^ +note: Capturing slice[Deref] -> Immutable + --> $DIR/patterns-capture-analysis.rs:166:15 | LL | match slice { | ^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:160:5 + --> $DIR/patterns-capture-analysis.rs:163:5 | LL | / || { LL | | @@ -223,13 +243,13 @@ LL | | }; | |_____^ | note: Min Capture slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:163:15 + --> $DIR/patterns-capture-analysis.rs:166:15 | LL | match slice { | ^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:172:5 + --> $DIR/patterns-capture-analysis.rs:176:5 | LL | / || { LL | | @@ -240,13 +260,18 @@ LL | | }; | |_____^ | note: Capturing slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:175:15 + --> $DIR/patterns-capture-analysis.rs:179:15 + | +LL | match slice { + | ^^^^^ +note: Capturing slice[Deref] -> Immutable + --> $DIR/patterns-capture-analysis.rs:179:15 | LL | match slice { | ^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:172:5 + --> $DIR/patterns-capture-analysis.rs:176:5 | LL | / || { LL | | @@ -257,13 +282,13 @@ LL | | }; | |_____^ | note: Min Capture slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:175:15 + --> $DIR/patterns-capture-analysis.rs:179:15 | LL | match slice { | ^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:189:5 + --> $DIR/patterns-capture-analysis.rs:194:5 | LL | / || { LL | | diff --git a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs new file mode 100644 index 0000000000000..a8e4b9cd18235 --- /dev/null +++ b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs @@ -0,0 +1,23 @@ +// This example used to compile, but the fact that it should was never properly +// discussed. With further experience, we concluded that capture precision +// depending on whether some types are inhabited goes too far, introducing a +// bunch of headaches without much benefit. +//@ edition:2021 +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum Void {} + +pub fn main() { + let mut r = Result::::Err((0, 0)); + let mut f = || { + let Err((ref mut a, _)) = r; + *a = 1; + }; + let mut g = || { + //~^ ERROR: cannot borrow `r` as mutable more than once at a time + let Err((_, ref mut b)) = r; + *b = 2; + }; + f(); + g(); + assert_eq!(r, Err((1, 2))); +} diff --git a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr new file mode 100644 index 0000000000000..dea85ff947ced --- /dev/null +++ b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr @@ -0,0 +1,20 @@ +error[E0499]: cannot borrow `r` as mutable more than once at a time + --> $DIR/only-inhabited-variant-stable.rs:15:17 + | +LL | let mut f = || { + | -- first mutable borrow occurs here +LL | let Err((ref mut a, _)) = r; + | - first borrow occurs due to use of `r` in closure +... +LL | let mut g = || { + | ^^ second mutable borrow occurs here +LL | +LL | let Err((_, ref mut b)) = r; + | - second borrow occurs due to use of `r` in closure +... +LL | f(); + | - first borrow later used here + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0499`. diff --git a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.exhaustive_patterns.stderr b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.exhaustive_patterns.stderr new file mode 100644 index 0000000000000..58a5348aa391a --- /dev/null +++ b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.exhaustive_patterns.stderr @@ -0,0 +1,20 @@ +error[E0499]: cannot borrow `r` as mutable more than once at a time + --> $DIR/only-inhabited-variant.rs:16:17 + | +LL | let mut f = || { + | -- first mutable borrow occurs here +LL | let Err((ref mut a, _)) = r; + | - first borrow occurs due to use of `r` in closure +... +LL | let mut g = || { + | ^^ second mutable borrow occurs here +LL | +LL | let Err((_, ref mut b)) = r; + | - second borrow occurs due to use of `r` in closure +... +LL | f(); + | - first borrow later used here + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0499`. diff --git a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.normal.stderr b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.normal.stderr new file mode 100644 index 0000000000000..58a5348aa391a --- /dev/null +++ b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.normal.stderr @@ -0,0 +1,20 @@ +error[E0499]: cannot borrow `r` as mutable more than once at a time + --> $DIR/only-inhabited-variant.rs:16:17 + | +LL | let mut f = || { + | -- first mutable borrow occurs here +LL | let Err((ref mut a, _)) = r; + | - first borrow occurs due to use of `r` in closure +... +LL | let mut g = || { + | ^^ second mutable borrow occurs here +LL | +LL | let Err((_, ref mut b)) = r; + | - second borrow occurs due to use of `r` in closure +... +LL | f(); + | - first borrow later used here + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0499`. diff --git a/tests/ui/closures/2229_closure_analysis/run_pass/multivariant.rs b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.rs similarity index 55% rename from tests/ui/closures/2229_closure_analysis/run_pass/multivariant.rs rename to tests/ui/closures/2229_closure_analysis/only-inhabited-variant.rs index 74f37b514e4a4..4638387347269 100644 --- a/tests/ui/closures/2229_closure_analysis/run_pass/multivariant.rs +++ b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant.rs @@ -1,8 +1,9 @@ -// Test precise capture of a multi-variant enum (when remaining variants are -// visibly uninhabited). +// This example used to compile, but the fact that it should was never properly +// discussed. With further experience, we concluded that capture precision +// depending on whether some types are inhabited goes too far, introducing a +// bunch of headaches without much benefit. //@ revisions: normal exhaustive_patterns //@ edition:2021 -//@ run-pass #![cfg_attr(exhaustive_patterns, feature(exhaustive_patterns))] #![feature(never_type)] @@ -13,6 +14,7 @@ pub fn main() { *a = 1; }; let mut g = || { + //~^ ERROR: cannot borrow `r` as mutable more than once at a time let Err((_, ref mut b)) = r; *b = 2; }; diff --git a/tests/ui/closures/or-patterns-issue-137467.rs b/tests/ui/closures/or-patterns-issue-137467.rs new file mode 100644 index 0000000000000..38144aa1e8162 --- /dev/null +++ b/tests/ui/closures/or-patterns-issue-137467.rs @@ -0,0 +1,170 @@ +//@ edition:2024 +//@ check-pass + +const X: u32 = 0; + +fn match_literal(x: (u32, u32, u32)) { + let _ = || { + let ((0, a, _) | (_, _, a)) = x; + a + }; +} + +fn match_range(x: (u32, u32, u32)) { + let _ = || { + let ((0..5, a, _) | (_, _, a)) = x; + a + }; +} + +fn match_const(x: (u32, u32, u32)) { + let _ = || { + let ((X, a, _) | (_, _, a)) = x; + a + }; +} + +enum Choice { A, B } + +fn match_unit_variant(x: (Choice, u32, u32)) { + let _ = || { + let ((Choice::A, a, _) | (Choice::B, _, a)) = x; + a + }; +} + +struct Unit; + +fn match_unit_struct(mut x: (Unit, u32)) { + let r = &mut x.0; + let _ = || { + let (Unit, a) = x; + a + }; + + let _ = *r; +} + +enum Also { Unit } + +fn match_unit_enum(mut x: (Also, u32)) { + let r = &mut x.0; + let _ = || { + let (Also::Unit, a) = x; + a + }; + + let _ = *r; +} + +enum TEnum { + A(u32), + B(u32), +} + +enum SEnum { + A { a: u32 }, + B { a: u32 }, +} + +fn match_tuple_enum(x: TEnum) { + let _ = || { + let (TEnum::A(a) | TEnum::B(a)) = x; + a + }; +} + +fn match_struct_enum(x: SEnum) { + let _ = || { + let (SEnum::A { a } | SEnum::B { a }) = x; + a + }; +} + +enum TSingle { + A(u32, u32), +} + +enum SSingle { + A { a: u32, b: u32 }, +} + +struct TStruct(u32, u32); +struct SStruct { a: u32, b: u32 } + +fn match_struct(mut x: SStruct) { + let r = &mut x.a; + let _ = || { + let SStruct { b, .. } = x; + b + }; + + let _ = *r; +} + +fn match_tuple_struct(mut x: TStruct) { + let r = &mut x.0; + let _ = || { + let TStruct(_, a) = x; + a + }; + + let _ = *r; +} + +fn match_singleton(mut x: SSingle) { + let SSingle::A { a: ref mut r, .. } = x; + let _ = || { + let SSingle::A { b, .. } = x; + b + }; + + let _ = *r; +} + +fn match_tuple_singleton(mut x: TSingle) { + let TSingle::A(ref mut r, _) = x; + let _ = || { + let TSingle::A(_, a) = x; + a + }; + + let _ = *r; +} + +fn match_slice(x: (&[u32], u32, u32)) { + let _ = || { + let (([], a, _) | ([_, ..], _, a)) = x; + a + }; +} + +// Original testcase, for completeness +enum Camera { + Normal { base_transform: i32 }, + Volume { transform: i32 }, +} + +fn draw_ui(camera: &mut Camera) { + || { + let (Camera::Normal { + base_transform: _transform, + } + | Camera::Volume { + transform: _transform, + }) = camera; + }; +} + +fn draw_ui2(camera: &mut Camera) { + || { + let (Camera::Normal { + base_transform: _, + } + | Camera::Volume { + transform: _, + }) = camera; + }; +} + +fn main() {} From 647e8bde17ab11d93559cbb35a86b89f0f32216f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maja=20K=C4=85dzio=C5=82ka?= Date: Wed, 26 Mar 2025 20:59:50 +0100 Subject: [PATCH 2/4] ExprUseVisitor: remove maybe_read_scrutinee The split between walk_pat and maybe_read_scrutinee has now become redundant. Due to this change, one testcase within the testsuite has become similar enough to a known ICE to also break. I am leaving this as future work, as it requires feature(type_alias_impl_trait) --- .../rustc_hir_typeck/src/expr_use_visitor.rs | 171 +++--------------- tests/crashes/{119786.rs => 119786-1.rs} | 0 tests/crashes/119786-2.rs | 15 ++ tests/crashes/119786-3.rs | 15 ++ ...oo-{closure#0}-{closure#0}.built.after.mir | 29 ++- ...-{closure#0}-{synthetic#0}.built.after.mir | 24 +-- .../match/match-edge-cases_2.stderr | 2 +- .../match/patterns-capture-analysis.rs | 22 +-- .../match/patterns-capture-analysis.stderr | 90 +++------ .../at-pattern-weirdness-issue-137553.rs | 41 +++++ .../issue-96572-unconstrained.rs | 9 - 11 files changed, 155 insertions(+), 263 deletions(-) rename tests/crashes/{119786.rs => 119786-1.rs} (100%) create mode 100644 tests/crashes/119786-2.rs create mode 100644 tests/crashes/119786-3.rs create mode 100644 tests/ui/closures/at-pattern-weirdness-issue-137553.rs diff --git a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs index 7999b0f05e88e..cfac3e08e5788 100644 --- a/compiler/rustc_hir_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_hir_typeck/src/expr_use_visitor.rs @@ -7,9 +7,7 @@ use std::cell::{Ref, RefCell}; use std::ops::Deref; -use std::slice::from_ref; -use hir::Expr; use hir::def::DefKind; use hir::pat_util::EnumerateAndAdjustIterator as _; use rustc_abi::{FIRST_VARIANT, FieldIdx, VariantIdx}; @@ -312,7 +310,8 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx let param_place = self.cat_rvalue(param.hir_id, param_ty); - self.walk_irrefutable_pat(¶m_place, param.pat)?; + self.fake_read_scrutinee(¶m_place, false)?; + self.walk_pat(¶m_place, param.pat, false)?; } self.consume_expr(body.value)?; @@ -454,13 +453,9 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx hir::ExprKind::Match(discr, arms, _) => { let discr_place = self.cat_expr(discr)?; - self.maybe_read_scrutinee( - discr, - discr_place.clone(), - arms.iter().map(|arm| arm.pat), - )?; + self.fake_read_scrutinee(&discr_place, true)?; + self.walk_expr(discr)?; - // treatment of the discriminant is handled while walking the arms. for arm in arms { self.walk_arm(&discr_place, arm)?; } @@ -597,116 +592,25 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx Ok(()) } - fn maybe_read_scrutinee<'t>( + #[instrument(skip(self), level = "debug")] + fn fake_read_scrutinee( &self, - discr: &Expr<'_>, - discr_place: PlaceWithHirId<'tcx>, - pats: impl Iterator>, + discr_place: &PlaceWithHirId<'tcx>, + refutable: bool, ) -> Result<(), Cx::Error> { - // Matching should not always be considered a use of the place, hence - // discr does not necessarily need to be borrowed. - // We only want to borrow discr if the pattern contain something other - // than wildcards. - let mut needs_to_be_read = false; - for pat in pats { - self.cat_pattern(discr_place.clone(), pat, &mut |place, pat| { - match &pat.kind { - PatKind::Missing => unreachable!(), - PatKind::Binding(.., opt_sub_pat) => { - // If the opt_sub_pat is None, then the binding does not count as - // a wildcard for the purpose of borrowing discr. - if opt_sub_pat.is_none() { - needs_to_be_read = true; - } - } - PatKind::Never => { - // A never pattern reads the value. - // FIXME(never_patterns): does this do what I expect? - needs_to_be_read = true; - } - PatKind::Expr(PatExpr { kind: PatExprKind::Path(qpath), hir_id, span }) => { - // A `Path` pattern is just a name like `Foo`. This is either a - // named constant or else it refers to an ADT variant - - let res = self.cx.typeck_results().qpath_res(qpath, *hir_id); - match res { - Res::Def(DefKind::Const, _) | Res::Def(DefKind::AssocConst, _) => { - // Named constants have to be equated with the value - // being matched, so that's a read of the value being matched. - // - // FIXME: We don't actually reads for ZSTs. - needs_to_be_read = true; - } - _ => { - // Otherwise, this is a struct/enum variant, and so it's - // only a read if we need to read the discriminant. - needs_to_be_read |= - self.is_multivariant_adt(place.place.ty(), *span); - } - } - } - PatKind::TupleStruct(..) | PatKind::Struct(..) | PatKind::Tuple(..) => { - // For `Foo(..)`, `Foo { ... }` and `(...)` patterns, check if we are matching - // against a multivariant enum or struct. In that case, we have to read - // the discriminant. Otherwise this kind of pattern doesn't actually - // read anything (we'll get invoked for the `...`, which may indeed - // perform some reads). - - let place_ty = place.place.ty(); - needs_to_be_read |= self.is_multivariant_adt(place_ty, pat.span); - } - PatKind::Expr(_) | PatKind::Range(..) => { - // If the PatKind is a Lit or a Range then we want - // to borrow discr. - needs_to_be_read = true; - } - PatKind::Slice(lhs, wild, rhs) => { - // We don't need to test the length if the pattern is `[..]` - if matches!((lhs, wild, rhs), (&[], Some(_), &[])) - // Arrays have a statically known size, so - // there is no need to read their length - || place.place.ty().peel_refs().is_array() - { - } else { - needs_to_be_read = true; - } - } - PatKind::Or(_) - | PatKind::Box(_) - | PatKind::Deref(_) - | PatKind::Ref(..) - | PatKind::Guard(..) - | PatKind::Wild - | PatKind::Err(_) => { - // If the PatKind is Or, Box, or Ref, the decision is made later - // as these patterns contains subpatterns - // If the PatKind is Wild or Err, the decision is made based on the other patterns - // being examined - } - } - - Ok(()) - })? - } + let closure_def_id = match discr_place.place.base { + PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id), + _ => None, + }; - if needs_to_be_read { - self.borrow_expr(discr, BorrowKind::Immutable)?; + let cause = if refutable { + FakeReadCause::ForMatchedPlace(closure_def_id) } else { - let closure_def_id = match discr_place.place.base { - PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id), - _ => None, - }; + FakeReadCause::ForLet(closure_def_id) + }; - self.delegate.borrow_mut().fake_read( - &discr_place, - FakeReadCause::ForMatchedPlace(closure_def_id), - discr_place.hir_id, - ); + self.delegate.borrow_mut().fake_read(discr_place, cause, discr_place.hir_id); - // We always want to walk the discriminant. We want to make sure, for instance, - // that the discriminant has been initialized. - self.walk_expr(discr)?; - } Ok(()) } @@ -723,12 +627,11 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx self.walk_expr(expr)?; let expr_place = self.cat_expr(expr)?; f()?; + self.fake_read_scrutinee(&expr_place, els.is_some())?; + self.walk_pat(&expr_place, pat, false)?; if let Some(els) = els { - // borrowing because we need to test the discriminant - self.maybe_read_scrutinee(expr, expr_place.clone(), from_ref(pat).iter())?; self.walk_block(els)?; } - self.walk_irrefutable_pat(&expr_place, pat)?; Ok(()) } @@ -900,16 +803,6 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx discr_place: &PlaceWithHirId<'tcx>, arm: &hir::Arm<'_>, ) -> Result<(), Cx::Error> { - let closure_def_id = match discr_place.place.base { - PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id), - _ => None, - }; - - self.delegate.borrow_mut().fake_read( - discr_place, - FakeReadCause::ForMatchedPlace(closure_def_id), - discr_place.hir_id, - ); self.walk_pat(discr_place, arm.pat, arm.guard.is_some())?; if let Some(ref e) = arm.guard { @@ -920,27 +813,6 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx Ok(()) } - /// Walks a pat that occurs in isolation (i.e., top-level of fn argument or - /// let binding, and *not* a match arm or nested pat.) - fn walk_irrefutable_pat( - &self, - discr_place: &PlaceWithHirId<'tcx>, - pat: &hir::Pat<'_>, - ) -> Result<(), Cx::Error> { - let closure_def_id = match discr_place.place.base { - PlaceBase::Upvar(upvar_id) => Some(upvar_id.closure_expr_id), - _ => None, - }; - - self.delegate.borrow_mut().fake_read( - discr_place, - FakeReadCause::ForLet(closure_def_id), - discr_place.hir_id, - ); - self.walk_pat(discr_place, pat, false)?; - Ok(()) - } - /// The core driver for walking a pattern /// /// This should mirror how pattern-matching gets lowered to MIR, as @@ -1960,8 +1832,11 @@ impl<'tcx, Cx: TypeInformationCtxt<'tcx>, D: Delegate<'tcx>> ExprUseVisitor<'tcx /// Here, we cannot perform such an accurate checks, because querying /// whether a type is inhabited requires that it has been fully inferred, /// which cannot be guaranteed at this point. + #[instrument(skip(self, span), level = "debug")] fn is_multivariant_adt(&self, ty: Ty<'tcx>, span: Span) -> bool { - if let ty::Adt(def, _) = self.cx.try_structurally_resolve_type(span, ty).kind() { + let ty = self.cx.try_structurally_resolve_type(span, ty); + debug!(?ty, "is_multivariant_adt: resolved"); + if let ty::Adt(def, _) = ty.kind() { // Note that if a non-exhaustive SingleVariant is defined in another crate, we need // to assume that more cases will be added to the variant in the future. This mean // that we should handle non-exhaustive SingleVariant the same way we would handle diff --git a/tests/crashes/119786.rs b/tests/crashes/119786-1.rs similarity index 100% rename from tests/crashes/119786.rs rename to tests/crashes/119786-1.rs diff --git a/tests/crashes/119786-2.rs b/tests/crashes/119786-2.rs new file mode 100644 index 0000000000000..76c5deb4605ae --- /dev/null +++ b/tests/crashes/119786-2.rs @@ -0,0 +1,15 @@ +//@ known-bug: #119786 +//@ edition:2021 + +fn enum_upvar() { + type T = impl Copy; + let foo: T = Some((1u32, 2u32)); + let x = move || { + match foo { + None => (), + Some(_) => (), + } + }; +} + +pub fn main() {} diff --git a/tests/crashes/119786-3.rs b/tests/crashes/119786-3.rs new file mode 100644 index 0000000000000..34bb90fd0fae3 --- /dev/null +++ b/tests/crashes/119786-3.rs @@ -0,0 +1,15 @@ +//@ known-bug: #119786 +//@ edition:2021 + +fn enum_upvar() { + type T = impl Copy; + let foo: T = Some((1u32, 2u32)); + let x = move || { + match foo { + None => (), + Some((a, b)) => (), + } + }; +} + +pub fn main() {} diff --git a/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#0}.built.after.mir b/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#0}.built.after.mir index 0c8a17ff70b2e..82950d621e202 100644 --- a/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#0}.built.after.mir +++ b/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#0}.built.after.mir @@ -4,18 +4,16 @@ fn foo::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_fake yields () { debug _task_context => _2; - debug f => (*(_1.0: &&Foo)); + debug f => (*(_1.0: &Foo)); let mut _0: (); let mut _3: &Foo; let mut _4: &&Foo; - let mut _5: &&&Foo; - let mut _6: isize; - let mut _7: bool; + let mut _5: isize; + let mut _6: bool; bb0: { - PlaceMention((*(_1.0: &&Foo))); - _6 = discriminant((*(*(_1.0: &&Foo)))); - switchInt(move _6) -> [0: bb2, otherwise: bb1]; + _5 = discriminant((*(_1.0: &Foo))); + switchInt(move _5) -> [0: bb2, otherwise: bb1]; } bb1: { @@ -32,17 +30,15 @@ yields () } bb4: { - FakeRead(ForMatchedPlace(None), (*(_1.0: &&Foo))); unreachable; } bb5: { - _3 = &fake shallow (*(*(_1.0: &&Foo))); - _4 = &fake shallow (*(_1.0: &&Foo)); - _5 = &fake shallow (_1.0: &&Foo); - StorageLive(_7); - _7 = const true; - switchInt(move _7) -> [0: bb8, otherwise: bb7]; + _3 = &fake shallow (*(_1.0: &Foo)); + _4 = &fake shallow (_1.0: &Foo); + StorageLive(_6); + _6 = const true; + switchInt(move _6) -> [0: bb8, otherwise: bb7]; } bb6: { @@ -50,10 +46,9 @@ yields () } bb7: { - StorageDead(_7); + StorageDead(_6); FakeRead(ForMatchGuard, _3); FakeRead(ForMatchGuard, _4); - FakeRead(ForMatchGuard, _5); _0 = const (); goto -> bb10; } @@ -63,7 +58,7 @@ yields () } bb9: { - StorageDead(_7); + StorageDead(_6); goto -> bb6; } diff --git a/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{synthetic#0}.built.after.mir b/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{synthetic#0}.built.after.mir index 9070c95bca4d4..2bbdb514d46ad 100644 --- a/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{synthetic#0}.built.after.mir +++ b/tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{synthetic#0}.built.after.mir @@ -4,18 +4,16 @@ fn foo::{closure#0}::{synthetic#0}(_1: {async closure body@$DIR/async_closure_fa yields () { debug _task_context => _2; - debug f => (_1.0: &Foo); + debug f => (*(_1.0: &Foo)); let mut _0: (); let mut _3: &Foo; let mut _4: &&Foo; - let mut _5: &&&Foo; - let mut _6: isize; - let mut _7: bool; + let mut _5: isize; + let mut _6: bool; bb0: { - PlaceMention((_1.0: &Foo)); - _6 = discriminant((*(_1.0: &Foo))); - switchInt(move _6) -> [0: bb2, otherwise: bb1]; + _5 = discriminant((*(_1.0: &Foo))); + switchInt(move _5) -> [0: bb2, otherwise: bb1]; } bb1: { @@ -29,24 +27,22 @@ yields () bb3: { _3 = &fake shallow (*(_1.0: &Foo)); - _4 = &fake shallow (_1.0: &Foo); nop; - StorageLive(_7); - _7 = const true; - switchInt(move _7) -> [0: bb5, otherwise: bb4]; + StorageLive(_6); + _6 = const true; + switchInt(move _6) -> [0: bb5, otherwise: bb4]; } bb4: { - StorageDead(_7); + StorageDead(_6); FakeRead(ForMatchGuard, _3); FakeRead(ForMatchGuard, _4); - FakeRead(ForMatchGuard, _5); _0 = const (); goto -> bb6; } bb5: { - StorageDead(_7); + StorageDead(_6); falseEdge -> [real: bb1, imaginary: bb1]; } diff --git a/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_2.stderr b/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_2.stderr index d82db0481a06f..3f5fe9eda423f 100644 --- a/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_2.stderr +++ b/tests/ui/closures/2229_closure_analysis/match/match-edge-cases_2.stderr @@ -4,7 +4,7 @@ error[E0505]: cannot move out of `ts` because it is borrowed LL | let _b = || { match ts { | -- -- borrow occurs due to use in closure | | - | borrow of `ts` occurs here + | borrow of `ts.x` occurs here ... LL | let mut mut_ts = ts; | ^^ move out of `ts` occurs here diff --git a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs index a9d2777d93f13..16cb9d7355da5 100644 --- a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs +++ b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.rs @@ -14,7 +14,6 @@ fn test_1_should_capture() { //~| ERROR Min Capture analysis includes: match variant { //~^ NOTE: Capturing variant[] -> Immutable - //~| NOTE: Capturing variant[] -> Immutable //~| NOTE: Min Capture variant[] -> Immutable Some(_) => {} _ => {} @@ -65,9 +64,8 @@ fn test_6_should_capture_single_variant() { //~^ ERROR First Pass analysis includes: //~| ERROR Min Capture analysis includes: match variant { - //~^ NOTE: Capturing variant[] -> Immutable - //~| NOTE: Capturing variant[(0, 0)] -> Immutable - //~| NOTE: Min Capture variant[] -> Immutable + //~^ NOTE: Capturing variant[(0, 0)] -> Immutable + //~| NOTE: Min Capture variant[(0, 0)] -> Immutable SingleVariant::Points(a) => { println!("{:?}", a); } @@ -133,7 +131,6 @@ fn test_5_should_capture_multi_variant() { //~| ERROR Min Capture analysis includes: match variant { //~^ NOTE: Capturing variant[] -> Immutable - //~| NOTE: Capturing variant[] -> Immutable //~| NOTE: Min Capture variant[] -> Immutable MVariant::A => {} _ => {} @@ -151,9 +148,8 @@ fn test_7_should_capture_slice_len() { //~^ ERROR First Pass analysis includes: //~| ERROR Min Capture analysis includes: match slice { - //~^ NOTE: Capturing slice[] -> Immutable - //~| NOTE: Capturing slice[Deref] -> Immutable - //~| NOTE: Min Capture slice[] -> Immutable + //~^ NOTE: Capturing slice[Deref] -> Immutable + //~| NOTE: Min Capture slice[Deref] -> Immutable [_,_,_] => {}, _ => {} } @@ -164,9 +160,8 @@ fn test_7_should_capture_slice_len() { //~^ ERROR First Pass analysis includes: //~| ERROR Min Capture analysis includes: match slice { - //~^ NOTE: Capturing slice[] -> Immutable - //~| NOTE: Capturing slice[Deref] -> Immutable - //~| NOTE: Min Capture slice[] -> Immutable + //~^ NOTE: Capturing slice[Deref] -> Immutable + //~| NOTE: Min Capture slice[Deref] -> Immutable [] => {}, _ => {} } @@ -177,9 +172,8 @@ fn test_7_should_capture_slice_len() { //~^ ERROR First Pass analysis includes: //~| ERROR Min Capture analysis includes: match slice { - //~^ NOTE: Capturing slice[] -> Immutable - //~| NOTE: Capturing slice[Deref] -> Immutable - //~| NOTE: Min Capture slice[] -> Immutable + //~^ NOTE: Capturing slice[Deref] -> Immutable + //~| NOTE: Min Capture slice[Deref] -> Immutable [_, .. ,_] => {}, _ => {} } diff --git a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr index 4b9d6fad0e504..73c685e152765 100644 --- a/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr +++ b/tests/ui/closures/2229_closure_analysis/match/patterns-capture-analysis.stderr @@ -14,11 +14,6 @@ note: Capturing variant[] -> Immutable | LL | match variant { | ^^^^^^^ -note: Capturing variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:15:15 - | -LL | match variant { - | ^^^^^^^ error: Min Capture analysis includes: --> $DIR/patterns-capture-analysis.rs:12:5 @@ -38,7 +33,7 @@ LL | match variant { | ^^^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:31:5 + --> $DIR/patterns-capture-analysis.rs:30:5 | LL | / || { LL | | @@ -49,7 +44,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:50:5 + --> $DIR/patterns-capture-analysis.rs:49:5 | LL | / || { LL | | @@ -60,7 +55,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:64:5 + --> $DIR/patterns-capture-analysis.rs:63:5 | LL | / || { LL | | @@ -70,19 +65,14 @@ LL | | match variant { LL | | }; | |_____^ | -note: Capturing variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:67:15 - | -LL | match variant { - | ^^^^^^^ note: Capturing variant[(0, 0)] -> Immutable - --> $DIR/patterns-capture-analysis.rs:67:15 + --> $DIR/patterns-capture-analysis.rs:66:15 | LL | match variant { | ^^^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:64:5 + --> $DIR/patterns-capture-analysis.rs:63:5 | LL | / || { LL | | @@ -92,14 +82,14 @@ LL | | match variant { LL | | }; | |_____^ | -note: Min Capture variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:67:15 +note: Min Capture variant[(0, 0)] -> Immutable + --> $DIR/patterns-capture-analysis.rs:66:15 | LL | match variant { | ^^^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:84:5 + --> $DIR/patterns-capture-analysis.rs:82:5 | LL | / || { LL | | @@ -110,7 +100,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:96:5 + --> $DIR/patterns-capture-analysis.rs:94:5 | LL | / || { LL | | @@ -121,7 +111,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:109:5 + --> $DIR/patterns-capture-analysis.rs:107:5 | LL | / || { LL | | @@ -132,7 +122,7 @@ LL | | }; | |_____^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:131:5 + --> $DIR/patterns-capture-analysis.rs:129:5 | LL | / || { LL | | @@ -143,18 +133,13 @@ LL | | }; | |_____^ | note: Capturing variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:134:15 - | -LL | match variant { - | ^^^^^^^ -note: Capturing variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:134:15 + --> $DIR/patterns-capture-analysis.rs:132:15 | LL | match variant { | ^^^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:131:5 + --> $DIR/patterns-capture-analysis.rs:129:5 | LL | / || { LL | | @@ -165,13 +150,13 @@ LL | | }; | |_____^ | note: Min Capture variant[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:134:15 + --> $DIR/patterns-capture-analysis.rs:132:15 | LL | match variant { | ^^^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:150:5 + --> $DIR/patterns-capture-analysis.rs:147:5 | LL | / || { LL | | @@ -181,19 +166,14 @@ LL | | match slice { LL | | }; | |_____^ | -note: Capturing slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:153:15 - | -LL | match slice { - | ^^^^^ note: Capturing slice[Deref] -> Immutable - --> $DIR/patterns-capture-analysis.rs:153:15 + --> $DIR/patterns-capture-analysis.rs:150:15 | LL | match slice { | ^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:150:5 + --> $DIR/patterns-capture-analysis.rs:147:5 | LL | / || { LL | | @@ -203,14 +183,14 @@ LL | | match slice { LL | | }; | |_____^ | -note: Min Capture slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:153:15 +note: Min Capture slice[Deref] -> Immutable + --> $DIR/patterns-capture-analysis.rs:150:15 | LL | match slice { | ^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:163:5 + --> $DIR/patterns-capture-analysis.rs:159:5 | LL | / || { LL | | @@ -220,19 +200,14 @@ LL | | match slice { LL | | }; | |_____^ | -note: Capturing slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:166:15 - | -LL | match slice { - | ^^^^^ note: Capturing slice[Deref] -> Immutable - --> $DIR/patterns-capture-analysis.rs:166:15 + --> $DIR/patterns-capture-analysis.rs:162:15 | LL | match slice { | ^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:163:5 + --> $DIR/patterns-capture-analysis.rs:159:5 | LL | / || { LL | | @@ -242,14 +217,14 @@ LL | | match slice { LL | | }; | |_____^ | -note: Min Capture slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:166:15 +note: Min Capture slice[Deref] -> Immutable + --> $DIR/patterns-capture-analysis.rs:162:15 | LL | match slice { | ^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:176:5 + --> $DIR/patterns-capture-analysis.rs:171:5 | LL | / || { LL | | @@ -259,19 +234,14 @@ LL | | match slice { LL | | }; | |_____^ | -note: Capturing slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:179:15 - | -LL | match slice { - | ^^^^^ note: Capturing slice[Deref] -> Immutable - --> $DIR/patterns-capture-analysis.rs:179:15 + --> $DIR/patterns-capture-analysis.rs:174:15 | LL | match slice { | ^^^^^ error: Min Capture analysis includes: - --> $DIR/patterns-capture-analysis.rs:176:5 + --> $DIR/patterns-capture-analysis.rs:171:5 | LL | / || { LL | | @@ -281,14 +251,14 @@ LL | | match slice { LL | | }; | |_____^ | -note: Min Capture slice[] -> Immutable - --> $DIR/patterns-capture-analysis.rs:179:15 +note: Min Capture slice[Deref] -> Immutable + --> $DIR/patterns-capture-analysis.rs:174:15 | LL | match slice { | ^^^^^ error: First Pass analysis includes: - --> $DIR/patterns-capture-analysis.rs:194:5 + --> $DIR/patterns-capture-analysis.rs:188:5 | LL | / || { LL | | diff --git a/tests/ui/closures/at-pattern-weirdness-issue-137553.rs b/tests/ui/closures/at-pattern-weirdness-issue-137553.rs new file mode 100644 index 0000000000000..7c934d4a14339 --- /dev/null +++ b/tests/ui/closures/at-pattern-weirdness-issue-137553.rs @@ -0,0 +1,41 @@ +//@ edition:2024 +//@ check-pass + +// Background: +fn f1() { + let mut a = (21, 37); + // only captures a.0, example compiles fine + let mut f = || { + let (ref mut x, _) = a; + *x = 42; + }; + a.1 = 69; + f(); +} + +// This used to error out: +fn f2() { + let mut a = (21, 37); + // used to capture all of a, now captures only a.0 + let mut f = || { + match a { + (ref mut x, _) => *x = 42, + } + }; + a.1 = 69; + f(); +} + +// This was inconsistent with the following: +fn main() { + let mut a = (21, 37); + // the useless @-pattern would cause it to capture only a.0. now the + // behavior is consistent with the case that doesn't use the @-pattern + let mut f = || { + match a { + (ref mut x @ _, _) => *x = 42, + } + }; + a.1 = 69; + f(); +} diff --git a/tests/ui/type-alias-impl-trait/issue-96572-unconstrained.rs b/tests/ui/type-alias-impl-trait/issue-96572-unconstrained.rs index 7f0f6a214aae9..383d9108eb908 100644 --- a/tests/ui/type-alias-impl-trait/issue-96572-unconstrained.rs +++ b/tests/ui/type-alias-impl-trait/issue-96572-unconstrained.rs @@ -23,15 +23,6 @@ fn upvar() { }; } -fn enum_upvar() { - type T = impl Copy; - let foo: T = Some((1u32, 2u32)); - let x = move || match foo { - None => (), - Some((a, b)) => (), - }; -} - fn r#struct() { #[derive(Copy, Clone)] struct Foo((u32, u32)); From f4fc4a72e48ab8b13690069b546874bfec8b3db1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maja=20K=C4=85dzio=C5=82ka?= Date: Thu, 27 Mar 2025 16:50:11 +0100 Subject: [PATCH 3/4] Add test case for issue #138973 --- tests/ui/closures/or-patterns-issue-137467.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/ui/closures/or-patterns-issue-137467.rs b/tests/ui/closures/or-patterns-issue-137467.rs index 38144aa1e8162..5a1e84e1c9a0d 100644 --- a/tests/ui/closures/or-patterns-issue-137467.rs +++ b/tests/ui/closures/or-patterns-issue-137467.rs @@ -24,6 +24,13 @@ fn match_const(x: (u32, u32, u32)) { }; } +// related testcase reported in #138973 +fn without_bindings(x: u32) { + let _ = || { + let (0 | _) = x; + }; +} + enum Choice { A, B } fn match_unit_variant(x: (Choice, u32, u32)) { From af3aabaafb869fce0e8fb0403b13e0f2ee8504f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maja=20K=C4=85dzio=C5=82ka?= Date: Tue, 1 Apr 2025 20:32:13 +0200 Subject: [PATCH 4/4] Avoid using #[derive] in test As per code review, it is preferred to not use derives in tests that aren't about them. --- .../2229_closure_analysis/only-inhabited-variant-stable.rs | 3 +-- .../2229_closure_analysis/only-inhabited-variant-stable.stderr | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs index a8e4b9cd18235..c7f367cc48ab9 100644 --- a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs +++ b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.rs @@ -3,7 +3,6 @@ // depending on whether some types are inhabited goes too far, introducing a // bunch of headaches without much benefit. //@ edition:2021 -#[derive(Clone, Copy, PartialEq, Eq, Debug)] enum Void {} pub fn main() { @@ -19,5 +18,5 @@ pub fn main() { }; f(); g(); - assert_eq!(r, Err((1, 2))); + assert!(matches!(r, Err((1, 2)))); } diff --git a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr index dea85ff947ced..7f4c8942b0d91 100644 --- a/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr +++ b/tests/ui/closures/2229_closure_analysis/only-inhabited-variant-stable.stderr @@ -1,5 +1,5 @@ error[E0499]: cannot borrow `r` as mutable more than once at a time - --> $DIR/only-inhabited-variant-stable.rs:15:17 + --> $DIR/only-inhabited-variant-stable.rs:14:17 | LL | let mut f = || { | -- first mutable borrow occurs here