Skip to content

Commit

Permalink
Auto merge of rust-lang#129392 - compiler-errors:raw-ref-op-doesnt-di…
Browse files Browse the repository at this point in the history
…verge-but-more, r=<try>

[EXPERIMENT] Do not consider match/let/raw-ref of deref that evalautes to ! to diverge

This is the more involved version of rust-lang#129371. It's pretty ugly rn.

r? `@ghost`
  • Loading branch information
bors committed Aug 22, 2024
2 parents 0f6e1ae + 9bb136b commit de0e806
Show file tree
Hide file tree
Showing 9 changed files with 160 additions and 14 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
67 changes: 65 additions & 2 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"),
}

// Any expression that produces a value of type `!` must have diverged
if ty.is_never() {
// 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() && self.expr_is_rvalue_for_divergence(expr) {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
}

Expand All @@ -256,6 +257,68 @@ 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(..), .. }) => 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
9 changes: 9 additions & 0 deletions compiler/rustc_hir_typeck/src/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use rustc_trait_selection::traits::{ObligationCause, ObligationCauseCode};
use ty::VariantDef;

use super::report_unexpected_variant_res;
use crate::diverges::Diverges;
use crate::gather_locals::DeclOrigin;
use crate::{errors, FnCtxt, LoweredTy};

Expand Down Expand Up @@ -276,6 +277,14 @@ 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 | PatKind::Or(_)) {
if ty.is_never() {
self.diverges.set(self.diverges.get() | Diverges::always(pat.span));
}
}

self.write_ty(pat.hir_id, ty);

// (note_1): In most of the cases where (note_1) is referenced
Expand Down
13 changes: 13 additions & 0 deletions tests/ui/never_type/diverging-place-match.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![feature(never_type)]

fn make_up_a_value<T>() -> T {
unsafe {
//~^ ERROR mismatched types
let x: *const ! = 0 as _;
let _: ! = *x;
// Since `*x` "diverges" in HIR, but doesn't count as a read in MIR, this
// is unsound since we act as if it diverges but it doesn't.
}
}

fn main() {}
20 changes: 20 additions & 0 deletions tests/ui/never_type/diverging-place-match.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0308]: mismatched types
--> $DIR/diverging-place-match.rs:4:5
|
LL | fn make_up_a_value<T>() -> T {
| - expected this type parameter
LL | / unsafe {
LL | |
LL | | let x: *const ! = 0 as _;
LL | | let _: ! = *x;
LL | | // Since `*x` "diverges" in HIR, but doesn't count as a read in MIR, this
LL | | // is unsound since we act as if it diverges but it doesn't.
LL | | }
| |_____^ expected type parameter `T`, found `()`
|
= note: expected type parameter `T`
found unit type `()`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0308`.
13 changes: 13 additions & 0 deletions tests/ui/raw-ref-op/never-place-isnt-diverging.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![feature(never_type)]

fn make_up_a_value<T>() -> T {
unsafe {
//~^ ERROR mismatched types
let x: *const ! = 0 as _;
&raw const *x;
// Since `*x` is `!`, HIR typeck used to think that it diverges
// and allowed the block to coerce to any value, leading to UB.
}
}

fn main() {}
20 changes: 20 additions & 0 deletions tests/ui/raw-ref-op/never-place-isnt-diverging.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0308]: mismatched types
--> $DIR/never-place-isnt-diverging.rs:4:5
|
LL | fn make_up_a_value<T>() -> T {
| - expected this type parameter
LL | / unsafe {
LL | |
LL | | let x: *const ! = 0 as _;
LL | | &raw const *x;
LL | | // Since `*x` is `!`, HIR typeck used to think that it diverges
LL | | // and allowed the block to coerce to any value, leading to UB.
LL | | }
| |_____^ expected type parameter `T`, found `()`
|
= note: expected type parameter `T`
found unit type `()`

error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0308`.
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 de0e806

Please sign in to comment.