Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

infer_ty(IntVar) should not print as _ in user error messages #24770

Closed
pnkfelix opened this issue Apr 24, 2015 · 9 comments
Closed

infer_ty(IntVar) should not print as _ in user error messages #24770

pnkfelix opened this issue Apr 24, 2015 · 9 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints

Comments

@pnkfelix
Copy link
Member

spawned off of #24728 (comment)

In an example like:

fn main() {
    let vs = vec![1, 2, 3, 4];

    for v in &vs {
        match v {
            1 => {}
            _ => {}
        }
    }
}

which yields the error message:

<anon>:4:5: 9:6 error: type mismatch resolving  \ (linebreak solely for presentation here)
 `<core::slice::Iter<'_, _> as core::iter::Iterator>::Item == _`:
 expected &-ptr,
    found integral variable [E0271]
<anon>:4     for v in &vs {
<anon>:5         match v {
<anon>:6             1 => {}
<anon>:7             _ => {}
<anon>:8         }
<anon>:9     }

i find it frustrating that the first line prints the integral type variable as just _.

It is not a completely unconstrained type; it must be some kind of integer.

(It is true that one should be able to figure this out from the third line. But I think we can still do better on the first line itself.)

I am thinking something like this could suffice:

error: type mismatch resolving   \
 `<core::slice::Iter<'_, _> as core::iter::Iterator>::Item == iN`

Another slightly more verbose option (but less prone to misinterpretation when e.g. unsigned integers are involved, or when the user has defined a type named iN) would be:

error: type mismatch resolving   \
 `<core::slice::Iter<'_, _> as core::iter::Iterator>::Item == <int type>`

Yet another option, which is both succinct but will not collide with user definitions (at least not today):

error: type mismatch resolving   \
 `int? == <core::slice::Iter<'_, _> as core::iter::Iterator>::Item`

and likewise one would imagine for the FloatVar case

error: type mismatch resolving   \
 `float? == <core::slice::Iter<'_, _> as core::iter::Iterator>::Item`

I forced the int? and float? to be on the left-hand side to ensure that they did not get mistaken for a question mark ending the error message itself. (I did consider ?int and ?float but I worry a little that those would get confused with the generalization marker ?Sized ... maybe that is a small price to pay...)

@pnkfelix
Copy link
Member Author

(it might also be interesting to try to just write core::slice::Iter<'_, _>::Item when there is no other trait in scope that defines an associated type named Item, to further condense this error message to its core. But I digress.)

@pnkfelix
Copy link
Member Author

also, from #14007 i infer that we used to do better here ? Was it a deliberate choice to stop printing <generic integer #1> and print _ instead?

@pnkfelix
Copy link
Member Author

cc @nikomatsakis

@pnkfelix
Copy link
Member Author

Seems like the use of _ for int-type variables was introduced in commit a2624fc (PR #18264); I am not sure what code paths produced the print outs that @zwarich referenced in #14007

@pnkfelix
Copy link
Member Author

ah, seems like it was a deliberate choice, based on the argument presented here by @nikomatsakis

But I think that the code in this ticket's description is a strong counter-example to that claim: the failure to unify &T with <integral typevar> is precisely due to the distinction between normal type variables and integral ones.

@nikomatsakis
Copy link
Contributor

Some thoughts:

  1. This error message is bad. It arises when we essentially have a constraint mismatch we didn't detect until late -- I wonder if we can improve it overall.
  2. In general I don't like how we do integral variables, it seems like it may limit us in the future. I'd rather we used general purpose variables and then added side-obligations that required them to be integral. This would be more forwards compat and it would make the distinction we are trying to draw here non-existent. Perhaps worth experimenting with, unclear how much code would break as a result.

@pnkfelix
Copy link
Member Author

Another option i just thought of while brainstorming is to print i32 for infer_ty(IntVar). The argument for doing so is that if we have not yet seen some constraint forces this to be a more specific int type (e.g. u8), then there's a good chance that all of the evidence we have so far (and maybe all of the evidence we will ever see in this compilation unit) will still lead us to just apply the integer fallback to i32.

(Likewise, we would print f64 for infer_ty(FloatVar).)

@eddyb
Copy link
Member

eddyb commented Mar 12, 2016

I think we should use "integer type" and "floating-point type" instead of "integral variable" and "floating-point variable".

@durka
Copy link
Contributor

durka commented Aug 23, 2016

I think this can be closed.

error[E0308]: mismatched types
 --> <anon>:6:13
  |
6 |             1 => {}
  |             ^ expected &{integer}, found integral variable
  |
  = note: expected type `&{integer}`
  = note:    found type `{integer}`

@eddyb eddyb closed this as completed Aug 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints
Projects
None yet
Development

No branches or pull requests

5 participants