-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Update error message for E0323, E0324 and E0325 #35372
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jonathandturner (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. |
"item `{}` is an associated const, \ | ||
which doesn't match its trait `{:?}`", | ||
impl_const.name, | ||
impl_trait_ref) | ||
.span_label(impl_item.span, &format!("does not match trait")) | ||
.span_label( | ||
tcx.map.span_if_local(ty_trait_item.def_id()).unwrap(), |
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.
this span_if_local
seems like it's the wrong choice, is there a better way to get the trait item def span?
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.
btw, I asked in #rust-internals about using span_if_local here, and got: "span_if_local is not okay here; trait defn may come from an extern crate and it will panic then"
You may want to try another direction
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.
that was my thought as welll seeing local
. I think i'll ask in #rust-internals
Updated, as mentioned in irc, there is no way to get a non-local span so I only add the original trait note if it's a local one |
Looks good! @bors r+ rollup |
📌 Commit e0035c9 has been approved by |
Update error message for E0323, E0324 and E0325 Fixes rust-lang#35325, rust-lang#35327 and rust-lang#35329 as part of rust-lang#35233 r? @jonathandturner
Update error message for E0323, E0324 and E0325 Fixes rust-lang#35325, rust-lang#35327 and rust-lang#35329 as part of rust-lang#35233 r? @jonathandturner
Fixes #35325, #35327 and #35329 as part of #35233
r? @jonathandturner