-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Improve spans for specialization error #98782
Improve spans for specialization error #98782
Conversation
r? @oli-obk (rust-highfive has picked a reviewer for you, use r? to override) |
_ => { | ||
tcx.sess | ||
.struct_span_err(span, &format!("cannot specialize on `{:?}`", predicate)) | ||
.struct_span_err(span, &format!("cannot specialize on predicate `{}`", predicate)) |
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.
the other predicates have a display impl, why not use it
tcx.sess | ||
.struct_span_err( | ||
span, | ||
&format!("cannot specialize on associated type `{projection_ty} == {term}`",), |
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.
hm, i guess i could rebind projection_ty
if it has bound vars, but i don't think people are specializing on for<'a> Ty: Trait<Proj = &'a ()>
, so it's not necessarily worth the complexity I guess. Also we'd need to skip the binder on term still, so we don't print for<..> assoc == for<..> term
..
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.
yea, not really worth it for the diagnostic. You could emit another note if the binders actually bind anything stating that specialization on lifetimes is not... ideal. But unless we have some good specialization docs for that that we could link to, not really worth it.
@@ -384,9 +389,17 @@ fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, predicate: ty::Predicate<'tc | |||
.emit(); |
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 can't leave a comment on line (new) 385 (old 380), but we should probably just print the whole trait ref instead of just the def path...?
☔ The latest upstream changes (presumably #93967) made this pull request unmergeable. Please resolve the merge conflicts. |
12c7ae5
to
a368830
Compare
@bors r+ rollup |
📌 Commit a368830 has been approved by |
…r-span, r=oli-obk Improve spans for specialization error Fixes rust-lang#98777
…r-span, r=oli-obk Improve spans for specialization error Fixes rust-lang#98777
…r-span, r=oli-obk Improve spans for specialization error Fixes rust-lang#98777
…askrgr Rollup of 8 pull requests Successful merges: - rust-lang#98738 (Clarify MIR semantics of checked binary operations) - rust-lang#98782 (Improve spans for specialization error) - rust-lang#98793 (Lint against executable files in the root directory) - rust-lang#98814 (rustdoc: Censor certain complex unevaluated const exprs) - rust-lang#98878 (add more `rustc_pass_by_value`) - rust-lang#98879 (Fix "wrap closure in parenthesis" suggestion for `async` closure) - rust-lang#98886 (incr.comp.: Make split-dwarf commandline options [TRACKED].) - rust-lang#98898 (Add "no-div-regex" eslint rule) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #98777