Fix reversed current/expected type #5070
Conversation
| @@ -142,7 +142,7 @@ pub fn check_pat_variant(pcx: pat_ctxt, pat: @ast::pat, path: @ast::path, | |||
| // Check that the type of the value being matched is a subtype of | |||
| // the type of the pattern. | |||
| let pat_ty = fcx.node_ty(pat.id); | |||
| demand::suptype(fcx, pat.span, pat_ty, expected); | |||
| demand::suptype(fcx, pat.span, expected, pat_ty); | |||
There was a problem hiding this comment.
It is wrong to swap the order of these two types. The pat_ty should be be the supertype. If the problem is the error message is coming out incorrect, there is another way to solve it, though it's mildly more involved.
There was a problem hiding this comment.
However, I think in this case which one is "expected" and not is a matter of perspective. I kind of expect the pattern to be the "expected" type in the print-out, which is precisely how it works now (before this change).
There was a problem hiding this comment.
I disagree. I think "found" type should be the type of the expression highlighted, e.g. let a: int = 'a' highlights 'a' and says expected 'int' but found 'char'. In this case the pattern is highlighted, so either pattern should be "found" type, or highlighting should change.
There was a problem hiding this comment.
That makes sense. Well, I am somewhat indifferent. It's easy enough to change where the highlight is placed, and it's easy enough to change which type is expected. But this specific change is semantically incorrect. Let me know which way you'd rather go and I'll advise how best to get there.
There was a problem hiding this comment.
Hi. I got what I was wrong. I am making new patch for this.
I understands:
- it should not test suptype in reversed direction.
- but highlighted type and error message is mismatched.
Is it right?
There was a problem hiding this comment.
In my opinion, highlighting should stay, since the pattern can be far away from the matched expression. So how does one change which type is "expected" in the error message?
There was a problem hiding this comment.
How about b218765f ? Could I replace this commit with it?
There was a problem hiding this comment.
Close but it is also incorrect. I left a comment in that comment. I think the general idea of adding demand::subtype and demand::suptype is fine, that's more-or-less what I was going to suggest.
There was a problem hiding this comment.
Thanks. I've fixed it like 7cdc800
|
@nikomatsakis I pushed new version with new variable name. But I didn't remove XXX because I couldn't resolve that case: https://gist.github.com/youknowone/5040645 |
|
@youknowone I don't understand, what is the output from that case and what is the expected output? In any case, if you want to keep the XXX in there, you need to open a bug and change it to me FIXME(#1234), where |
Fix some reversed type of arm pattern and type of search pattern in error message.
|
@nikomatsakis I was stupid. I fixed it too now. |
|
@youknowone thanks very much for this. I just marked it r+. |
It is reversed that type of arm pattern and type of search pattern in error message.
|
thanks! |
Improve `suspicious_map`documentation Fixes rust-lang#4793 changelog: none
It is reversed that type of arm pattern and type of search pattern
in error message.