Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit a0179f5

Browse files
compiler-errorscompiler-errors
committed
Be far more strict about what we consider to be a read of never
1 parent b1a2bd9 commit a0179f5

11 files changed

+160
-55
lines changed

‎compiler/rustc_hir_typeck/src/coercion.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10071007

10081008
let cause =
10091009
cause.unwrap_or_else(|| self.cause(expr.span, ObligationCauseCode::ExprAssignable));
1010-
let coerce = Coerce::new(self, cause, allow_two_phase, self.expr_constitutes_read(expr));
1010+
let coerce = Coerce::new(
1011+
self,
1012+
cause,
1013+
allow_two_phase,
1014+
self.expr_guaranteed_to_constitute_read_for_never(expr),
1015+
);
10111016
let ok = self.commit_if_ok(|_| coerce.coerce(source, target))?;
10121017

10131018
let (adjustments, _) = self.register_infer_ok_obligations(ok);
@@ -1209,6 +1214,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12091214
// probably aren't processing function arguments here and even if we were,
12101215
// they're going to get autorefed again anyway and we can apply 2-phase borrows
12111216
// at that time.
1217+
//
1218+
// NOTE: we set `coerce_never` to `true` here because coercion LUBs only
1219+
// operate on values and not places, so a never coercion is valid.
12121220
let mut coerce = Coerce::new(self, cause.clone(), AllowTwoPhase::No, true);
12131221
coerce.use_lub = true;
12141222

‎compiler/rustc_hir_typeck/src/expr.rs‎

Lines changed: 48 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
6565

