Skip to content

fix the check for which expressions read never type #149545

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix the check for which expressions read never type #149545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62f7cf4
move `expr_guaranteed_to_constitute_read_for_never` and `pat_guarante…
WaffleLapkin Dec 2, 2025
784b283
add a test for nested type ascriptions of never type
WaffleLapkin Dec 2, 2025
c5f15bd
fix the check for which expressions read never type
WaffleLapkin Dec 2, 2025
ef27140
Move `tests/ui/{unreachable-code => reachable}`
WaffleLapkin Dec 2, 2025
4736576
add one more test for type ascription of never
WaffleLapkin Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1793,6 +1793,54 @@ impl<'hir> Pat<'hir> {
});
is_never_pattern
}

/// Whether this pattern constitutes a read of value of the scrutinee that
/// it is matching against. This is used to determine whether we should
/// perform `NeverToAny` coercions.
///
/// See [`expr_guaranteed_to_constitute_read_for_never`][m] for the nuances of
/// what happens when this returns true.
///
/// [m]: ../../rustc_middle/ty/struct.TyCtxt.html#method.expr_guaranteed_to_constitute_read_for_never
pub fn is_guaranteed_to_constitute_read_for_never(&self) -> bool {
match self.kind {
// Does not constitute a read.
PatKind::Wild => false,

// Might not constitute a read, since the condition might be false.
PatKind::Guard(_, _) => true,
Comment on lines +1810 to +1811
Copy link
Member

@jackh726 jackh726 Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A drive-by thought: this comment doesn't seem to match the function? From the comment, the expectation is false, given the function is guaranteed but this comment just it's not guaranteed.


// 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.
PatKind::Or(subpats) => {
subpats.iter().all(|pat| pat.is_guaranteed_to_constitute_read_for_never())
}

// Does constitute a read, since it is equivalent to a discriminant read.
PatKind::Never => true,

// All of these constitute a read, or match on something that isn't `!`,
// which would require a `NeverToAny` coercion.
PatKind::Missing
| PatKind::Binding(_, _, _, _)
| PatKind::Struct(_, _, _)
| PatKind::TupleStruct(_, _, _)
| PatKind::Tuple(_, _)
| PatKind::Box(_)
| PatKind::Ref(_, _, _)
| PatKind::Deref(_)
| PatKind::Expr(_)
| PatKind::Range(_, _, _)
| PatKind::Slice(_, _, _)
| PatKind::Err(_) => true,
}
}
}

/// A single field in a struct pattern.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/coercion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1078,7 +1078,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self,
cause,
allow_two_phase,
self.expr_guaranteed_to_constitute_read_for_never(expr),
self.tcx.expr_guaranteed_to_constitute_read_for_never(expr),
);
let ok = self.commit_if_ok(|_| coerce.coerce(source, target))?;

Expand Down
197 changes: 2 additions & 195 deletions compiler/rustc_hir_typeck/src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// While we don't allow *arbitrary* coercions here, we *do* allow
// coercions from ! to `expected`.
if self.try_structurally_resolve_type(expr.span, ty).is_never()
&& self.expr_guaranteed_to_constitute_read_for_never(expr)
&& self.tcx.expr_guaranteed_to_constitute_read_for_never(expr)
{
if let Some(adjustments) = self.typeck_results.borrow().adjustments().get(expr.hir_id) {
let reported = self.dcx().span_delayed_bug(
Expand Down Expand Up @@ -320,7 +320,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// diverging would be unsound since we may never actually read the `!`.
// e.g. `let _ = *never_ptr;` with `never_ptr: *const !`.
if self.try_structurally_resolve_type(expr.span, ty).is_never()
&& self.expr_guaranteed_to_constitute_read_for_never(expr)
&& self.tcx.expr_guaranteed_to_constitute_read_for_never(expr)
{
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
}
Expand All @@ -339,199 +339,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ty
}

/// Whether this expression constitutes a read of value of the type that
/// it evaluates to.
///
/// This is used to determine if we should consider the block to diverge
/// if the expression evaluates to `!`, and if we should insert a `NeverToAny`
/// coercion for values of type `!`.
///
/// This function generally returns `false` if the expression is a place
/// 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.
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
// 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;
}

let parent_node = self.tcx.parent_hir_node(expr.hir_id);
match parent_node {
hir::Node::Expr(parent_expr) => {
match parent_expr.kind {
// 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.
ExprKind::AddrOf(..) | hir::ExprKind::Field(..) => false,

// Place-preserving expressions only constitute reads if their
// parent expression constitutes a read.
ExprKind::Type(..) | ExprKind::UnsafeBinderCast(..) => {
self.expr_guaranteed_to_constitute_read_for_never(expr)
}

ExprKind::Assign(lhs, _, _) => {
// Only the LHS does not constitute a read
expr.hir_id != lhs.hir_id
}

// 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()
.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_guaranteed_to_constitute_read_for_never(*pat)
}

// Any expression child of these expressions constitute reads.
ExprKind::Array(_)
| ExprKind::Call(_, _)
| ExprKind::Use(_, _)
| ExprKind::MethodCall(_, _, _, _)
| ExprKind::Tup(_)
| ExprKind::Binary(_, _, _)
| ExprKind::Unary(_, _)
| ExprKind::Cast(_, _)
| ExprKind::DropTemps(_)
| ExprKind::If(_, _, _)
| ExprKind::Closure(_)
| ExprKind::Block(_, _)
| ExprKind::AssignOp(_, _, _)
| ExprKind::Index(_, _, _)
| ExprKind::Break(_, _)
| ExprKind::Ret(_)
| ExprKind::Become(_)
| ExprKind::InlineAsm(_)
| ExprKind::Struct(_, _, _)
| ExprKind::Repeat(_, _)
| ExprKind::Yield(_, _) => true,

// These expressions have no (direct) sub-exprs.
ExprKind::ConstBlock(_)
| ExprKind::Loop(_, _, _, _)
| ExprKind::Lit(_)
| ExprKind::Path(_)
| ExprKind::Continue(_)
| ExprKind::OffsetOf(_, _)
| ExprKind::Err(_) => unreachable!("no sub-expr expected for {:?}", expr.kind),
}
}

// 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, .. }) => {
assert_eq!(target.hir_id, expr.hir_id);
self.pat_guaranteed_to_constitute_read_for_never(*pat)
}

// These nodes (if they have a sub-expr) do constitute a read.
hir::Node::Block(_)
| hir::Node::Arm(_)
| hir::Node::ExprField(_)
| hir::Node::AnonConst(_)
| hir::Node::ConstBlock(_)
| hir::Node::ConstArg(_)
| hir::Node::Stmt(_)
| hir::Node::Item(hir::Item {
kind: hir::ItemKind::Const(..) | hir::ItemKind::Static(..),
..
})
| hir::Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Const(..), ..
})
| hir::Node::ImplItem(hir::ImplItem { kind: hir::ImplItemKind::Const(..), .. }) => true,

hir::Node::TyPat(_) | hir::Node::Pat(_) => {
self.dcx().span_delayed_bug(expr.span, "place expr not allowed in pattern");
true
}

// These nodes do not have direct sub-exprs.
hir::Node::Param(_)
| hir::Node::Item(_)
| hir::Node::ForeignItem(_)
| hir::Node::TraitItem(_)
| hir::Node::ImplItem(_)
| hir::Node::Variant(_)
| hir::Node::Field(_)
| hir::Node::PathSegment(_)
| hir::Node::Ty(_)
| hir::Node::AssocItemConstraint(_)
| hir::Node::TraitRef(_)
| hir::Node::PatField(_)
| hir::Node::PatExpr(_)
| hir::Node::LetStmt(_)
| hir::Node::Synthetic
| hir::Node::Err(_)
| hir::Node::Ctor(_)
| hir::Node::Lifetime(_)
| hir::Node::GenericParam(_)
| hir::Node::Crate(_)
| hir::Node::Infer(_)
| hir::Node::WherePredicate(_)
| hir::Node::PreciseCapturingNonLifetimeArg(_)
| hir::Node::OpaqueTy(_) => {
unreachable!("no sub-expr expected for {parent_node:?}")
}
}
}

/// Whether this pattern constitutes a read of value of the scrutinee that
/// 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_guaranteed_to_constitute_read_for_never(&self, pat: &hir::Pat<'_>) -> bool {
match pat.kind {
// Does not constitute a read.
hir::PatKind::Wild => false,

// Might not constitute a read, since the condition might be false.
hir::PatKind::Guard(_, _) => true,

// 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 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::Missing
| hir::PatKind::Binding(_, _, _, _)
| hir::PatKind::Struct(_, _, _)
| hir::PatKind::TupleStruct(_, _, _)
| hir::PatKind::Tuple(_, _)
| hir::PatKind::Box(_)
| hir::PatKind::Ref(_, _, _)
| hir::PatKind::Deref(_)
| hir::PatKind::Expr(_)
| hir::PatKind::Range(_, _, _)
| hir::PatKind::Slice(_, _, _)
| hir::PatKind::Err(_) => true,
}
}

#[instrument(skip(self, expr), level = "debug")]
fn check_expr_kind(
&self,
Expand Down
Loading
Loading