-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Improved error message when type must be bound due to generator. #59111
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
Conversation
r? @zackmdavis (rust_highfive has picked a reviewer for you, use r? to override) |
(This PR is to fix #58930 ) |
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.
Wow, nice!
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Penny drops - rust master has moved on. Will fetch upstream and diagnose. |
The one thing I was wondering about was whether I could use the fully_resolve visitor in resolve.rs and map the TyVid inside the FixupError to a Ty and a Span... I tried:
but alas the probe returned an error. I'm also not sure how to map a Ty (or TyVid) to the relevant span. What we have works and is definitely an improvement on the status quo, just wondering if it could be done with less code / simpler. |
ping from triage @zackmdavis awaiting your review on this |
@nikomatsakis I think I've done all the requested changes? |
(sorry I haven't really been involved here despite being tagged by @bors) |
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.
Hi @gilescope -- sorry for the delay. This looks basically good, but I left some nits.
☔ The latest upstream changes (presumably #60025) made this pull request unmergeable. Please resolve the merge conflicts. |
077b65c
to
262b285
Compare
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
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.
Looks great! r=me with nits below fixed. Thanks!
@bors delegate=gilescope @gilescope -- this should mean that you can do |
📌 Commit 262b2851abe6e7b4e2ebbac9c53e35a6c98e8005 has been approved by `nikomatsakis`` |
✌️ @gilescope can now approve this pull request |
@bors r- bors got a bit overeager there, I think. |
Error now mentions type var name and span is highlighted.
57e67ca
to
66e41bc
Compare
(squashed as all looking good) |
@bors r=nikomatsakis |
📌 Commit 66e41bc has been approved by |
@nikomatsakis great, hopefully that's a wrap. |
Improved error message when type must be bound due to generator. Fixes #58930. Keen to get some feedback - is this as minimal as we can get it or is there an existing visitor I could repurpose?
☀️ Test successful - checks-travis, status-appveyor |
Fixes #58930.
Keen to get some feedback - is this as minimal as we can get it or is there an existing visitor I could repurpose?