Skip to content

Commit

Permalink
Evaluating place expr that is never read from does not diverge
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Oct 5, 2024
1 parent 9096f4f commit 6371ef6
Show file tree
Hide file tree
Showing 10 changed files with 146 additions and 12 deletions.
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 @@ -238,8 +238,11 @@ 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 a place expression that isn't being read from, in which case
// diverging would be unsound since we may never actually read the `!`.
// e.g. `let _ = *never_ptr;` with `never_ptr: *const !`.
if ty.is_never() && self.expr_constitutes_read(expr) {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
}

Expand All @@ -257,6 +260,66 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty
}

pub(super) fn expr_constitutes_read(&self, expr: &'tcx hir::Expr<'tcx>) -> bool {
// We only care about place exprs. Anything else returns an immediate
// which would constitute a read. We don't care about distinguishing
// "syntactic" place exprs since if the base of a field projection is
// not a place then it would've been UB to read from it anyways since
// that constitutes a read.
if !expr.is_syntactic_place_expr() {
return true;
}

// If this expression has any adjustments applied after the place expression,
// 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;
// No need to continue.
false
}
});
does_read
}

match self.tcx.parent_hir_node(expr.hir_id) {
// Addr-of, field projections, and LHS of assignment don't constitute reads.
// Assignment does call `drop_in_place`, though, but its safety
// requirements are not the same.
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,

// If we have a subpattern that performs a read, we want to consider this
// to diverge for compatibility to support something like `let x: () = *never_ptr;`.
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
7 changes: 7 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 tracing::{debug, instrument, trace};
use ty::VariantDef;

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

Expand Down Expand Up @@ -276,6 +277,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};

// All other patterns constitute a read, which causes us to diverge
// if the type is never.
if ty.is_never() && !matches!(pat.kind, PatKind::Wild | PatKind::Never | PatKind::Or(_)) {
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
fn process_never(_1: *const !) -> () {
debug input => _1;
let mut _0: ();
let _2: &!;
scope 1 {
debug _input => _2;
debug _input => _1;
}

bb0: {
unreachable;
return;
}
}
2 changes: 0 additions & 2 deletions tests/mir-opt/uninhabited_enum.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ pub fn process_never(input: *const !) {
#[no_mangle]
pub fn process_void(input: *const Void) {
let _input = unsafe { &*input };
// In the future, this should end with `unreachable`, but we currently only do
// unreachability analysis for `!`.
}

fn main() {}
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 6371ef6

Please sign in to comment.