-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Improve diagnostics for E0282 #24966
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -290,7 +290,7 @@ pub fn maybe_report_ambiguity<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>, | |
{ | ||
span_err!(infcx.tcx.sess, obligation.cause.span, E0282, | ||
"unable to infer enough type information about `{}`; \ | ||
type annotations required", | ||
type annotations or generic parameter binding required", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ruud-v-a with a change like this, you need to update the test suite to reflect the change to the expected error message. I highly recommend you do a local There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the heads-up. I did run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe I misinterpreted the log files, but it certainly seemed like the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pnkfelix It were definitely those tests that failed. To be sure, I ran
Last time I only checked the last line, and I assumed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, okay, that is a little bit more comforting, but still a little worrisome. Maybe this is something going wrong with our return codes on Windows, causing the |
||
self_ty.user_string(infcx.tcx)); | ||
} else { | ||
span_err!(infcx.tcx.sess, obligation.cause.span, E0283, | ||
|
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 throughout these examples it should be safe to leave off the
_i32
annotation, right?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.
Yes, but then the type of
x: Vec<_>
depends on the integer literal default, which is a feature interacting here that is not really related to the error. Maybe we could use an other type likechar
to avoid defaults altogether, but is there a good and simple way to get an iterator there that is not silly? (E.g.let v = vec!('a', 'b', 'c'); v.iter().collect::<Vec<_>>()
seems like a silly example to me.)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.
If you're still inclined to use something like char, there is the option of starting with a string and iterating over its chars, like this:
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.
and if you did that, you could avoid talking about
VecDeque
; you could just say thatVec<char>
andString
are suitable candidates: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.
@pnkfelix Yes, that is much better!