Skip to content

Commit

Permalink
Lint uninhabited types in typeck
Browse files Browse the repository at this point in the history
  • Loading branch information
camsteffen authored and cjgillot committed Oct 30, 2024
1 parent f6e7f7c commit b777188
Show file tree
Hide file tree
Showing 24 changed files with 197 additions and 167 deletions.
4 changes: 3 additions & 1 deletion compiler/rustc_hir_typeck/src/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,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
18 changes: 15 additions & 3 deletions compiler/rustc_hir_typeck/src/diverges.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::{cmp, ops};

use rustc_hir::HirId;
use rustc_span::Span;

/// Tracks whether executing a node may exit normally (versus
Expand All @@ -13,6 +14,14 @@ pub(crate) enum Diverges {
/// 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(DivergeReason, Span),
Expand Down Expand Up @@ -54,16 +63,18 @@ impl ops::BitOrAssign for Diverges {
impl Diverges {
pub(super) fn is_always(self) -> bool {
match self {
Self::Maybe => false,
Self::Maybe | Diverges::UninhabitedExpr(..) | Diverges::Warned => false,
Self::Always(..) | Self::WarnedAlways => true,
}
}

fn ordinal(&self) -> u8 {
match self {
Self::Maybe => 0,
Self::Always { .. } => 1,
Self::WarnedAlways => 2,
Self::UninhabitedExpr(..) => 1,
Self::Warned => 2,
Self::Always { .. } => 3,
Self::WarnedAlways => 4,
}
}
}
Expand All @@ -72,5 +83,6 @@ impl Diverges {
pub(crate) enum DivergeReason {
AllArmsDiverge,
NeverPattern,
UninhabitedExpr(HirId),
Other,
}
58 changes: 53 additions & 5 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,20 +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(DivergeReason::Other, 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 @@ -353,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 @@ -382,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 @@ -443,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
41 changes: 34 additions & 7 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 @@ -50,8 +51,12 @@ 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(reason, orig_span) = 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 @@ -73,20 +78,42 @@ 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());
let custom_note = match reason {
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::NeverPattern => "any code following a never pattern is unreachable",
DivergeReason::Other => "any code following this expression is unreachable",
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
6 changes: 6 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ pub(crate) struct FnCtxt<'a, 'tcx> {
/// eventually).
pub(super) param_env: ty::ParamEnv<'tcx>,

/// The module in which the current function is defined. This
/// is used to compute type inhabitedness, which accounts for
/// visibility information.
pub(super) parent_module: DefId,

/// If `Some`, this stores coercion information for returned
/// expressions. If `None`, this is in a context where return is
/// inappropriate, such as a const expression.
Expand Down Expand Up @@ -127,6 +132,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
FnCtxt {
body_id,
param_env,
parent_module: root_ctxt.tcx.parent_module_from_def_id(body_id).to_def_id(),
ret_coercion: None,
ret_coercion_span: Cell::new(None),
coroutine_types: None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,27 +92,7 @@ impl<'tcx> InhabitedPredicate<'tcx> {
Self::NotInModule(id) => in_module(id).map(|in_mod| !in_mod),
// `t` may be a projection, for which `inhabited_predicate` returns a `GenericType`. As
// we have a param_env available, we can do better.
Self::GenericType(t) => {
let normalized_pred = tcx
.try_normalize_erasing_regions(param_env, t)
.map_or(self, |t| t.inhabited_predicate(tcx));
match normalized_pred {
// We don't have more information than we started with, so consider inhabited.
Self::GenericType(_) => Ok(true),
pred => {
// A type which is cyclic when monomorphized can happen here since the
// layout error would only trigger later. See e.g. `tests/ui/sized/recursive-type-2.rs`.
if eval_stack.contains(&t) {
return Ok(true); // Recover; this will error later.
}
eval_stack.push(t);
let ret =
pred.apply_inner(tcx, param_env, eval_stack, in_module, reveal_opaque);
eval_stack.pop();
ret
}
}
}
Self::GenericType(_) => Ok(true),
Self::OpaqueType(key) => match reveal_opaque(key) {
// Unknown opaque is assumed inhabited.
None => Ok(true),
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -752,11 +752,6 @@ passes_unnecessary_partial_stable_feature = the feature `{$feature}` has been pa
passes_unnecessary_stable_feature = the feature `{$feature}` has been stable since {$since} and no longer requires an attribute to enable
passes_unreachable_due_to_uninhabited = unreachable {$descr}
.label = unreachable {$descr}
.label_orig = any code following this expression is unreachable
.note = this expression has type `{$ty}`, which is uninhabited
passes_unrecognized_field =
unrecognized field name `{$name}`
Expand Down
12 changes: 0 additions & 12 deletions compiler/rustc_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1679,18 +1679,6 @@ pub(crate) struct SkippingConstChecks {
pub span: Span,
}

#[derive(LintDiagnostic)]
#[diag(passes_unreachable_due_to_uninhabited)]
pub(crate) struct UnreachableDueToUninhabited<'desc, 'tcx> {
pub descr: &'desc str,
#[label]
pub expr: Span,
#[label(passes_label_orig)]
#[note]
pub orig: Span,
pub ty: Ty<'tcx>,
}

#[derive(LintDiagnostic)]
#[diag(passes_unused_var_maybe_capture_ref)]
#[help]
Expand Down
Loading

0 comments on commit b777188

Please sign in to comment.