From da43fb1caec88a69ab1c9745534753fbd04dfe67 Mon Sep 17 00:00:00 2001 From: jackh726 Date: Mon, 15 Dec 2025 17:03:52 +0000 Subject: [PATCH 1/3] Change branch to assert --- compiler/rustc_hir_typeck/src/coercion.rs | 36 +++++++++++------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 5e1e567d103ec..39f97eac24f24 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1287,29 +1287,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let mut coerce = Coerce::new(self, cause.clone(), AllowTwoPhase::No, true); coerce.use_lub = true; - // First try to coerce the new expression to the type of the previous ones, - // but only if the new expression has no coercion already applied to it. - let mut first_error = None; - if !self.typeck_results.borrow().adjustments().contains_key(new.hir_id) { - let result = self.commit_if_ok(|_| coerce.coerce(new_ty, prev_ty)); - match result { - Ok(ok) => { - let (adjustments, target) = self.register_infer_ok_obligations(ok); - self.apply_adjustments(new, adjustments); - debug!( - "coercion::try_find_coercion_lub: was able to coerce from new type {:?} to previous type {:?} ({:?})", - new_ty, prev_ty, target - ); - return Ok(target); - } - Err(e) => first_error = Some(e), + // This might be okay, but we previously branched on this without any + // test, so I'm just keeping the assert to avoid surprising behavior. + assert!(!self.typeck_results.borrow().adjustments().contains_key(new.hir_id)); + + // First try to coerce the new expression to the type of the previous ones. + let result = self.commit_if_ok(|_| coerce.coerce(new_ty, prev_ty)); + let first_error = match result { + Ok(ok) => { + let (adjustments, target) = self.register_infer_ok_obligations(ok); + self.apply_adjustments(new, adjustments); + debug!( + "coercion::try_find_coercion_lub: was able to coerce from new type {:?} to previous type {:?} ({:?})", + new_ty, prev_ty, target + ); + return Ok(target); } - } + Err(e) => e, + }; let ok = self .commit_if_ok(|_| coerce.coerce(prev_ty, new_ty)) // Avoid giving strange errors on failed attempts. - .map_err(|e| first_error.unwrap_or(e))?; + .map_err(|_| first_error)?; let (adjustments, target) = self.register_infer_ok_obligations(ok); for expr in exprs { From 68646ccf96e6fdb084ee62326806bda9df568824 Mon Sep 17 00:00:00 2001 From: jackh726 Date: Wed, 17 Dec 2025 15:48:58 +0000 Subject: [PATCH 2/3] Always use LUB for match arms --- compiler/rustc_hir_typeck/src/_match.rs | 1 + compiler/rustc_hir_typeck/src/coercion.rs | 16 ++++++++++-- ...atterns.opt1.SimplifyCfg-initial.after.mir | 26 +++++++++++-------- ...atterns.opt2.SimplifyCfg-initial.after.mir | 18 ++++++++----- ...atterns.opt3.SimplifyCfg-initial.after.mir | 22 +++++++++------- .../coercion/coerce-loop-issue-122561.stderr | 6 ----- tests/ui/issues/issue-28839.rs | 14 +++++++--- tests/ui/issues/issue-28839.stderr | 15 +++++++++++ tests/ui/nll/issue-52213.rs | 2 +- tests/ui/nll/issue-52213.stderr | 18 +++++++------ 10 files changed, 91 insertions(+), 47 deletions(-) create mode 100644 tests/ui/issues/issue-28839.stderr diff --git a/compiler/rustc_hir_typeck/src/_match.rs b/compiler/rustc_hir_typeck/src/_match.rs index ded03c88a16e9..e5335de4506e7 100644 --- a/compiler/rustc_hir_typeck/src/_match.rs +++ b/compiler/rustc_hir_typeck/src/_match.rs @@ -75,6 +75,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; CoerceMany::with_capacity(coerce_first, arms.len()) }; + coercion.force_lub(); let mut prior_non_diverging_arms = vec![]; // Used only for diagnostics. let mut prior_arm = None; diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 39f97eac24f24..946be5a5e6aed 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1382,6 +1382,7 @@ pub(crate) struct CoerceMany<'tcx> { expected_ty: Ty<'tcx>, final_ty: Option>, expressions: Vec<&'tcx hir::Expr<'tcx>>, + force_lub: bool, } impl<'tcx> CoerceMany<'tcx> { @@ -1394,7 +1395,18 @@ impl<'tcx> CoerceMany<'tcx> { /// Creates a `CoerceMany` with a given capacity. pub(crate) fn with_capacity(expected_ty: Ty<'tcx>, capacity: usize) -> Self { - CoerceMany { expected_ty, final_ty: None, expressions: Vec::with_capacity(capacity) } + CoerceMany { + expected_ty, + final_ty: None, + expressions: Vec::with_capacity(capacity), + force_lub: false, + } + } + + pub(crate) fn force_lub(&mut self) { + // Don't accidentally let someone switch this after coercing things + assert!(self.expressions.is_empty()); + self.force_lub = true; } /// Returns the "expected type" with which this coercion was @@ -1507,7 +1519,7 @@ impl<'tcx> CoerceMany<'tcx> { // Handle the actual type unification etc. let result = if let Some(expression) = expression { - if self.expressions.is_empty() { + if !self.force_lub && self.expressions.is_empty() { // Special-case the first expression we are coercing. // To be honest, I'm not entirely sure why we do this. // We don't allow two-phase borrows, see comment in try_find_coercion_lub for why diff --git a/tests/mir-opt/building/match/never_patterns.opt1.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/never_patterns.opt1.SimplifyCfg-initial.after.mir index bba4d9c0149a1..78bd238689cc6 100644 --- a/tests/mir-opt/building/match/never_patterns.opt1.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/never_patterns.opt1.SimplifyCfg-initial.after.mir @@ -3,22 +3,24 @@ fn opt1(_1: &Result) -> &u32 { debug res => _1; let mut _0: &u32; - let mut _2: isize; - let _3: &u32; - let mut _4: !; - let mut _5: (); + let _2: &u32; + let mut _3: isize; + let _4: &u32; + let mut _5: !; + let mut _6: (); scope 1 { - debug x => _3; + debug x => _4; } bb0: { + StorageLive(_2); PlaceMention(_1); falseEdge -> [real: bb4, imaginary: bb1]; } bb1: { - _2 = discriminant((*_1)); - switchInt(move _2) -> [1: bb3, otherwise: bb2]; + _3 = discriminant((*_1)); + switchInt(move _3) -> [1: bb3, otherwise: bb2]; } bb2: { @@ -32,10 +34,12 @@ fn opt1(_1: &Result) -> &u32 { } bb4: { - StorageLive(_3); - _3 = &(((*_1) as Ok).0: u32); - _0 = &(*_3); - StorageDead(_3); + StorageLive(_4); + _4 = &(((*_1) as Ok).0: u32); + _2 = &(*_4); + StorageDead(_4); + _0 = &(*_2); + StorageDead(_2); return; } } diff --git a/tests/mir-opt/building/match/never_patterns.opt2.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/never_patterns.opt2.SimplifyCfg-initial.after.mir index fc0769d6f7dcc..494849a596f2b 100644 --- a/tests/mir-opt/building/match/never_patterns.opt2.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/never_patterns.opt2.SimplifyCfg-initial.after.mir @@ -3,18 +3,22 @@ fn opt2(_1: &Result) -> &u32 { debug res => _1; let mut _0: &u32; - let mut _2: isize; - let _3: &u32; + let _2: &u32; + let mut _3: isize; + let _4: &u32; scope 1 { - debug x => _3; + debug x => _4; } bb0: { + StorageLive(_2); PlaceMention(_1); - StorageLive(_3); - _3 = &(((*_1) as Ok).0: u32); - _0 = &(*_3); - StorageDead(_3); + StorageLive(_4); + _4 = &(((*_1) as Ok).0: u32); + _2 = &(*_4); + StorageDead(_4); + _0 = &(*_2); + StorageDead(_2); return; } } diff --git a/tests/mir-opt/building/match/never_patterns.opt3.SimplifyCfg-initial.after.mir b/tests/mir-opt/building/match/never_patterns.opt3.SimplifyCfg-initial.after.mir index 86347db4d92eb..4de6c0f30235f 100644 --- a/tests/mir-opt/building/match/never_patterns.opt3.SimplifyCfg-initial.after.mir +++ b/tests/mir-opt/building/match/never_patterns.opt3.SimplifyCfg-initial.after.mir @@ -3,23 +3,27 @@ fn opt3(_1: &Result) -> &u32 { debug res => _1; let mut _0: &u32; - let mut _2: isize; - let _3: &u32; + let _2: &u32; + let mut _3: isize; + let _4: &u32; scope 1 { - debug x => _3; + debug x => _4; } bb0: { + StorageLive(_2); PlaceMention(_1); - _2 = discriminant((*_1)); - switchInt(move _2) -> [1: bb2, otherwise: bb1]; + _3 = discriminant((*_1)); + switchInt(move _3) -> [1: bb2, otherwise: bb1]; } bb1: { - StorageLive(_3); - _3 = &(((*_1) as Ok).0: u32); - _0 = &(*_3); - StorageDead(_3); + StorageLive(_4); + _4 = &(((*_1) as Ok).0: u32); + _2 = &(*_4); + StorageDead(_4); + _0 = &(*_2); + StorageDead(_2); return; } diff --git a/tests/ui/coercion/coerce-loop-issue-122561.stderr b/tests/ui/coercion/coerce-loop-issue-122561.stderr index 3fd6671565f18..39ea0bb6e6ea8 100644 --- a/tests/ui/coercion/coerce-loop-issue-122561.stderr +++ b/tests/ui/coercion/coerce-loop-issue-122561.stderr @@ -90,12 +90,6 @@ LL | | } | = note: expected type `!` found unit type `()` - = note: `for` loops evaluate to unit type `()` -help: consider adding a diverging expression here - | -LL ~ } -LL + /* `loop {}` or `panic!("...")` */ - | error[E0308]: mismatched types --> $DIR/coerce-loop-issue-122561.rs:35:32 diff --git a/tests/ui/issues/issue-28839.rs b/tests/ui/issues/issue-28839.rs index 76b0fa2d6e089..7c5960c0a70fc 100644 --- a/tests/ui/issues/issue-28839.rs +++ b/tests/ui/issues/issue-28839.rs @@ -1,12 +1,20 @@ -//@ run-pass +//@ check-fail pub struct Foo; -pub fn get_foo2<'a>(foo: &'a mut Option<&'a mut Foo>) -> &'a mut Foo { +pub fn get_foo1<'a>(foo: &'a mut Option<&'a mut Foo>) -> &'a mut Foo { match foo { - // Ensure that this is not considered a move, but rather a reborrow. &mut Some(ref mut x) => *x, + //~^ ERROR cannot move out of + &mut None => panic!(), + } +} + +pub fn get_foo2<'a>(foo: &'a mut Option<&'a mut Foo>) -> &'a mut Foo { + match foo { &mut None => panic!(), + &mut Some(ref mut x) => *x, + //~^ ERROR cannot move out of } } diff --git a/tests/ui/issues/issue-28839.stderr b/tests/ui/issues/issue-28839.stderr new file mode 100644 index 0000000000000..29bbada5673d7 --- /dev/null +++ b/tests/ui/issues/issue-28839.stderr @@ -0,0 +1,15 @@ +error[E0507]: cannot move out of `*x` which is behind a mutable reference + --> $DIR/issue-28839.rs:7:33 + | +LL | &mut Some(ref mut x) => *x, + | ^^ move occurs because `*x` has type `&mut Foo`, which does not implement the `Copy` trait + +error[E0507]: cannot move out of `*x` which is behind a mutable reference + --> $DIR/issue-28839.rs:16:33 + | +LL | &mut Some(ref mut x) => *x, + | ^^ move occurs because `*x` has type `&mut Foo`, which does not implement the `Copy` trait + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0507`. diff --git a/tests/ui/nll/issue-52213.rs b/tests/ui/nll/issue-52213.rs index a016924a869a8..2c8ffdca4df78 100644 --- a/tests/ui/nll/issue-52213.rs +++ b/tests/ui/nll/issue-52213.rs @@ -1,7 +1,7 @@ fn transmute_lifetime<'a, 'b, T>(t: &'a (T,)) -> &'b T { match (&t,) { - ((u,),) => u, //~^ ERROR lifetime may not live long enough + ((u,),) => u, } } diff --git a/tests/ui/nll/issue-52213.stderr b/tests/ui/nll/issue-52213.stderr index ed3723a7f03b0..2e6a0d81cba02 100644 --- a/tests/ui/nll/issue-52213.stderr +++ b/tests/ui/nll/issue-52213.stderr @@ -1,13 +1,15 @@ error: lifetime may not live long enough - --> $DIR/issue-52213.rs:3:20 + --> $DIR/issue-52213.rs:2:5 | -LL | fn transmute_lifetime<'a, 'b, T>(t: &'a (T,)) -> &'b T { - | -- -- lifetime `'b` defined here - | | - | lifetime `'a` defined here -LL | match (&t,) { -LL | ((u,),) => u, - | ^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` +LL | fn transmute_lifetime<'a, 'b, T>(t: &'a (T,)) -> &'b T { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +LL | / match (&t,) { +LL | | +LL | | ((u,),) => u, +LL | | } + | |_____^ function was supposed to return data with lifetime `'b` but it is returning data with lifetime `'a` | = help: consider adding the following bound: `'a: 'b` From 2535e3a19572752995a0ed408249ab063818a769 Mon Sep 17 00:00:00 2001 From: jackh726 Date: Sat, 3 Jan 2026 04:47:45 +0000 Subject: [PATCH 3/3] Add comment --- compiler/rustc_hir_typeck/src/coercion.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index 946be5a5e6aed..f9142d9ad7934 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/compiler/rustc_hir_typeck/src/coercion.rs @@ -1519,14 +1519,20 @@ impl<'tcx> CoerceMany<'tcx> { // Handle the actual type unification etc. let result = if let Some(expression) = expression { + // For the *first* expression being coerced, we call `fcx.coerce`, + // which will actually end up using `Sub` rather tha `Lub`. + // This is not the most ideal thing to do, but this breaks all over + // the place when removing this special-case. + // So, for now, to keep things *logically* a bit more simple, we + // have a `force_lub` option that callers can use (currently only + // is set in match coercion) to guarantee that all expressions use + // Lub coercion. if !self.force_lub && self.expressions.is_empty() { - // Special-case the first expression we are coercing. - // To be honest, I'm not entirely sure why we do this. - // We don't allow two-phase borrows, see comment in try_find_coercion_lub for why fcx.coerce( expression, expression_ty, self.expected_ty, + // We don't allow two-phase borrows, see comment in try_find_coercion_lub for why AllowTwoPhase::No, Some(cause.clone()), )