Auto merge of #150097 - jackh726:coercion-cleanup, r=<try>

Coercion cleanup

Author: rust-bors[bot]

compiler/rustc_hir_typeck/src/_match.rs

@@ -75,6 +75,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             };
             CoerceMany::with_coercion_sites(coerce_first, arms)
         };
+        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

@@ -1290,29 +1290,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 {
@@ -1387,6 +1387,7 @@ pub(crate) struct CoerceMany<'tcx, 'exprs, E: AsCoercionSite> {
     final_ty: Option<Ty<'tcx>>,
     expressions: Expressions<'tcx, 'exprs, E>,
     pushed: usize,
+    force_lub: bool,
 }
 
 /// The type of a `CoerceMany` that is storing up the expressions into
@@ -1416,7 +1417,13 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
     }
 
     fn make(expected_ty: Ty<'tcx>, expressions: Expressions<'tcx, 'exprs, E>) -> Self {
-        CoerceMany { expected_ty, final_ty: None, expressions, pushed: 0 }
+        CoerceMany { expected_ty, final_ty: None, expressions, pushed: 0, force_lub: false }
+    }
+
+    pub(crate) fn force_lub(&mut self) {
+        // Don't accidentally let someone switch this after coercing things
+        assert_eq!(self.pushed, 0);
+        self.force_lub = true;
     }
 
     /// Returns the "expected type" with which this coercion was
@@ -1529,10 +1536,9 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
 
         // Handle the actual type unification etc.
         let result = if let Some(expression) = expression {
-            if self.pushed == 0 {
-                // 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
+            if !self.force_lub && self.pushed == 0 {
+                // For this *first* expression, we do *not* use LUB
+                // (which `try_find_coercion_lub` does).
                 fcx.coerce(
                     expression,
                     expression_ty,

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/coercion/multistep-fail.rs

@@ -0,0 +1,111 @@
+//@ check-fail
+//@ known-bug: #148283
+
+#![allow(static_mut_refs)]
+#![allow(dead_code)]
+use std::ops::Deref;
+
+pub static mut ACTIONS: Vec<&'static str> = Vec::new();
+
+pub struct Wrap<T: ?Sized>(T);
+
+// Deref Chain: FinalType <- UnsizedArray <- IntWrapper <- ArrayWrapper <- TopType
+pub struct TopType;
+pub type ArrayWrapper = Wrap<[i32; 0]>;
+pub struct IntWrapper;
+pub type UnsizedArray = Wrap<[i32]>;
+pub struct FinalType;
+pub struct TopTypeNoTrait;
+
+impl Deref for TopType {
+    type Target = ArrayWrapper;
+    fn deref(&self) -> &Self::Target {
+        unsafe { ACTIONS.push("deref TopType->ArrayWrapper"); }
+        &Wrap([])
+    }
+}
+
+impl Deref for ArrayWrapper {
+    type Target = IntWrapper;
+    fn deref(&self) -> &Self::Target {
+        unsafe { ACTIONS.push("deref ArrayWrapper->IntWrapper"); }
+        &IntWrapper
+    }
+}
+
+impl Deref for IntWrapper {
+    type Target = UnsizedArray;
+    fn deref(&self) -> &Self::Target {
+        unsafe { ACTIONS.push("deref IntWrapper->UnsizedArray"); }
+        &Wrap([])
+    }
+}
+
+impl Deref for UnsizedArray {
+    type Target = FinalType;
+    fn deref(&self) -> &Self::Target {
+        unsafe { ACTIONS.push("deref UnsizedArray->FinalType"); }
+        &FinalType
+    }
+}
+
+impl Deref for TopTypeNoTrait {
+    type Target = ArrayWrapper;
+    fn deref(&self) -> &Self::Target {
+        unsafe { ACTIONS.push("deref TopTypeNoTrait->ArrayWrapper"); }
+        &Wrap([])
+    }
+}
+
+trait Trait {
+    fn self_ty(&self);
+
+    fn complete(&self) -> Vec<&'static str> {
+        self.self_ty();
+        let actions = unsafe { ACTIONS.clone() };
+        unsafe { ACTIONS.clear() };
+        actions
+    }
+}
+
+impl Trait for TopType {
+    fn self_ty(&self) {
+        unsafe { ACTIONS.push("self_ty TopType"); }
+    }
+}
+
+impl Trait for ArrayWrapper {
+    fn self_ty(&self) {
+        unsafe { ACTIONS.push("self_ty ArrayWrapper"); }
+    }
+}
+
+impl Trait for IntWrapper {
+    fn self_ty(&self) {
+        unsafe { ACTIONS.push("self_ty IntWrapper"); }
+    }
+}
+
+impl Trait for UnsizedArray {
+    fn self_ty(&self) {
+        unsafe { ACTIONS.push("self_ty UnsizedArray"); }
+    }
+}
+
+impl Trait for FinalType {
+    fn self_ty(&self) {
+        unsafe { ACTIONS.push("self_ty FinalType"); }
+    }
+}
+
+fn deref_to_dyn_direct() {
+    let x = match 0 {
+        0 => &TopTypeNoTrait as &TopTypeNoTrait,
+        1 => &TopTypeNoTrait as &FinalType as &dyn Trait,
+        _ => loop {},
+    };
+}
+
+fn main() {
+    deref_to_dyn_direct();
+}

tests/ui/coercion/multistep-fail.stderr

@@ -0,0 +1,22 @@
+error[E0277]: the trait bound `TopTypeNoTrait: Trait` is not satisfied
+  --> $DIR/multistep-fail.rs:104:14
+   |
+LL |         1 => &TopTypeNoTrait as &FinalType as &dyn Trait,
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ unsatisfied trait bound
+   |
+help: the trait `Trait` is not implemented for `TopTypeNoTrait`
+  --> $DIR/multistep-fail.rs:18:1
+   |
+LL | pub struct TopTypeNoTrait;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: the following other types implement trait `Trait`:
+             FinalType
+             IntWrapper
+             TopType
+             Wrap<[i32; 0]>
+             Wrap<[i32]>
+   = note: required for the cast from `&TopTypeNoTrait` to `&dyn Trait`
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.