Skip to content

Commit

Permalink
Refactor Diverges in typeck
Browse files Browse the repository at this point in the history
Avoid deriving PartialOrd on Diverges since it includes fields which
should not affect ordering.
  • Loading branch information
camsteffen authored and cjgillot committed Apr 7, 2024
1 parent e78913b commit 5ba9e4b
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 80 deletions.
14 changes: 5 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, Diag};
use rustc_hir::def::{CtorOf, DefKind, Res};
use rustc_hir::def_id::LocalDefId;
Expand Down Expand Up @@ -30,7 +30,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 @@ -154,13 +155,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
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 @@ -2,7 +2,7 @@ use std::cell::RefCell;

use crate::coercion::CoerceMany;
use crate::gather_locals::GatherLocalsVisitor;
use crate::{CoroutineTypes, Diverges, FnCtxt};
use crate::{CoroutineTypes, DivergeReason, Diverges, FnCtxt};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::intravisit::Visitor;
Expand Down Expand Up @@ -76,10 +76,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
51 changes: 25 additions & 26 deletions compiler/rustc_hir_typeck/src/diverges.rs
Original file line number Diff line number Diff line change
@@ -1,33 +1,21 @@
use rustc_span::{Span, DUMMY_SP};
use rustc_span::Span;

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)]
#[derive(Copy, Clone, Debug)]
pub enum Diverges {
/// 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, Span),

/// Same as `Always` but with a reachability
/// warning already emitted.
Expand All @@ -39,14 +27,15 @@ pub 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 @@ -63,15 +52,25 @@ 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 => 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,
NeverPattern,
Other,
}
7 changes: 4 additions & 3 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::type_error_struct;
use crate::CoroutineTypes;
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 @@ -253,7 +253,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// 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));
self.diverges
.set(self.diverges.get() | Diverges::Always(DivergeReason::Other, expr.span));
}

// Record the type, which applies it effects.
Expand Down Expand Up @@ -1432,7 +1433,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
58 changes: 28 additions & 30 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::callee::{self, DeferredCallResolution};
use crate::errors::CtorIsPrivate;
use crate::method::{self, MethodCallee, SelfSource};
use crate::rvalue_scopes;
use crate::{BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy};
use crate::{BreakableCtxt, DivergeReason, Diverges, Expectation, FnCtxt, LoweredTy};
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, Diag, ErrorGuaranteed, MultiSpan, StashKey};
Expand Down Expand Up @@ -48,36 +48,34 @@ 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);

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,
msg.clone(),
|lint| {
lint.span_label(span, msg).span_label(
orig_span,
custom_note
.unwrap_or("any code following this expression is unreachable"),
);
},
)
}
let Diverges::Always(reason, orig_span) = 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;
}

self.diverges.set(Diverges::WarnedAlways);

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, msg.clone(), |lint| {
let label = match reason {
DivergeReason::AllArmsDiverge => {
"any code following this `match` expression is unreachable, as all arms diverge"
}
DivergeReason::NeverPattern => "any code following a never pattern is unreachable",
DivergeReason::Other => "any code following this expression is unreachable",
};
lint.span_label(span, msg).span_label(orig_span, label);
})
}

/// Resolves type and const variables in `ty` if possible. Unlike the infcx
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ use crate::method::MethodCallee;
use crate::TupleArgumentsFlag::*;
use crate::{errors, Expectation::*};
use crate::{
struct_span_code_err, BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy, Needs,
TupleArgumentsFlag,
struct_span_code_err, BreakableCtxt, DivergeReason, Diverges, Expectation, FnCtxt, LoweredTy,
Needs, TupleArgumentsFlag,
};
use itertools::Itertools;
use rustc_ast as ast;
Expand Down Expand Up @@ -1637,10 +1637,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub fn check_decl_local(&self, local: &'tcx hir::LetStmt<'tcx>) {
self.check_decl(local.into());
if local.pat.is_never_pattern() {
self.diverges.set(Diverges::Always {
span: local.pat.span,
custom_note: Some("any code following a never pattern is unreachable"),
});
self.diverges.set(Diverges::Always(DivergeReason::NeverPattern, local.pat.span));
}
}

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 @@ -47,7 +47,7 @@ pub use typeck_root_ctxt::TypeckRootCtxt;

use crate::check::check_fn;
use crate::coercion::DynamicCoerceMany;
use crate::diverges::Diverges;
use crate::diverges::{DivergeReason, Diverges};
use crate::expectation::Expectation;
use crate::fn_ctxt::LoweredTy;
use crate::gather_locals::GatherLocalsVisitor;
Expand Down

0 comments on commit 5ba9e4b

Please sign in to comment.