Do not coerce places if they do not constitute reads

Author: compiler-errors

compiler/rustc_hir_typeck/src/coercion.rs

@@ -81,6 +81,7 @@ struct Coerce<'a, 'tcx> {
     /// See #47489 and #48598
     /// See docs on the "AllowTwoPhase" type for a more detailed discussion
     allow_two_phase: AllowTwoPhase,
+    coerce_never: bool,
 }
 
 impl<'a, 'tcx> Deref for Coerce<'a, 'tcx> {
@@ -124,8 +125,9 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
         fcx: &'f FnCtxt<'f, 'tcx>,
         cause: ObligationCause<'tcx>,
         allow_two_phase: AllowTwoPhase,
+        coerce_never: bool,
     ) -> Self {
-        Coerce { fcx, cause, allow_two_phase, use_lub: false }
+        Coerce { fcx, cause, allow_two_phase, use_lub: false, coerce_never }
     }
 
     fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> {
@@ -176,7 +178,11 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
 
         // Coercing from `!` to any type is allowed:
         if a.is_never() {
-            return success(simple(Adjust::NeverToAny)(b), b, vec![]);
+            if self.coerce_never {
+                return success(simple(Adjust::NeverToAny)(b), b, vec![]);
+            } else {
+                return self.unify_and(a, b, identity);
+            }
         }
 
         // Coercing *from* an unresolved inference variable means that
@@ -978,7 +984,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     /// The expressions *must not* have any preexisting adjustments.
     pub fn coerce(
         &self,
-        expr: &hir::Expr<'_>,
+        expr: &'tcx hir::Expr<'tcx>,
         expr_ty: Ty<'tcx>,
         mut target: Ty<'tcx>,
         allow_two_phase: AllowTwoPhase,
@@ -995,7 +1001,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let cause =
             cause.unwrap_or_else(|| self.cause(expr.span, ObligationCauseCode::ExprAssignable));
-        let coerce = Coerce::new(self, cause, allow_two_phase);
+        let coerce =
+            Coerce::new(self, cause, allow_two_phase, self.expr_is_rvalue_for_divergence(expr));
         let ok = self.commit_if_ok(|_| coerce.coerce(source, target))?;
 
         let (adjustments, _) = self.register_infer_ok_obligations(ok);
@@ -1018,7 +1025,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         let cause = self.cause(DUMMY_SP, ObligationCauseCode::ExprAssignable);
         // We don't ever need two-phase here since we throw out the result of the coercion
-        let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
+        let coerce = Coerce::new(self, cause, AllowTwoPhase::No, true);
         self.probe(|_| {
             let Ok(ok) = coerce.coerce(source, target) else {
                 return false;
@@ -1035,7 +1042,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     pub fn deref_steps(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> Option<usize> {
         let cause = self.cause(DUMMY_SP, ObligationCauseCode::ExprAssignable);
         // We don't ever need two-phase here since we throw out the result of the coercion
-        let coerce = Coerce::new(self, cause, AllowTwoPhase::No);
+        let coerce = Coerce::new(self, cause, AllowTwoPhase::No, true);
         coerce
             .autoderef(DUMMY_SP, expr_ty)
             .find_map(|(ty, steps)| self.probe(|_| coerce.unify(ty, target)).ok().map(|_| steps))
@@ -1192,7 +1199,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         // probably aren't processing function arguments here and even if we were,
         // they're going to get autorefed again anyway and we can apply 2-phase borrows
         // at that time.
-        let mut coerce = Coerce::new(self, cause.clone(), AllowTwoPhase::No);
+        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,

compiler/rustc_hir_typeck/src/expr.rs

@@ -239,29 +239,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // Any expression that produces a value of type `!` must have diverged,
         // unless it's the place of a raw ref expr, or a scrutinee of a match.
-        if ty.is_never() {
-            if matches!(expr.kind, hir::ExprKind::Unary(hir::UnOp::Deref, _)) {
-                match self.tcx.parent_hir_node(expr.hir_id) {
-                    hir::Node::Expr(hir::Expr {
-                        kind: hir::ExprKind::AddrOf(hir::BorrowKind::Raw, ..),
-                        ..
-                    }) => {}
-                    hir::Node::Expr(hir::Expr {
-                        kind: hir::ExprKind::Let(hir::LetExpr { init: target, .. }),
-                        ..
-                    })
-                    | hir::Node::Expr(hir::Expr {
-                        kind: hir::ExprKind::Match(target, _, _), ..
-                    })
-                    | hir::Node::LetStmt(hir::LetStmt { init: Some(target), .. })
-                        if expr.hir_id == target.hir_id => {}
-                    _ => {
-                        self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
-                    }
-                }
-            } else {
-                self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
-            }
+        if ty.is_never() && self.expr_is_rvalue_for_divergence(expr) {
+            self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
         }
 
         // Record the type, which applies it effects.
@@ -278,6 +257,71 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         ty
     }
 
+    // FIXME: built-in indexing should be supported here.
+    /// THIS IS SUBTLE BUT I DONT WANT TO EXPLAIN IT YET.
+    pub(super) fn expr_is_rvalue_for_divergence(&self, expr: &'tcx hir::Expr<'tcx>) -> bool {
+        match expr.kind {
+            ExprKind::Path(QPath::Resolved(
+                _,
+                hir::Path {
+                    res: Res::Local(..) | Res::Def(DefKind::Static { .. }, _) | Res::Err,
+                    ..
+                },
+            ))
+            | ExprKind::Unary(hir::UnOp::Deref, _)
+            | ExprKind::Field(..)
+            | ExprKind::Index(..) => {
+                // All places.
+            }
+
+            _ => return true,
+        }
+
+        // If this expression has any adjustments, they may constitute reads.
+        if !self.typeck_results.borrow().expr_adjustments(expr).is_empty() {
+            return true;
+        }
+
+        fn pat_does_read(pat: &hir::Pat<'_>) -> bool {
+            let mut does_read = false;
+            pat.walk(|pat| {
+                if matches!(
+                    pat.kind,
+                    hir::PatKind::Wild | hir::PatKind::Never | hir::PatKind::Or(_)
+                ) {
+                    true
+                } else {
+                    does_read = true;
+                    false
+                }
+            });
+            does_read
+        }
+
+        match self.tcx.parent_hir_node(expr.hir_id) {
+            hir::Node::Expr(hir::Expr {
+                kind: hir::ExprKind::AddrOf(hir::BorrowKind::Raw, ..),
+                ..
+            }) => false,
+            hir::Node::Expr(hir::Expr {
+                kind: hir::ExprKind::Assign(target, _, _) | hir::ExprKind::Field(target, _),
+                ..
+            }) if expr.hir_id == target.hir_id => false,
+            hir::Node::LetStmt(hir::LetStmt { init: Some(target), pat, .. })
+            | hir::Node::Expr(hir::Expr {
+                kind: hir::ExprKind::Let(hir::LetExpr { init: target, pat, .. }),
+                ..
+            }) if expr.hir_id == target.hir_id && !pat_does_read(*pat) => false,
+            hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Match(target, arms, _), .. })
+                if expr.hir_id == target.hir_id
+                    && !arms.iter().any(|arm| pat_does_read(arm.pat)) =>
+            {
+                false
+            }
+            _ => true,
+        }
+    }
+
     #[instrument(skip(self, expr), level = "debug")]
     fn check_expr_kind(
         &self,

compiler/rustc_hir_typeck/src/pat.rs

@@ -279,7 +279,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // All other patterns constitute a read, which causes us to diverge
         // if the type is never.
-        if !matches!(pat.kind, PatKind::Wild | PatKind::Never) {
+        if !matches!(pat.kind, PatKind::Wild | PatKind::Never | PatKind::Or(_)) {
             if ty.is_never() {
                 self.diverges.set(self.diverges.get() | Diverges::always(pat.span));
             }

tests/ui/reachable/expr_assign.stderr

@@ -14,12 +14,13 @@ LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
 
 error: unreachable expression
-  --> $DIR/expr_assign.rs:20:14
+  --> $DIR/expr_assign.rs:20:9
    |
 LL |         *p = return;
-   |         --   ^^^^^^ unreachable expression
-   |         |
-   |         any code following this expression is unreachable
+   |         ^^^^^------
+   |         |    |
+   |         |    any code following this expression is unreachable
+   |         unreachable expression
 
 error: unreachable expression
   --> $DIR/expr_assign.rs:26:15

tests/ui/reachable/unwarned-match-on-never.stderr

@@ -2,7 +2,7 @@ error: unreachable expression
   --> $DIR/unwarned-match-on-never.rs:10:5
    |
 LL |     match x {}
-   |           - any code following this expression is unreachable
+   |     ---------- any code following this expression is unreachable
 LL |     // But matches in unreachable code are warned.
 LL |     match x {}
    |     ^^^^^^^^^^ unreachable expression