From c7a9908fb8407d986e9b4e0b832889a8b805315d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 22 Aug 2024 11:21:52 -0400 Subject: [PATCH] Do not coerce places if they do not constitute reads --- compiler/rustc_hir_typeck/src/coercion.rs | 21 +++-- compiler/rustc_hir_typeck/src/expr.rs | 90 ++++++++++++++----- compiler/rustc_hir_typeck/src/pat.rs | 2 +- tests/ui/reachable/expr_assign.stderr | 9 +- .../reachable/unwarned-match-on-never.stderr | 2 +- 5 files changed, 88 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/coercion.rs b/compiler/rustc_hir_typeck/src/coercion.rs index b47d8a97bceea..aba36f1566d43 100644 --- a/compiler/rustc_hir_typeck/src/coercion.rs +++ b/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 { 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, diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 2536d0e5009b4..cd534ca51e168 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/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, diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index afa354960a0ee..13ba19194bb6f 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/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)); } diff --git a/tests/ui/reachable/expr_assign.stderr b/tests/ui/reachable/expr_assign.stderr index c51156b3f40cf..cfbbe04db769d 100644 --- a/tests/ui/reachable/expr_assign.stderr +++ b/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 diff --git a/tests/ui/reachable/unwarned-match-on-never.stderr b/tests/ui/reachable/unwarned-match-on-never.stderr index a296d2a055e09..c1ad3511b4e08 100644 --- a/tests/ui/reachable/unwarned-match-on-never.stderr +++ b/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