-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Fix nonsense non-tupled Fn
trait error
#99942
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
Fix nonsense non-tupled Fn
trait error
#99942
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
Fn
trait error
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.
Sorry for the delay @compiler-errors.
When we have Fn<A>
with a concrete type A
, we should probably report that a tuple is expected. The diagnostic improvement for generic A
is great though.
@@ -0,0 +1,8 @@ | |||
#![feature(unboxed_closures)] | |||
|
|||
fn a<F: Fn<usize>>(f: F) {} |
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.
Shouldn't we just report that a tuple is expected and bail out?
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.
Well making this a hard error in #99943, so I don't want to just introduce a special case error logic here.
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, reporting that a tuple is expected and bailing out is not gonna work when you have a generic Fn<T>
where T
is a type param.
_ => vec![ArgKind::empty()], | ||
_ => { | ||
not_tupled = true; | ||
vec![ArgKind::empty()] |
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.
Why do we keep a single ArgKind::empty()
, like 1-tuples, instead of an empty vec?
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.
No particular reason, I can fix that I guess.
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.
Oh, I know why. This code also handles Generator<T>
which takes a generic argument that isn't a tupled set of arguments.
@rustbot author |
I actually don't know if I have anything to change here. See replied comments. @rustbot ready |
@bors r+ |
@bors r-
|
9e52d22
to
1c084f1
Compare
@bors r=cjgillot |
Given this code:
We currently emit this error:
Notably, it says the same thing for "expected" and "found"!
Fix the output so that we instead emit:
The error could still use some work, namely the "mismatched types" part, but I'm leaving it a bit rough since the only way you'd ever get this error is when you're messing with
#![feature(unboxed_closures)]
.Simply making sure we actually print out the difference in trait-refs is good enough for me. I could probably factor in some additional improvements if those are desired.