Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve spans for specialization error #98782

Merged
merged 1 commit into from
Jul 5, 2022
+31 −18
Merged

Improve spans for specialization error #98782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions compiler/rustc_typeck/src/impl_wf_check/min_specialization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,11 +279,16 @@ fn check_predicates<'tcx>(
span: Span,
) {
let tcx = infcx.tcx;
let impl1_predicates: Vec<_> = traits::elaborate_predicates(
let instantiated = tcx.predicates_of(impl1_def_id).instantiate(tcx, impl1_substs);
let impl1_predicates: Vec<_> = traits::elaborate_predicates_with_span(
tcx,
tcx.predicates_of(impl1_def_id).instantiate(tcx, impl1_substs).predicates.into_iter(),
std::iter::zip(
instantiated.predicates,
// Don't drop predicates (unsound!) because `spans` is too short
instantiated.spans.into_iter().chain(std::iter::repeat(span)),
),
)
.map(|obligation| obligation.predicate)
.map(|obligation| (obligation.predicate, obligation.cause.span))
.collect();

let mut impl2_predicates = if impl2_node.is_from_trait() {
Expand Down Expand Up @@ -321,7 +326,7 @@ fn check_predicates<'tcx>(
// which is sound because we forbid impls like the following
//
// impl<D: Debug> AlwaysApplicable for D { }
let always_applicable_traits = impl1_predicates.iter().copied().filter(|&predicate| {
let always_applicable_traits = impl1_predicates.iter().copied().filter(|&(predicate, _)| {
matches!(
trait_predicate_kind(tcx, predicate),
Some(TraitSpecializationKind::AlwaysApplicable)
Expand All @@ -345,11 +350,11 @@ fn check_predicates<'tcx>(
}
}
impl2_predicates.extend(
traits::elaborate_predicates(tcx, always_applicable_traits)
traits::elaborate_predicates_with_span(tcx, always_applicable_traits)
.map(|obligation| obligation.predicate),
);

for predicate in impl1_predicates {
for (predicate, span) in impl1_predicates {
if !impl2_predicates.contains(&predicate) {
check_specialization_on(tcx, predicate, span)
}
Expand Down Expand Up @@ -384,9 +389,17 @@ fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, predicate: ty::Predicate<'tc
.emit();
Copy link
Member Author

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...?

}
}
ty::PredicateKind::Projection(ty::ProjectionPredicate { projection_ty, term }) => {
tcx.sess
.struct_span_err(
span,
&format!("cannot specialize on associated type `{projection_ty} == {term}`",),
Copy link
Member Author

@compiler-errors compiler-errors Jul 1, 2022 •
edited
Loading

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..

Copy link
Contributor

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.

)
.emit();
}
_ => {
tcx.sess
.struct_span_err(span, &format!("cannot specialize on `{:?}`", predicate))
.struct_span_err(span, &format!("cannot specialize on predicate `{}`", predicate))
Copy link
Member Author

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

.emit();
}
}
Expand Down
6 changes: 3 additions & 3 deletions src/test/ui/specialization/min_specialization/repeated_projection_type.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: cannot specialize on `Binder(ProjectionPredicate(ProjectionTy { substs: [V], item_def_id: DefId(0:6 ~ repeated_projection_type[54ea]::Id::This) }, Ty((I,))), [])`
--> $DIR/repeated_projection_type.rs:19:1
error: cannot specialize on associated type `<V as Id>::This == (I,)`
--> $DIR/repeated_projection_type.rs:19:15
|
LL | impl<I, V: Id<This = (I,)>> X for V {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^^

error: aborting due to previous error

4 changes: 2 additions & 2 deletions src/test/ui/specialization/min_specialization/spec-marker-supertraits.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: cannot specialize on trait `HasMethod`
--> $DIR/spec-marker-supertraits.rs:22:1
--> $DIR/spec-marker-supertraits.rs:22:9
|
LL | impl<T: Marker> Spec for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^

error: aborting due to previous error

4 changes: 2 additions & 2 deletions src/test/ui/specialization/min_specialization/specialization_super_trait.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: cannot specialize on trait `Default`
--> $DIR/specialization_super_trait.rs:13:1
--> $DIR/specialization_super_trait.rs:13:9
|
LL | impl<T: Default> SpecMarker for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^

error: aborting due to previous error

4 changes: 2 additions & 2 deletions src/test/ui/specialization/min_specialization/specialization_trait.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,10 @@ LL | impl<T> SpecMarker for (T, T) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: cannot specialize on trait `Clone`
--> $DIR/specialization_trait.rs:21:1
--> $DIR/specialization_trait.rs:21:9
|
LL | impl<T: Clone> SpecMarker for [T] {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^

error: aborting due to 3 previous errors

4 changes: 2 additions & 2 deletions src/test/ui/specialization/min_specialization/specialize_on_trait.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: cannot specialize on trait `SpecMarker`
--> $DIR/specialize_on_trait.rs:15:1
--> $DIR/specialize_on_trait.rs:15:9
|
LL | impl<T: SpecMarker> X for T {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^

error: aborting due to previous error