-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Improved error message when type must be bound due to generator. #59111
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # 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?