Coercion cleanup

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;

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 {
@@ -1382,6 +1382,7 @@ pub(crate) struct CoerceMany<'tcx> {
     expected_ty: Ty<'tcx>,
     final_ty: Option<Ty<'tcx>>,
     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,14 +1519,20 @@ impl<'tcx> CoerceMany<'tcx> {
 
         // Handle the actual type unification etc.
         let result = if let Some(expression) = expression {
-            if 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
+            // 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() {
                 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()),
                 )

tests/mir-opt/building/match/never_patterns.opt1.SimplifyCfg-initial.after.mir

@@ -3,22 +3,24 @@
 fn opt1(_1: &Result<u32, Void>) -> &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, Void>) -> &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;
     }
 }

tests/mir-opt/building/match/never_patterns.opt2.SimplifyCfg-initial.after.mir

@@ -3,18 +3,22 @@
 fn opt2(_1: &Result<u32, Void>) -> &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;
     }
 }

tests/mir-opt/building/match/never_patterns.opt3.SimplifyCfg-initial.after.mir

@@ -3,23 +3,27 @@
 fn opt3(_1: &Result<u32, Void>) -> &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;
     }
 

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

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
     }
 }
 

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`.

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,
     }
 }
 

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`