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 #100288

Closed
wants to merge 10 commits into from
Closed
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
13 changes: 4 additions & 9 deletions compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::coercion::{AsCoercionSite, CoerceMany};
use crate::{Diverges, Expectation, FnCtxt, Needs};
use crate::{DivergeReason, Diverges, Expectation, FnCtxt, Needs};
use rustc_errors::{Applicability, MultiSpan};
use rustc_hir::{self as hir, ExprKind};
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
Expand Down Expand Up @@ -29,7 +29,7 @@ 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));
return tcx.types.never;
}

Expand Down Expand Up @@ -204,13 +204,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);
}

// We won't diverge unless the scrutinee or all arms diverge.
Expand Down
64 changes: 31 additions & 33 deletions compiler/rustc_hir_typeck/src/diverges.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,21 @@
use rustc_span::source_map::DUMMY_SP;
use rustc_span::{self, Span};
use rustc_hir as hir;

use std::{cmp, ops};

/// 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)]
pub enum Diverges {
#[derive(Copy, Clone, Debug)]
pub enum Diverges<'a> {
/// Potentially unknown, some cases converge,
/// others require a CFG to determine them.
Maybe,

/// 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, &'a hir::Expr<'a>),

/// Same as `Always` but with a reachability
/// warning already emitted.
Expand All @@ -37,42 +24,53 @@ pub enum Diverges {

// Convenience impls for combining `Diverges`.

impl ops::BitAnd for 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 {
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)
}
}

impl ops::BitAndAssign for Diverges {
impl ops::BitAndAssign for Diverges<'_> {
fn bitand_assign(&mut self, other: Self) {
*self = *self & other;
}
}

impl ops::BitOrAssign for Diverges {
impl ops::BitOrAssign for Diverges<'_> {
fn bitor_assign(&mut self, other: Self) {
*self = *self | other;
}
}

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 }
impl Diverges<'_> {
pub(super) fn is_always(self) -> bool {
match self {
Self::Maybe => 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::Always { .. } => 1,
Self::WarnedAlways => 2,
}
}
}

#[derive(Clone, Copy, Debug)]
pub enum DivergeReason {
AllArmsDiverge,
Uninhabited,
Other,
}
66 changes: 62 additions & 4 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::method::SelfSource;
use crate::type_error_struct;
use crate::Expectation::{self, ExpectCastableToType, ExpectHasType, NoExpectation};
use crate::{
report_unexpected_variant_res, BreakableCtxt, Diverges, FnCtxt, Needs,
report_unexpected_variant_res, BreakableCtxt, DivergeReason, Diverges, FnCtxt, Needs,
TupleArgumentsFlag::DontTupleArguments,
};
use rustc_ast as ast;
Expand Down Expand Up @@ -244,12 +244,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::MethodCall(segment, ..) => {
self.warn_if_unreachable(expr.hir_id, segment.ident.span, "call")
}
// allow field access when the struct and the field are both uninhabited
ExprKind::Field(..)
if matches!(
self.diverges.get(),
Diverges::Always(DivergeReason::Uninhabited, _)
) && self.tcx.is_ty_uninhabited_from(
self.parent_module,
self.freshen(ty),
self.param_env,
) => {}
_ => 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() {
self.diverges.set(self.diverges.get() | Diverges::always(expr.span));
if !self.diverges.get().is_always() {
if ty.is_never() {
self.diverges.set(Diverges::Always(DivergeReason::Other, expr));
} else if expr_may_be_uninhabited(expr)
&& self.tcx.is_ty_uninhabited_from(
self.parent_module,
self.freshen(ty),
self.param_env,
)
{
self.diverges.set(Diverges::Always(DivergeReason::Uninhabited, expr));
}
}

// Record the type, which applies it effects.
Expand Down Expand Up @@ -1306,7 +1326,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
})
});
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 Expand Up @@ -2886,3 +2906,41 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}

fn expr_may_be_uninhabited(expr: &hir::Expr<'_>) -> bool {
match expr.kind {
ExprKind::Call(..)
| ExprKind::MethodCall(..)
| ExprKind::Cast(..)
| ExprKind::Unary(hir::UnOp::Deref, _)
| ExprKind::Field(..)
| ExprKind::Path(..)
| ExprKind::Struct(..) => true,
ExprKind::Box(..)
| ExprKind::ConstBlock(..)
| ExprKind::Array(..)
| ExprKind::Tup(..)
| ExprKind::Binary(..)
| ExprKind::Unary(hir::UnOp::Neg | hir::UnOp::Not, _)
| ExprKind::Lit(..)
| ExprKind::Type(..)
| ExprKind::DropTemps(..)
| ExprKind::Let(..)
| ExprKind::If(..)
| ExprKind::Loop(..)
| ExprKind::Match(..)
| ExprKind::Closure(..)
| ExprKind::Block(..)
| ExprKind::Assign(..)
| ExprKind::AssignOp(..)
| ExprKind::Index(..)
| ExprKind::AddrOf(..)
| ExprKind::Break(..)
| ExprKind::Continue(..)
| ExprKind::Ret(..)
| ExprKind::InlineAsm(..)
| ExprKind::Repeat(..)
| ExprKind::Yield(..)
| ExprKind::Err => false,
}
}
66 changes: 39 additions & 27 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::callee::{self, DeferredCallResolution};
use crate::method::{self, MethodCallee, SelfSource};
use crate::rvalue_scopes;
use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LocalTy};
use crate::{BreakableCtxt, DivergeReason, Diverges, Expectation, FnCtxt, LocalTy};
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, Diagnostic, ErrorGuaranteed, MultiSpan};
Expand Down Expand Up @@ -36,43 +36,55 @@ use rustc_trait_selection::traits::{
self, ObligationCause, ObligationCauseCode, TraitEngine, TraitEngineExt,
};

