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 Oct 30, 2024
1 parent 298c746 commit f6e7f7c
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 58 deletions.
14 changes: 5 additions & 9 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
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
50 changes: 24 additions & 26 deletions compiler/rustc_hir_typeck/src/diverges.rs
Original file line number Diff line number Diff line change
@@ -1,34 +1,21 @@
use std::{cmp, ops};

use rustc_span::{DUMMY_SP, Span};
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,

/// 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 +27,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 +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(crate) enum DivergeReason {
AllArmsDiverge,
NeverPattern,
Other,
}
12 changes: 7 additions & 5 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 @@ -246,7 +246,8 @@ 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 ty.is_never() && self.expr_guaranteed_to_constitute_read_for_never(expr) {
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 @@ -1507,7 +1508,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 +1612,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
18 changes: 12 additions & 6 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ 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 {
let Diverges::Always(reason, orig_span) = self.diverges.get() else {
return;
};

Expand Down Expand Up @@ -79,10 +81,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
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 = 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, custom_note);
})
}

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 @@ -42,8 +42,8 @@ use crate::method::probe::IsSuggestion;
use crate::method::probe::Mode::MethodCall;
use crate::method::probe::ProbeScope::TraitsInScope;
use crate::{
BreakableCtxt, Diverges, Expectation, FnCtxt, LoweredTy, Needs, TupleArgumentsFlag, errors,
struct_span_code_err,
BreakableCtxt, DivergeReason, Diverges, Expectation, FnCtxt, LoweredTy, Needs,
TupleArgumentsFlag, errors, struct_span_code_err,
};

#[derive(Clone, Copy, Default)]
Expand Down Expand Up @@ -1761,10 +1761,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
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 @@ -61,7 +61,7 @@ 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 f6e7f7c

Please # to comment.