Properly mark loop as diverging if it has no breaks by compiler-errors · Pull Request #128443 · rust-lang/rust (original) (raw)

Due to specifics about the desugaring of the .await operator, HIR typeck doesn't recognize that .awaiting an impl Future<Output = !> will diverge in the same way as calling a fn() -> !.

This is because the await operator desugars to approximately:

loop { match future.poll(...) { Poll::Ready(x) => break x, Poll::Pending => {} } }

We know that the value of x is !, however since break is a coercion site, we coerce ! to some ?0 (the type of the loop expression). Then since the type of the loop {...} expression is ?0, we will not detect the loop as diverging like we do with other expressions that evaluate to !:

// 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));
}

We can technically fix this in two ways:

  1. Make coercion of loop exprs more eagerly result in a type of ! when the only break expressions have type !.
  2. Make loops understand that if they have only diverging break values, then the loop diverges as well.

(1.) likely has negative effects on inference, and seems like a weird special case to drill into coercion. However, it turns out that (2.) is very easy to implement, we already record whether a loop has any break expressions, and when we do so, we actually skip over any break expressions with diverging values!:

// If we encountered a `break`, then (no surprise) it may be possible to break from the
// loop... unless the value being returned from the loop diverges itself, e.g.
// `break return 5` or `break loop {}`.
ctxt.may_break |= !self.diverges.get().is_always();

Thus, we can consider the loop as diverging if we see that it has no breaks, which is the change implemented in this PR.

This is not usually a problem in regular code for two reasons:

  1. In regular code, we already mark break diverging() as unreachable if diverging() is unreachable. We don't do this for .await, since we suppress unreachable errors within .await (Silence unreachable code lint from await desugaring #64930). Un-suppressing this code will result in spurious unreachable expression errors pointing to internal await machinery.
  2. In loops that truly have no breaks (e.g. loop {}), we already evaluate the type of the loop to !, so this special case is kinda moot. This only affects loops that have breaks with values of type !.

Thus, this seems like a change that may affect more code than just .await, but it likely does not in meaningful ways; if it does, it's certainly correct to apply.

Fixes #128434