When encountering unexpected closure return type, point at return type/expression

[estebank]

error[E0271]: expected `{closure@fallback-closure-wrap.rs:18:40}` to be a closure that returns `()`, but it returns `!`
  --> $DIR/fallback-closure-wrap.rs:19:9
   |
LL |     let error = Closure::wrap(Box::new(move || {
   |                                        -------
LL |         panic!("Can't connect to server.");
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `!`
   |
   = note: expected unit type `()`
                   found type `!`
   = note: required for the cast from `Box<{closure@$DIR/fallback-closure-wrap.rs:18:40: 18:47}>` to `Box<dyn FnMut()>`

error[E0271]: expected `{closure@dont-ice-for-type-mismatch-in-closure-in-async.rs:6:10}` to be a closure that returns `bool`, but it returns `Option<()>`
  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:6:16
   |
LL |     call(|| -> Option<()> {
   |     ---- ------^^^^^^^^^^
   |     |          |
   |     |          expected `bool`, found `Option<()>`
   |     required by a bound introduced by this call
   |
   = note: expected type `bool`
              found enum `Option<()>`
note: required by a bound in `call`
  --> $DIR/dont-ice-for-type-mismatch-in-closure-in-async.rs:3:25
   |
LL | fn call(_: impl Fn() -> bool) {}
   |                         ^^^^ required by this bound in `call`

error[E0271]: expected `{closure@f670.rs:28:13}` to be a closure that returns `Result<(), _>`, but it returns `!`
    --> f670.rs:28:20
     |
28   |     let c = |e| -> ! {
     |             -------^
     |                    |
     |                    expected `Result<(), _>`, found `!`
...
32   |     f().or_else(c);
     |         ------- required by a bound introduced by this call
-Ztrack-diagnostics: created at compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs:1433:28
     |
     = note: expected enum `Result<(), _>`
                found type `!`
note: required by a bound in `Result::<T, E>::or_else`
    --> /home/gh-estebank/rust/library/core/src/result.rs:1406:39
     |
1406 |     pub fn or_else<F, O: FnOnce(E) -> Result<T, F>>(self, op: O) -> Result<T, F> {
     |                                       ^^^^^^^^^^^^ required by this bound in `Result::<T, E>::or_else`

CC #111539.

[rustbot]

r? @Nadrieril

rustbot has assigned @Nadrieril.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[Nadrieril]

Huge improvement, this has bitten me before :) Would prefer boring labels over no labels for the diagnostic spans.

[compiler-errors]

Why are we passing a different span than the span passed in via the obligation cause?

This at least needs an explanation; especially when an argument seems to be redundant in 19 out of 20 calls, and only differs for one, but there's literally no documentation to explain why it's like that. I find that adding new arguments like this with basically no (apparent) rhyme or reason (even if there's a good justification that's not documented) to these very pervasive functions (like, note_type_err is the entrypoint to basically all type error reporting!) to really harm maintainability of these APIs :(

[compiler-errors]

An idea is to rename this to override_span: Option<Span>, and move it to the end of the arg list (or perhaps just behind seconary_span, and add a doc comment explaining that passing None will use the obligation cause's span. Then most of the callsites will just need to pass None rather than redundantly passing both the obligation cause and the obligation cause's span as two args.

[estebank]

Let me know what you think.

[Nadrieril]

Looks good to me now, I'll let @compiler-errors decide

[compiler-errors]

After the nit I commented, then there should only be one Some() call, which makes it consistent with the comment, so 👍

[compiler-errors]

Suggested change

	let span = override_span.unwrap_or(cause.span);
	let span = override_span.unwrap_or(cause.span());

Turns out that .span and .span() do different things! This is kinda what I meant when I said that these APIs are unmaintainable today.

[compiler-errors]

This should fix the regression in spans, afaict.

[compiler-errors]

#132243 removes the need for that pointless/confusing method.