Skip to content
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.

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

Make typeck aware of uninhabited types #108993

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 8 additions & 10 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_trait_selection::traits::{
use tracing::{debug, instrument};

use crate::coercion::{AsCoercionSite, CoerceMany};
use crate::{Diverges, Expectation, FnCtxt, Needs};
use crate::{DivergeReason, Diverges, Expectation, FnCtxt, Needs};

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
#[instrument(skip(self), level = "debug", ret)]
Expand All @@ -31,7 +31,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// If there are no arms, that is a diverging match; a special case.
if arms.is_empty() {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
self.diverges
.set(self.diverges.get() | Diverges::Always(DivergeReason::Other, expr.span));
return tcx.types.never;
}

Expand Down Expand Up @@ -150,13 +151,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// we can emit a better note. Rather than pointing
// at a diverging expression in an arbitrary arm,
// we can point at the entire `match` expression
if let (Diverges::Always { .. }, hir::MatchSource::Normal) = (all_arms_diverge, match_src) {
all_arms_diverge = Diverges::Always {
span: expr.span,
custom_note: Some(
"any code following this `match` expression is unreachable, as all arms diverge",
),
};
if let (Diverges::Always(..), hir::MatchSource::Normal) = (all_arms_diverge, match_src) {
all_arms_diverge = Diverges::Always(DivergeReason::AllArmsDiverge, expr.span);
}

// We won't diverge unless the scrutinee or all arms diverge.
Expand Down Expand Up @@ -253,7 +249,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// warn the user about the match arms being unreachable.
fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm<'tcx>]) {
for arm in arms {
self.warn_if_unreachable(arm.body.hir_id, arm.body.span, "arm");
if !arm.pat.is_never_pattern() {
self.warn_if_unreachable(arm.body.hir_id, arm.body.span, "arm");
}
}
}

Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_hir_typeck/src/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use tracing::{debug, instrument};

use crate::coercion::CoerceMany;
use crate::gather_locals::GatherLocalsVisitor;
use crate::{CoroutineTypes, Diverges, FnCtxt};
use crate::{CoroutineTypes, DivergeReason, Diverges, FnCtxt};

/// Helper used for fns and closures. Does the grungy work of checking a function
/// body and returns the function context used for that purpose, since in the case of a fn item
Expand Down Expand Up @@ -89,10 +89,8 @@ pub(super) fn check_fn<'a, 'tcx>(
let ty_span = ty.map(|ty| ty.span);
fcx.check_pat_top(param.pat, param_ty, ty_span, None, None);
if param.pat.is_never_pattern() {
fcx.function_diverges_because_of_empty_arguments.set(Diverges::Always {
span: param.pat.span,
custom_note: Some("any code following a never pattern is unreachable"),
});
fcx.function_diverges_because_of_empty_arguments
.set(Diverges::Always(DivergeReason::NeverPattern, param.pat.span));
}

// Check that argument is Sized.
Expand Down
62 changes: 36 additions & 26 deletions compiler/rustc_hir_typeck/src/diverges.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,30 @@
use std::{cmp, ops};

use rustc_span::{DUMMY_SP, Span};
use rustc_hir::HirId;
use rustc_span::Span;

/// Tracks whether executing a node may exit normally (versus
/// return/break/panic, which "diverge", leaving dead code in their
/// wake). Tracked semi-automatically (through type variables marked
/// as diverging), with some manual adjustments for control-flow
/// primitives (approximating a CFG).
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
#[derive(Copy, Clone, Debug)]
pub(crate) enum Diverges {
/// Potentially unknown, some cases converge,
/// others require a CFG to determine them.
Maybe,

/// This expression is uninhabited, we want to
/// emit a diagnostic but not pollute type checking.
UninhabitedExpr(HirId, Span),

/// Same as `UninhabitedExpr` but with reachability
/// warning already emitted.
Warned,

/// Definitely known to diverge and therefore
/// not reach the next sibling or its parent.
Always {
/// The `Span` points to the expression
/// that caused us to diverge
/// (e.g. `return`, `break`, etc).
span: Span,
/// In some cases (e.g. a `match` expression
/// where all arms diverge), we may be
/// able to provide a more informative
/// message to the user.
/// If this is `None`, a default message
/// will be generated, which is suitable
/// for most cases.
custom_note: Option<&'static str>,
},
Always(DivergeReason, Span),

/// Same as `Always` but with a reachability
/// warning already emitted.
Expand All @@ -40,14 +36,15 @@ pub(crate) enum Diverges {
impl ops::BitAnd for Diverges {
type Output = Self;
fn bitand(self, other: Self) -> Self {
cmp::min(self, other)
cmp::min_by_key(self, other, Self::ordinal)
}
}

impl ops::BitOr for Diverges {
type Output = Self;
fn bitor(self, other: Self) -> Self {
cmp::max(self, other)
// argument order is to prefer `self` if ordinal is equal
cmp::max_by_key(other, self, Self::ordinal)
}
}

Expand All @@ -64,15 +61,28 @@ impl ops::BitOrAssign for Diverges {
}

impl Diverges {
/// Creates a `Diverges::Always` with the provided `span` and the default note message.
pub(super) fn always(span: Span) -> Diverges {
Diverges::Always { span, custom_note: None }
pub(super) fn is_always(self) -> bool {
match self {
Self::Maybe | Diverges::UninhabitedExpr(..) | Diverges::Warned => false,
Self::Always(..) | Self::WarnedAlways => true,
}
}

pub(super) fn is_always(self) -> bool {
// Enum comparison ignores the
// contents of fields, so we just
// fill them in with garbage here.
self >= Diverges::Always { span: DUMMY_SP, custom_note: None }
fn ordinal(&self) -> u8 {
match self {
Self::Maybe => 0,
Self::UninhabitedExpr(..) => 1,
Self::Warned => 2,
Self::Always { .. } => 3,
Self::WarnedAlways => 4,
}
}
}

#[derive(Clone, Copy, Debug)]
pub(crate) enum DivergeReason {
AllArmsDiverge,
NeverPattern,
UninhabitedExpr(HirId),
Other,
}
66 changes: 58 additions & 8 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ use crate::errors::{
YieldExprOutsideOfCoroutine,
};
use crate::{
BreakableCtxt, CoroutineTypes, Diverges, FnCtxt, Needs, cast, fatally_break_rust,
report_unexpected_variant_res, type_error_struct,
BreakableCtxt, CoroutineTypes, DivergeReason, Diverges, FnCtxt, Needs, cast,
fatally_break_rust, report_unexpected_variant_res, type_error_struct,
};

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -234,19 +234,41 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// diverging expression (e.g. it arose from desugaring of `try { return }`),
// we skip issuing a warning because it is autogenerated code.
ExprKind::Call(..) if expr.span.is_desugaring(DesugaringKind::TryBlock) => {}
ExprKind::Call(callee, _) => self.warn_if_unreachable(expr.hir_id, callee.span, "call"),
ExprKind::Call(callee, _) => {
// Do not emit a warning for a call to a constructor.
let emit_warning = if let ExprKind::Path(ref qpath) = callee.kind {
let res = self.typeck_results.borrow().qpath_res(qpath, callee.hir_id);
!matches!(res, Res::Def(DefKind::Ctor(..), _))
} else {
true
};
if emit_warning {
self.warn_if_unreachable(expr.hir_id, callee.span, "call")
}
}
ExprKind::MethodCall(segment, ..) => {
self.warn_if_unreachable(expr.hir_id, segment.ident.span, "call")
}
// Allow field access when the struct is uninhabited.
ExprKind::Field(..)
if matches!(self.diverges.get(), Diverges::UninhabitedExpr(_, _)) => {}
_ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression"),
}

// 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_guaranteed_to_constitute_read_for_never(expr) {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
let cur_diverges = self.diverges.get();
if !cur_diverges.is_always() && self.expr_guaranteed_to_constitute_read_for_never(expr) {
if ty.is_never() {
// Any expression that produces a value of type `!` must have diverged.
self.diverges.set(cur_diverges | Diverges::Always(DivergeReason::Other, expr.span));
} else if self.ty_is_uninhabited(ty) {
// This expression produces a value of uninhabited type.
// This means it has diverged somehow.
self.diverges.set(cur_diverges | Diverges::UninhabitedExpr(expr.hir_id, expr.span));
}
}

// Record the type, which applies it effects.
Expand Down Expand Up @@ -352,6 +374,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.pat_guaranteed_to_constitute_read_for_never(*pat)
}

hir::Node::Pat(parent_pat) => match parent_pat.kind {
hir::PatKind::Lit(..) | hir::PatKind::Range(..) => false,

// These nodes do not have direct sub-exprs.
hir::PatKind::Wild
| hir::PatKind::Binding(..)
| hir::PatKind::Struct(..)
| hir::PatKind::TupleStruct(..)
| hir::PatKind::Or(..)
| hir::PatKind::Never
| hir::PatKind::Path(..)
| hir::PatKind::Tuple(..)
| hir::PatKind::Box(..)
| hir::PatKind::Deref(..)
| hir::PatKind::Ref(..)
| hir::PatKind::Slice(..)
| hir::PatKind::Err(..) => {
unreachable!("no sub-expr expected for {parent_pat:?}")
}
},

// These nodes (if they have a sub-expr) do constitute a read.
hir::Node::Block(_)
| hir::Node::Arm(_)
Expand Down Expand Up @@ -381,7 +424,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
| hir::Node::Ty(_)
| hir::Node::AssocItemConstraint(_)
| hir::Node::TraitRef(_)
| hir::Node::Pat(_)
| hir::Node::PatField(_)
| hir::Node::LetStmt(_)
| hir::Node::Synthetic
Expand Down Expand Up @@ -442,6 +484,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn ty_is_uninhabited(&self, ty: Ty<'tcx>) -> bool {
let ty = self.resolve_vars_if_possible(ty);
// Freshen the type as `is_inhabited_from` may call a query on `ty`.
let ty = self.freshen(ty);
!ty.is_inhabited_from(self.tcx, self.parent_module, self.param_env)
}

#[instrument(skip(self, expr), level = "debug")]
fn check_expr_kind(
&self,
Expand Down Expand Up @@ -1507,7 +1556,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// of a `break` or an outer `break` or `return`.
self.diverges.set(Diverges::Maybe);
} else {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
self.diverges
.set(self.diverges.get() | Diverges::Always(DivergeReason::Other, expr.span));
}

// If we permit break with a value, then result type is
Expand Down Expand Up @@ -1610,7 +1660,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
.unwrap_or_else(|| self.next_ty_var(expr.span));
let mut coerce = CoerceMany::with_coercion_sites(coerce_to, args);
assert_eq!(self.diverges.get(), Diverges::Maybe);
assert!(matches!(self.diverges.get(), Diverges::Maybe));
for e in args {
let e_ty = self.check_expr_with_hint(e, coerce_to);
let cause = self.misc(e.span);
Expand Down
51 changes: 42 additions & 9 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::collections::hash_map::Entry;
use std::slice;

Expand Down Expand Up @@ -31,7 +32,7 @@ use rustc_session::lint;
use rustc_span::Span;
use rustc_span::def_id::LocalDefId;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::symbol::kw;
use rustc_span::symbol::{kw, sym};
use rustc_target::abi::FieldIdx;
use rustc_trait_selection::error_reporting::infer::need_type_info::TypeAnnotationNeeded;
use rustc_trait_selection::traits::{
Expand All @@ -42,14 +43,20 @@ use tracing::{debug, instrument};
use crate::callee::{self, DeferredCallResolution};
use crate::errors::{self, CtorIsPrivate};
use crate::method::{self, MethodCallee};
use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy, rvalue_scopes};
use crate::{
BreakableCtxt, DivergeReason, Diverges, Expectation, FnCtxt, LoweredTy, rvalue_scopes,
};

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Produces warning on the given node, if the current point in the
/// function is unreachable, and there hasn't been another warning.
pub(crate) fn warn_if_unreachable(&self, id: HirId, span: Span, kind: &str) {
let Diverges::Always { span: orig_span, custom_note } = self.diverges.get() else {
return;
let (reason, orig_span) = match self.diverges.get() {
Diverges::UninhabitedExpr(hir_id, orig_span) => {
(DivergeReason::UninhabitedExpr(hir_id), orig_span)
}
Diverges::Always(reason, orig_span) => (reason, orig_span),
Diverges::Maybe | Diverges::Warned | Diverges::WarnedAlways => return,
};

match span.desugaring_kind() {
Expand All @@ -71,18 +78,44 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
_ => {}
}

if matches!(reason, DivergeReason::UninhabitedExpr(_)) {
if let Some(impl_of) = self.tcx.impl_of_method(self.body_id.to_def_id()) {
if self.tcx.has_attr(impl_of, sym::automatically_derived) {
// Built-in derives are generated before typeck,
// so they may contain unreachable code if there are uninhabited types
return;
}
}
}

// Don't warn twice.
self.diverges.set(Diverges::WarnedAlways);
self.diverges.set(match self.diverges.get() {
Diverges::UninhabitedExpr(..) => Diverges::Warned,
Diverges::Always(..) => Diverges::WarnedAlways,
Diverges::Maybe | Diverges::Warned | Diverges::WarnedAlways => bug!(),
});

debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind);

let msg = format!("unreachable {kind}");
self.tcx().node_span_lint(lint::builtin::UNREACHABLE_CODE, id, span, |lint| {
lint.primary_message(msg.clone());
lint.span_label(span, msg).span_label(
orig_span,
custom_note.unwrap_or("any code following this expression is unreachable"),
);
let custom_note: Cow<'_, _> = match reason {
DivergeReason::AllArmsDiverge => {
"any code following this `match` expression is unreachable, as all arms diverge"
.into()
}
DivergeReason::NeverPattern => {
"any code following a never pattern is unreachable".into()
}
DivergeReason::UninhabitedExpr(hir_id) => format!(
"this expression has type `{}`, which is uninhabited",
self.typeck_results.borrow().node_type(hir_id)
)
.into(),
DivergeReason::Other => "any code following this expression is unreachable".into(),
};
lint.span_label(span, msg).span_label(orig_span, custom_note);
})
}

Expand Down
Loading
Loading