On type errors, show closure signature

[estebank]

Instead of referring to closures with their span, format their signature.

r? @nikomatsakis

Fix #19939.

[nikomatsakis]

Test? =)

[estebank]

Will add, was pushed on a hurry before getting on plane to get your feedback :o)

This is a bit as hoc and doesn't use type resolve to infer the args and return type. Is there a way to do it here, or should I keep it as simple as it is in this PR? The same can be done in ppaux (for the sake of consistent output). If we change it in ppaux I would like to keep the @ span location on those.

The difference would be fn(_) -> _ vs fn(_) -> _ @ $FILE:xx:xx. I would also add at least on type mismatch errors a label against the expected closure's span. I don't know how we want to handle closures that don't have a local node (so they can't be printed in the new format in the way presented in this PR).

[nikomatsakis]

Well, leaving aside the mechanism, I'd like to know what you have in mind in terms of how you want things to look. I am not sure what I think about printing closure types as fn(...), since we have actual fn pointer types written that way which are distinct. That said, the current printout is clearly not good either!

[estebank]

I believe that for the following code:

fn closure_to_loc() {
    let mut x = |c| c + 1;
    x = |c| c + 1;
}

the output should be close to

error[E0308]: mismatched types
 --> src/main.rs:3:9
  |
3 |     x = |c| c + 1;
  |         ^^^^^^^^^ expected a closure, found a different closure
  |
  = note: expected type `|_| -> _`
             found type `|_| -> _`
note: no two closures, even if identical, have the same type
 --> src/main.rs:3:9
  |
2 |     let mut x = |c| c + 1;
  |                 --------- this was the expected closure
3 |     x = |c| c + 1;
  |         ^^^^^^^^^ but this closure was found
  = help: consider boxing your closure and/or using it as a trait object

instead of

error[E0308]: mismatched types
 --> src/main.rs:3:9
  |
3 |     x = |c| c + 1;
  |         ^^^^^^^^^ expected closure, found a different closure
  |
  = note: expected type `[closure@src/main.rs:2:17: 2:26]`
             found type `[closure@src/main.rs:3:9: 3:18]`
note: no two closures, even if identical, have the same type
 --> src/main.rs:3:9
  |
3 |     x = |c| c + 1;
  |         ^^^^^^^^^
help: consider boxing your closure and/or using it as a trait object
 --> src/main.rs:3:9
  |
3 |     x = |c| c + 1;
  |         ^^^^^^^^^

(This PR does not implement the entirety of this as of yet.)

[bors]

☔ The latest upstream changes (presumably #46349) made this pull request unmergeable. Please resolve the merge conflicts.

[arielb1]

I think that printing closures as fn(_) -> _ is terrible, because function pointers already print that way and are very distinct from closures. I might like an fn-item-like fn() {main::[closure#4]} (using an index of the closure within the function), because closures are very much like fn items. That's not a good idea, closures contain data, unlike fn items.

I also think changing the way closures are printed in some error messages, but not all of them, is just going to confuse everyone when they see the error they are less familiar with.

[arielb1]

I think the main way you get closure types in type errors is when you have a type error containing a complicated type with a closure, and we need to avoid the closure "dominating" the error message by its ugliness.

[estebank]

@arielb1 what do you think of referring to closures as move |_| -> _ like in the example? I think that we can either change the output everywhere or only in type errors, my main concern is the span information, I would like to avoid having that as text and instead have spans pointing at them if possible. Otherwise, an alternative (smaller) change could be the following:

error[E0308]: mismatched types
 --> src/main.rs:3:9
  |
2 |     let mut x = |c| c + 1;
  |                 --------- this closure was expected
3 |     x = |c| c + 1;
  |         ^^^^^^^^^ expected closure, found a different closure
  |
  = note: expected type `[closure]`
             found type `[closure]`
  = note: no two closures, even if identical, have the same type
  = help: consider boxing your closure and/or using it as a trait object

That being said, I think it'd be nice to have a (pseudo)signature available in the type error output.

[estebank]

Having multiple spans in a span_note is not currently supported so the proposed output in a previous comment wouldn't be currently possible. I could add support but will probably do so if we find other places where it'd make sense.

[arielb1]

@estebank

As I said, the main way you get closures in error messages is in more complicated types. I need to find a few good situations so we can figure out how to cover them with error messages.

[nikomatsakis]

Is there any particular reason to scrape this information from the HIR? It can also be extracted from the closure type readily enough. To get the closure signature, you would do something like this (I have a more convenient helper for this on the nll-master branch, but it's not landed yet):

ty::TyClosure(did1, substs1) => {
    let closure_sig_ty = substs.closure_sig_ty(def_id, self.tcx);
    let closure_sig_ty = self.shallow_resolve(&closure_sig_ty);
    let sig: PolyFnSig<'_> = closure_sig_ty.fn_sig(self.tcx);
}

[nikomatsakis]

I think the main way you get closure types in type errors is when you have a type error containing a complicated type with a closure, and we need to avoid the closure "dominating" the error message by its ugliness.

I think this makes sense. It feels like there are a number of situations in which we want to a way to "extract out" more complex subparts of a type. Well, maybe not a number, I'm thinking primarily of the desire to be able to say something like "the type of this variable is Vec<T> for some T that we can't infer" (e.g., to improve the error you get with let _ = vec![];). Still it seems related. If we had some convention for "pulling out" types, we might say

expected: Foo<Bar<X>>
where X is the type for the closure defined here

Is this a good idea to pursue? I'm not sure. Maybe.

[estebank]

@nikomatsakis I suggested something similar in #21025 (comment), I know that there is a ticket or comment where the same was proposed in more detail but couldn't find it. I think it is a worthwhile idea, specially for cases where we have

expected: Foo<Bar<X>>
   found: X
 where X: A<Very<Long<Common<Type>>>

[nikomatsakis]

@estebank do you want to try pursuing that instead then, see how it feels? I like the idea of finding a way to "pull out" more detailed information, seems like it could be a big improvement overall; on the other hand, types appear in a lot of contexts. It feels like ideally we'd due some sort of revamp of the Diagnostics API to be less string-based, so that it could uniformly process types that appear in all the various kinds of messages, notes, labels, and things

[bors]

☔ The latest upstream changes (presumably #46641) made this pull request unmergeable. Please resolve the merge conflicts.

[kennytm]

Triage ping @estebank — @nikomatsakis has left some comments for this PR! BTW there is also merge conflict.

[estebank]

I'll close this PR in lieu of implementing the suggestion above sometime in the near future :)