use std::borrow::Cow;
use std::collections::hash_map::Entry;
use std::slice;

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(in super::super) fn warn_if_unreachable(&self, id: hir::HirId, span: Span, kind: &str) {
// FIXME: Combine these two 'if' expressions into one once
// let chains are implemented
if let Diverges::Always { span: orig_span, custom_note } = self.diverges.get() {
// If span arose from a desugaring of `if` or `while`, then it is the condition itself,
// which diverges, that we are about to lint on. This gives suboptimal diagnostics.
// Instead, stop here so that the `if`- or `while`-expression's block is linted instead.
if !span.is_desugaring(DesugaringKind::CondTemporary)
&& !span.is_desugaring(DesugaringKind::Async)
&& !orig_span.is_desugaring(DesugaringKind::Await)
{
self.diverges.set(Diverges::WarnedAlways);
let Diverges::Always(reason, diverging_expr) = self.diverges.get() else {
return;
};
// If span arose from a desugaring of `if` or `while`, then it is the condition itself,
// which diverges, that we are about to lint on. This gives suboptimal diagnostics.
// Instead, stop here so that the `if`- or `while`-expression's block is linted instead.
if matches!(
span.desugaring_kind(),
Some(DesugaringKind::Async | DesugaringKind::Await | DesugaringKind::CondTemporary)
) {
return;
}

debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind);
self.diverges.set(Diverges::WarnedAlways);

let msg = format!("unreachable {}", kind);
self.tcx().struct_span_lint_hir(
lint::builtin::UNREACHABLE_CODE,
id,
span,
&msg,
|lint| {
lint.span_label(span, &msg).span_label(
orig_span,
custom_note
.unwrap_or("any code following this expression is unreachable"),
)
},
)
if matches!(reason, DivergeReason::Uninhabited) {
let def_id = self.tcx.hir().body_owner_def_id(hir::BodyId { hir_id: self.body_id });
if let Some(impl_of) = self.tcx.impl_of_method(def_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;
}
}
}

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

let msg = format!("unreachable {}", kind);
self.tcx().struct_span_lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg, |lint| {
let label = match reason {
DivergeReason::AllArmsDiverge =>
Cow::Borrowed("any code following this `match` expression is unreachable, as all arms diverge"),
DivergeReason::Uninhabited => format!(
"this expression has type `{}`, which is uninhabited",
self.typeck_results.borrow().node_type(diverging_expr.hir_id)
).into(),
DivergeReason::Other => Cow::Borrowed("any code following this expression is unreachable"),
};
lint.span_label(span, &msg).span_label(diverging_expr.span, label)
});
}

/// Resolves type and const variables in `ty` if possible. Unlike the infcx
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ pub struct FnCtxt<'a, 'tcx> {
/// eventually).
pub(super) param_env: ty::ParamEnv<'tcx>,

pub(super) parent_module: DefId,

/// Number of errors that had been reported when we started
/// checking this function. On exit, if we find that *more* errors
/// have been reported, we will skip regionck and other work that
Expand Down Expand Up @@ -110,7 +112,7 @@ pub struct FnCtxt<'a, 'tcx> {
///
/// An expression represents dead code if, after checking it,
/// the diverges flag is set to something other than `Maybe`.
pub(super) diverges: Cell<Diverges>,
pub(super) diverges: Cell<Diverges<'tcx>>,

/// Whether any child nodes have any type errors.
pub(super) has_errors: Cell<bool>,
Expand Down Expand Up @@ -138,6 +140,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
FnCtxt {
body_id,
param_env,
parent_module: inh.tcx.parent_module(body_id).to_def_id(),
err_count_on_creation: inh.tcx.sess.err_count(),
ret_coercion: None,
in_tail_expr: false,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ mod rvalue_scopes;
mod upvar;
mod writeback;

pub use diverges::Diverges;
pub use diverges::{DivergeReason, Diverges};
pub use expectation::Expectation;
pub use fn_ctxt::*;
pub use inherited::{Inherited, InheritedBuilder};
Expand Down
Loading