Skip to content

Commit

Permalink
Do not coerce places if they do not constitute reads
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Aug 27, 2024
1 parent cefd384 commit c7a9908
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 36 deletions.
21 changes: 14 additions & 7 deletions compiler/rustc_hir_typeck/src/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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>> {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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))
Expand Down Expand Up @@ -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,
Expand Down
90 changes: 67 additions & 23 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
9 changes: 5 additions & 4 deletions tests/ui/reachable/expr_assign.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/reachable/unwarned-match-on-never.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c7a9908

Please sign in to comment.