-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Be more specific when type parameter shadows primitive type #36338
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
self.note(&format!("expected {} `{}`", label, expected)); | ||
self.note(&format!(" found {} `{}`", label, found)); | ||
self.note(&format!("expected {} `{}` {}", label, expected, expected_extra)); | ||
self.note(&format!(" found {} `{}` {}", label, found, found_extra)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be preferable to move the added spaces to the _extra
variables, so that there isn't any trailing space in case the _extra
variables are empty.
That should also fix the current three ui test failures.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done so and rebased. All tests now pass.
07012ea
to
fcaa8b1
Compare
@@ -550,7 +550,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { | |||
}; | |||
|
|||
if !is_simple_error { | |||
diag.note_expected_found(&"type", &expected, &found); | |||
if expected == found { | |||
if let &TypeError::Sorts(ref values) = terr { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This should be indented four spaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, do we want to not error when terr
is not &TypeError::Sorts(..)
here?
LGTM modulo comment |
Yes, as @jseyfried noted, it seems like we are now missing a case where the "expected found" will not be printed, but otherwise looks good to me. |
It'll be nice to see this fixed! |
fcaa8b1
to
43e94fa
Compare
@nikomatsakis, @jseyfried incorporated your comments and rebased. |
When a type parameter shadows a primitive type, the error message was non obvious. For example, given the file `file.rs`: ```rust trait Parser<T> { fn parse(text: &str) -> Option<T>; } impl<bool> Parser<bool> for bool { fn parse(text: &str) -> Option<bool> { Some(true) } } fn main() { println!("{}", bool::parse("ok").unwrap_or(false)); } ``` The output was: ```bash % rustc file.rs error[E0308]: mismatched types --> file.rs:7:14 | 7 | Some(true) | ^^^^ expected type parameter, found bool | = note: expected type `bool` = note: found type `bool` error: aborting due to previous error ``` We now show extra information about the type: ```bash % rustc file.rs error[E0308]: mismatched types --> file.rs:7:14 | 7 | Some(true) | ^^^^ expected type parameter, found bool | = note: expected type `bool` (type parameter) = note: found type `bool` (bool) error: aborting due to previous error ``` Fixes rust-lang#35030
43e94fa
to
68e8624
Compare
@nikomatsakis I believe this might be ready for merging. Would you agree? |
@bors r+ |
📌 Commit 68e8624 has been approved by |
@estebank Thanks for the PR! |
@jseyfried my pleasure! |
Be more specific when type parameter shadows primitive type When a type parameter shadows a primitive type, the error message was non obvious. For example, given the file `file.rs`: ```rust trait Parser<T> { fn parse(text: &str) -> Option<T>; } impl<bool> Parser<bool> for bool { fn parse(text: &str) -> Option<bool> { Some(true) } } fn main() { println!("{}", bool::parse("ok").unwrap_or(false)); } ``` The output was: ```bash % rustc file.rs error[E0308]: mismatched types --> file.rs:7:14 | 7 | Some(true) | ^^^^ expected type parameter, found bool a | = note: expected type `bool` = note: found type `bool` error: aborting due to previous error ``` We now show extra information about the type: ```bash % rustc file.rs error[E0308]: mismatched types --> file.rs:7:14 | 7 | Some(true) | ^^^^ expected type parameter, found bool a | = note: expected type `bool` (type parameter) = note: found type `bool` (bool) error: aborting due to previous error ``` Fixes #35030
When a type parameter shadows a primitive type, the error message
was non obvious. For example, given the file
file.rs
:The output was:
We now show extra information about the type:
Fixes #35030