6666
// While we don't allow *arbitrary* coercions here, we *do* allow
6767
// coercions from ! to `expected`.
68-
if ty.is_never() && self.expr_constitutes_read(expr) {
68+
if ty.is_never() && self.expr_guaranteed_to_constitute_read_for_never(expr) {
6969
if let Some(_) = self.typeck_results.borrow().adjustments().get(expr.hir_id) {
7070
let reported = self.dcx().span_delayed_bug(
7171
expr.span,
@@ -245,7 +245,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
245245
// unless it's a place expression that isn't being read from, in which case
246246
// diverging would be unsound since we may never actually read the `!`.
247247
// e.g. `let _ = *never_ptr;` with `never_ptr: *const !`.
248-
if ty.is_never() && self.expr_constitutes_read(expr) {
248+
if ty.is_never() && self.expr_guaranteed_to_constitute_read_for_never(expr) {
249249
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
250250
}
251251

@@ -274,10 +274,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
274274
/// expression and the *parent* expression is the scrutinee of a match or
275275
/// the pointee of an `&` addr-of expression, since both of those parent
276276
/// expressions take a *place* and not a value.
277-
///
278-
/// This function is unfortunately a bit heuristical, though it is certainly
279-
/// far sounder than the prior status quo: <https://github.com/rust-lang/rust/issues/117288>.
280-
pub(super) fn expr_constitutes_read(&self, expr: &'tcx hir::Expr<'tcx>) -> bool {
277+
pub(super) fn expr_guaranteed_to_constitute_read_for_never(
278+
&self,
279+
expr: &'tcx hir::Expr<'tcx>,
280+
) -> bool {
281281
// We only care about place exprs. Anything else returns an immediate
282282
// which would constitute a read. We don't care about distinguishing
283283
// "syntactic" place exprs since if the base of a field projection is
@@ -300,15 +300,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
300300
expr.hir_id != lhs.hir_id
301301
}
302302

303-
// If we have a subpattern that performs a read, we want to consider this
304-
// to diverge for compatibility to support something like `let x: () = *never_ptr;`.
303+
// See note on `PatKind::Or` below for why this is `all`.
305304
ExprKind::Match(scrutinee, arms, _) => {
306305
assert_eq!(scrutinee.hir_id, expr.hir_id);
307-
arms.iter().any(|arm| self.pat_constitutes_read(arm.pat))
306+
arms.iter()
307+
.all(|arm| self.pat_guaranteed_to_constitute_read_for_never(arm.pat))
308308
}
309309
ExprKind::Let(hir::LetExpr { init, pat, .. }) => {
310310
assert_eq!(init.hir_id, expr.hir_id);
311-
self.pat_constitutes_read(*pat)
311+
self.pat_guaranteed_to_constitute_read_for_never(*pat)
312312
}
313313

314314
// Any expression child of these expressions constitute reads.
@@ -349,7 +349,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
349349
// to diverge for compatibility to support something like `let x: () = *never_ptr;`.
350350
hir::Node::LetStmt(hir::LetStmt { init: Some(target), pat, .. }) => {
351351
assert_eq!(target.hir_id, expr.hir_id);
352-
self.pat_constitutes_read(*pat)
352+
self.pat_guaranteed_to_constitute_read_for_never(*pat)
353353
}
354354

355355
// These nodes (if they have a sub-expr) do constitute a read.
@@ -400,36 +400,45 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
400400
}
401401

402402
/// Whether this pattern constitutes a read of value of the scrutinee that
403-
/// it is matching against.
403+
/// it is matching against. This is used to determine whether we should
404+
/// perform `NeverToAny` coercions
404405
///
405406
/// See above for the nuances of what happens when this returns true.
406-
pub(super) fn pat_constitutes_read(&self, pat: &hir::Pat<'_>) -> bool {
407-
let mut does_read = false;
408-
pat.walk(|pat| {
409-
match pat.kind {
410-
hir::PatKind::Wild | hir::PatKind::Never | hir::PatKind::Or(_) => {
411-
// Recurse
412-
true
413-
}
414-
hir::PatKind::Binding(_, _, _, _)
415-
| hir::PatKind::Struct(_, _, _)
416-
| hir::PatKind::TupleStruct(_, _, _)
417-
| hir::PatKind::Path(_)
418-
| hir::PatKind::Tuple(_, _)
419-
| hir::PatKind::Box(_)
420-
| hir::PatKind::Ref(_, _)
421-
| hir::PatKind::Deref(_)
422-
| hir::PatKind::Lit(_)
423-
| hir::PatKind::Range(_, _, _)
424-
| hir::PatKind::Slice(_, _, _)
425-
| hir::PatKind::Err(_) => {
426-
does_read = true;
427-
// No need to continue.
428-
false
429-
}
430-
}
431-
});
432-
does_read
407+
pub(super) fn pat_guaranteed_to_constitute_read_for_never(&self, pat: &hir::Pat<'_>) -> bool {
408+
match pat.kind {
409+
// Does not constitute a read.
410+
hir::PatKind::Wild => false,
411+
412+
// This is unnecessarily restrictive when the pattern that doesn't
413+
// constitute a read is unreachable.
414+
//
415+
// For example `match *never_ptr { value => {}, _ => {} }` or
416+
// `match *never_ptr { _ if false => {}, value => {} }`.
417+
//
418+
// It is however fine to be restrictive here; only returning `true`
419+
// can lead to unsoundness.
420+
hir::PatKind::Or(subpats) => {
421+
subpats.iter().all(|pat| self.pat_guaranteed_to_constitute_read_for_never(pat))
422+
}
423+
424+
// Does constitute a read, since it is equivalent to a discriminant read.
425+
hir::PatKind::Never => true,
426+
427+
// All of these constitute a read, or match on something that isn't `!`,
428+
// which would require a `NeverToAny` coercion.
429+
hir::PatKind::Binding(_, _, _, _)
430+
| hir::PatKind::Struct(_, _, _)
431+
| hir::PatKind::TupleStruct(_, _, _)
432+
| hir::PatKind::Path(_)
433+
| hir::PatKind::Tuple(_, _)
434+
| hir::PatKind::Box(_)
435+
| hir::PatKind::Ref(_, _)
436+
| hir::PatKind::Deref(_)
437+
| hir::PatKind::Lit(_)
438+
| hir::PatKind::Range(_, _, _)
439+
| hir::PatKind::Slice(_, _, _)
440+
| hir::PatKind::Err(_) => true,
441+
}
433442
}
434443

435444
#[instrument(skip(self, expr), level = "debug")]

‎compiler/rustc_hir_typeck/src/pat.rs‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ use tracing::{debug, instrument, trace};
2828
use ty::VariantDef;
2929

3030
use super::report_unexpected_variant_res;
31-
use crate::diverges::Diverges;
3231
use crate::gather_locals::DeclOrigin;
3332
use crate::{errors, FnCtxt, LoweredTy};
3433

@@ -278,12 +277,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
278277
}
279278
};
280279

281-
// All other patterns constitute a read, which causes us to diverge
282-
// if the type is never.
283-
if ty.is_never() && self.pat_constitutes_read(pat) {
284-
self.diverges.set(self.diverges.get() | Diverges::always(pat.span));
285-
}
286-
287280
self.write_ty(pat.hir_id, ty);
288281

289282
// (note_1): In most of the cases where (note_1) is referenced

‎tests/mir-opt/uninhabited_enum.process_never.SimplifyLocals-final.after.mir‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ fn process_never(_1: *const !) -> () {
44
debug input => _1;
55
let mut _0: ();
66
scope 1 {
7-
debug _input => _1;
7+
debug _input => const ();
88
}
99

1010
bb0: {
11-
return;
11+
unreachable;
1212
}
1313
}

‎tests/mir-opt/uninhabited_enum.process_void.SimplifyLocals-final.after.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ fn process_void(_1: *const Void) -> () {
44
debug input => _1;
55
let mut _0: ();
66
scope 1 {
7-
debug _input => _1;
7+
debug _input => const ZeroSized: Void;
88
}
99

1010
bb0: {

‎tests/mir-opt/uninhabited_enum.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
// skip-filecheck
22
#![feature(never_type)]
33

4+
#[derive(Copy, Clone)]
45
pub enum Void {}
56

67
// EMIT_MIR uninhabited_enum.process_never.SimplifyLocals-final.after.mir
78
#[no_mangle]
89
pub fn process_never(input: *const !) {
9-
let _input = unsafe { &*input };
10+
let _input = unsafe { *input };
1011
}
1112

1213
// EMIT_MIR uninhabited_enum.process_void.SimplifyLocals-final.after.mir
1314
#[no_mangle]
1415
pub fn process_void(input: *const Void) {
15-
let _input = unsafe { &*input };
16+
let _input = unsafe { *input };
17+
// In the future, this should end with `unreachable`, but we currently only do
18+
// unreachability analysis for `!`.
1619
}
1720

1821
fn main() {}

‎tests/ui/never_type/diverging-place-match.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,23 @@ fn field_projection() -> ! {
4444
}
4545
}
4646

47+
fn covered_arm() -> ! {
48+
unsafe {
49+
//~^ ERROR mismatched types
50+
let x: *const ! = 0 as _;
51+
let (_ | 1i32) = *x;
52+
//~^ ERROR mismatched types
53+
}
54+
}
55+
56+
// FIXME: This *could* be considered a read of `!`, but we're not that sophisticated..
57+
fn uncovered_arm() -> ! {
58+
unsafe {
59+
//~^ ERROR mismatched types
60+
let x: *const ! = 0 as _;
61+
let (1i32 | _) = *x;
62+
//~^ ERROR mismatched types
63+
}
64+
}
65+
4766
fn main() {}

‎tests/ui/never_type/diverging-place-match.stderr‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,56 @@ LL | | }
7878
= note: expected type `!`
7979
found unit type `()`
8080

81-
error: aborting due to 6 previous errors
81+
error[E0308]: mismatched types
82+
--> $DIR/diverging-place-match.rs:51:18
83+
|
84+
LL | let (_ | 1i32) = *x;
85+
| ^^^^ -- this expression has type `!`
86+
| |
87+
| expected `!`, found `i32`
88+
|
89+
= note: expected type `!`
90+
found type `i32`
91+
92+
error[E0308]: mismatched types
93+
--> $DIR/diverging-place-match.rs:48:5
94+
|
95+
LL | / unsafe {
96+
LL | |
97+
LL | | let x: *const ! = 0 as _;
98+
LL | | let (_ | 1i32) = *x;
99+
LL | |
100+
LL | | }
101+
| |_____^ expected `!`, found `()`
102+
|
103+
= note: expected type `!`
104+
found unit type `()`
105+
106+
error[E0308]: mismatched types
107+
--> $DIR/diverging-place-match.rs:61:14
108+
|
109+
LL | let (1i32 | _) = *x;
110+
| ^^^^ -- this expression has type `!`
111+
| |
112+
| expected `!`, found `i32`
113+
|
114+
= note: expected type `!`
115+
found type `i32`
116+
117+
error[E0308]: mismatched types
118+
--> $DIR/diverging-place-match.rs:58:5
119+
|
120+
LL | / unsafe {
121+
LL | |
122+
LL | | let x: *const ! = 0 as _;
123+
LL | | let (1i32 | _) = *x;
124+
LL | |
125+
LL | | }
126+
| |_____^ expected `!`, found `()`
127+
|
128+
= note: expected type `!`
129+
found unit type `()`
130+
131+
error: aborting due to 10 previous errors
82132

83133
For more information about this error, try `rustc --explain E0308`.

‎tests/ui/raw-ref-op/never-place-isnt-diverging.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,13 @@ fn make_up_a_value<T>() -> T {
1010
}
1111
}
1212

13+
14+
fn make_up_a_pointer<T>() -> *const T {
15+
unsafe {
16+
let x: *const ! = 0 as _;
17+
&raw const *x
18+
//~^ ERROR mismatched types
19+
}
20+
}
21+
1322
fn main() {}

‎tests/ui/raw-ref-op/never-place-isnt-diverging.stderr‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,20 @@ LL | | }
1515
= note: expected type parameter `T`
1616
found unit type `()`
1717

18-
error: aborting due to 1 previous error
18+
error[E0308]: mismatched types
19+
--> $DIR/never-place-isnt-diverging.rs:17:9
20+
|
21+
LL | fn make_up_a_pointer<T>() -> *const T {
22+
| - -------- expected `*const T` because of return type
23+
| |
24+
| expected this type parameter
25+
...
26+
LL | &raw const *x
27+
| ^^^^^^^^^^^^^ expected `*const T`, found `*const !`
28+
|
29+
= note: expected raw pointer `*const T`
30+
found raw pointer `*const !`
31+
32+
error: aborting due to 2 previous errors
1933

2034
For more information about this error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)