Skip to content

Commit

Permalink
Auto merge of #128544 - compiler-errors:perf-warn_if_unreachable, r=<…
Browse files Browse the repository at this point in the history
…try>

Check divergence value first before doing span operations in `warn_if_unreachable`

It's more expensive to extract the span's desugaring first rather than check the value of the divergence enum. For some reason I inverted these checks, probably for readability, but as a consequence I regressed perf:

#128443 (comment)

r? fmease
  • Loading branch information
bors committed Aug 2, 2024
2 parents 5367673 + f850e37 commit 9643070
Showing 1 changed file with 18 additions and 18 deletions.
36 changes: 18 additions & 18 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,28 +48,28 @@ 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) {
// 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) {
let Diverges::Always { span: orig_span, custom_note } = self.diverges.get() else {
return;
}
};

// Don't lint if the result of an async block or async function is `!`.
// This does not affect the unreachable lints *within* the body.
if span.is_desugaring(DesugaringKind::Async) {
return;
}
match span.desugaring_kind() {
// 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.
Some(DesugaringKind::CondTemporary) => return,

// Don't lint *within* the `.await` operator, since that's all just desugaring junk.
// We only want to lint if there is a subsequent expression after the `.await`.
if span.is_desugaring(DesugaringKind::Await) {
return;
}
// Don't lint if the result of an async block or async function is `!`.
// This does not affect the unreachable lints *within* the body.
Some(DesugaringKind::Async) => return,

let Diverges::Always { span: orig_span, custom_note } = self.diverges.get() else {
return;
};
// Don't lint *within* the `.await` operator, since that's all just desugaring
// junk. We only want to lint if there is a subsequent expression after the
// `.await` operator.
Some(DesugaringKind::Await) => return,

_ => {}
}

// Don't warn twice.
self.diverges.set(Diverges::WarnedAlways);
Expand Down

0 comments on commit 9643070

Please sign in to comment.