dnrops
/
rust
mirror of https://github.com/rust-lang/rust.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Be far more strict about what we consider to be a read of never

This commit is contained in:
Michael Goulet 2024-09-06 10:19:07 -04:00
parent d2bd018dad
commit e8d5eb2a2b
11 changed files with 176 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
compiler/rustc_hir_typeck/src/coercion.rs
View File

@ -1066,7 +1066,12 @@ 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, self.expr_constitutes_read(expr));
let coerce = Coerce::new(
self,
cause,
allow_two_phase,
self.expr_guaranteed_to_constitute_read_for_never(expr),
);
let ok = self.commit_if_ok(|_| coerce.coerce(source, target))?;
let (adjustments, _) = self.register_infer_ok_obligations(ok);
@ -1268,6 +1273,9 @@ 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.
//
// NOTE: we set `coerce_never` to `true` here because coercion LUBs only
// operate on values and not places, so a never coercion is valid.
let mut coerce = Coerce::new(self, cause.clone(), AllowTwoPhase::No, true);
coerce.use_lub = true;

85
compiler/rustc_hir_typeck/src/expr.rs
View File

@ -65,7 +65,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// While we don't allow *arbitrary* coercions here, we *do* allow
// coercions from ! to `expected`.
if ty.is_never() && self.expr_constitutes_read(expr) {
if ty.is_never() && self.expr_guaranteed_to_constitute_read_for_never(expr) {
if let Some(_) = self.typeck_results.borrow().adjustments().get(expr.hir_id) {
let reported = self.dcx().span_delayed_bug(
expr.span,
@ -245,7 +245,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// 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) {
if ty.is_never() && self.expr_guaranteed_to_constitute_read_for_never(expr) {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
}
@ -274,10 +274,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// expression and the *parent* expression is the scrutinee of a match or
/// the pointee of an `&` addr-of expression, since both of those parent
/// expressions take a *place* and not a value.
///
/// This function is unfortunately a bit heuristical, though it is certainly
/// far sounder than the prior status quo: <https://github.com/rust-lang/rust/issues/117288>.
pub(super) fn expr_constitutes_read(&self, expr: &'tcx hir::Expr<'tcx>) -> bool {
pub(super) fn expr_guaranteed_to_constitute_read_for_never(
&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
@ -300,15 +300,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr.hir_id != lhs.hir_id
}
// 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;`.
// See note on `PatKind::Or` below for why this is `all`.
ExprKind::Match(scrutinee, arms, _) => {
assert_eq!(scrutinee.hir_id, expr.hir_id);
arms.iter().any(|arm| self.pat_constitutes_read(arm.pat))
arms.iter()
.all(|arm| self.pat_guaranteed_to_constitute_read_for_never(arm.pat))
}
ExprKind::Let(hir::LetExpr { init, pat, .. }) => {
assert_eq!(init.hir_id, expr.hir_id);
self.pat_constitutes_read(*pat)
self.pat_guaranteed_to_constitute_read_for_never(*pat)
}
// Any expression child of these expressions constitute reads.
@ -349,7 +349,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// to diverge for compatibility to support something like `let x: () = *never_ptr;`.
hir::Node::LetStmt(hir::LetStmt { init: Some(target), pat, .. }) => {
assert_eq!(target.hir_id, expr.hir_id);
self.pat_constitutes_read(*pat)
self.pat_guaranteed_to_constitute_read_for_never(*pat)
}
// These nodes (if they have a sub-expr) do constitute a read.
@ -401,36 +401,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
/// Whether this pattern constitutes a read of value of the scrutinee that
/// it is matching against.
/// it is matching against. This is used to determine whether we should
/// perform `NeverToAny` coercions.
///
/// See above for the nuances of what happens when this returns true.
pub(super) fn pat_constitutes_read(&self, pat: &hir::Pat<'_>) -> bool {
let mut does_read = false;
pat.walk(|pat| {
match pat.kind {
hir::PatKind::Wild | hir::PatKind::Never | hir::PatKind::Or(_) => {
// Recurse
true
}
hir::PatKind::Binding(_, _, _, _)
| hir::PatKind::Struct(_, _, _)
| hir::PatKind::TupleStruct(_, _, _)
| hir::PatKind::Path(_)
| hir::PatKind::Tuple(_, _)
| hir::PatKind::Box(_)
| hir::PatKind::Ref(_, _)
| hir::PatKind::Deref(_)
| hir::PatKind::Lit(_)
| hir::PatKind::Range(_, _, _)
| hir::PatKind::Slice(_, _, _)
| hir::PatKind::Err(_) => {
does_read = true;
// No need to continue.
false
}
pub(super) fn pat_guaranteed_to_constitute_read_for_never(&self, pat: &hir::Pat<'_>) -> bool {
match pat.kind {
// Does not constitute a read.
hir::PatKind::Wild => false,
// This is unnecessarily restrictive when the pattern that doesn't
// constitute a read is unreachable.
//
// For example `match *never_ptr { value => {}, _ => {} }` or
// `match *never_ptr { _ if false => {}, value => {} }`.
//
// It is however fine to be restrictive here; only returning `true`
// can lead to unsoundness.
hir::PatKind::Or(subpats) => {
subpats.iter().all(|pat| self.pat_guaranteed_to_constitute_read_for_never(pat))
}
});
does_read
// Does constitute a read, since it is equivalent to a discriminant read.
hir::PatKind::Never => true,
// All of these constitute a read, or match on something that isn't `!`,
// which would require a `NeverToAny` coercion.
hir::PatKind::Binding(_, _, _, _)
| hir::PatKind::Struct(_, _, _)
| hir::PatKind::TupleStruct(_, _, _)
| hir::PatKind::Path(_)
| hir::PatKind::Tuple(_, _)
| hir::PatKind::Box(_)
| hir::PatKind::Ref(_, _)
| hir::PatKind::Deref(_)
| hir::PatKind::Lit(_)
| hir::PatKind::Range(_, _, _)
| hir::PatKind::Slice(_, _, _)
| hir::PatKind::Err(_) => true,
}
}
#[instrument(skip(self, expr), level = "debug")]

7
compiler/rustc_hir_typeck/src/pat.rs
View File

@ -27,7 +27,6 @@ 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};
@ -277,12 +276,6 @@ 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() && self.pat_constitutes_read(pat) {
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

4
tests/mir-opt/uninhabited_enum.process_never.SimplifyLocals-final.after.mir
View File

@ -4,10 +4,10 @@ fn process_never(_1: *const !) -> () {
debug input => _1;
let mut _0: ();
scope 1 {
debug _input => _1;
debug _input => const ();
}
bb0: {
return;
unreachable;
}
}

2
tests/mir-opt/uninhabited_enum.process_void.SimplifyLocals-final.after.mir
View File

@ -4,7 +4,7 @@ fn process_void(_1: *const Void) -> () {
debug input => _1;
let mut _0: ();
scope 1 {
debug _input => _1;
debug _input => const ZeroSized: Void;
}
bb0: {

7
tests/mir-opt/uninhabited_enum.rs
View File

@ -1,18 +1,21 @@
// skip-filecheck
#![feature(never_type)]
#[derive(Copy, Clone)]
pub enum Void {}
// EMIT_MIR uninhabited_enum.process_never.SimplifyLocals-final.after.mir
#[no_mangle]
pub fn process_never(input: *const !) {
let _input = unsafe { &*input };
let _input = unsafe { *input };
}
// EMIT_MIR uninhabited_enum.process_void.SimplifyLocals-final.after.mir
#[no_mangle]
pub fn process_void(input: *const Void) {
let _input = unsafe { &*input };
let _input = unsafe { *input };
// In the future, this should end with `unreachable`, but we currently only do
// unreachability analysis for `!`.
}
fn main() {}

27
tests/ui/never_type/diverging-place-match.rs
View File

@ -44,4 +44,31 @@ fn field_projection() -> ! {
}
}
fn covered_arm() -> ! {
unsafe {
//~^ ERROR mismatched types
let x: *const ! = 0 as _;
let (_ | 1i32) = *x;
//~^ ERROR mismatched types
}
}
// FIXME: This *could* be considered a read of `!`, but we're not that sophisticated..
fn uncovered_arm() -> ! {
unsafe {
//~^ ERROR mismatched types
let x: *const ! = 0 as _;
let (1i32 | _) = *x;
//~^ ERROR mismatched types
}
}
fn coerce_ref_binding() -> ! {
unsafe {
let x: *const ! = 0 as _;
let ref _x: () = *x;
//~^ ERROR mismatched types
}
}
fn main() {}

61
tests/ui/never_type/diverging-place-match.stderr
View File

@ -78,6 +78,65 @@ LL | | }
= note: expected type `!`
found unit type `()`
error: aborting due to 6 previous errors
error[E0308]: mismatched types
--> $DIR/diverging-place-match.rs:51:18
|
LL | let (_ | 1i32) = *x;
| ^^^^ -- this expression has type `!`
| |
| expected `!`, found `i32`
|
= note: expected type `!`
found type `i32`
error[E0308]: mismatched types
--> $DIR/diverging-place-match.rs:48:5
|
LL | / unsafe {
LL | |
LL | | let x: *const ! = 0 as _;
LL | | let (_ | 1i32) = *x;
LL | |
LL | | }
| |_____^ expected `!`, found `()`
|
= note: expected type `!`
found unit type `()`
error[E0308]: mismatched types
--> $DIR/diverging-place-match.rs:61:14
|
LL | let (1i32 | _) = *x;
| ^^^^ -- this expression has type `!`
| |
| expected `!`, found `i32`
|
= note: expected type `!`
found type `i32`
error[E0308]: mismatched types
--> $DIR/diverging-place-match.rs:58:5
|
LL | / unsafe {
LL | |
LL | | let x: *const ! = 0 as _;
LL | | let (1i32 | _) = *x;
LL | |
LL | | }
| |_____^ expected `!`, found `()`
|
= note: expected type `!`
found unit type `()`
error[E0308]: mismatched types
--> $DIR/diverging-place-match.rs:69:26
|
LL | let ref _x: () = *x;
| ^^ expected `()`, found `!`
|
= note: expected unit type `()`
found type `!`
error: aborting due to 11 previous errors
For more information about this error, try `rustc --explain E0308`.

9
tests/ui/raw-ref-op/never-place-isnt-diverging.rs
View File

@ -10,4 +10,13 @@ fn make_up_a_value<T>() -> T {
}
}
fn make_up_a_pointer<T>() -> *const T {
unsafe {
let x: *const ! = 0 as _;
&raw const *x
//~^ ERROR mismatched types
}
}
fn main() {}

16
tests/ui/raw-ref-op/never-place-isnt-diverging.stderr
View File

@ -15,6 +15,20 @@ LL | | }
= note: expected type parameter `T`
found unit type `()`
error: aborting due to 1 previous error
error[E0308]: mismatched types
--> $DIR/never-place-isnt-diverging.rs:17:9
|
LL | fn make_up_a_pointer<T>() -> *const T {
| - -------- expected `*const T` because of return type
| |
| expected this type parameter
...
LL | &raw const *x
| ^^^^^^^^^^^^^ expected `*const T`, found `*const !`
|
= note: expected raw pointer `*const T`
found raw pointer `*const !`
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0308`.

2
tests/ui/reachable/unwarned-match-on-never.stderr
View File

@ -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