-
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
silence mismatched types errors for implied projections #121863
Changes from all commits
6fa58be
189e784
443c816
aa55f6d
6bd970d
db48b93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -223,60 +223,87 @@ enum Elaborate { | |
None, | ||
} | ||
|
||
/// Points the cause span of a super predicate at the relevant associated type. | ||
/// | ||
/// Given a trait impl item: | ||
/// | ||
/// ```ignore (incomplete) | ||
/// impl TargetTrait for TargetType { | ||
/// type Assoc = SomeType; | ||
/// } | ||
/// ``` | ||
/// | ||
/// And a super predicate of `TargetTrait` that has any of the following forms: | ||
/// | ||
/// 1. `<OtherType as OtherTrait>::Assoc == <TargetType as TargetTrait>::Assoc` | ||
/// 2. `<<TargetType as TargetTrait>::Assoc as OtherTrait>::Assoc == OtherType` | ||
/// 3. `<TargetType as TargetTrait>::Assoc: OtherTrait` | ||
/// | ||
/// Replace the span of the cause with the span of the associated item: | ||
/// | ||
/// ```ignore (incomplete) | ||
/// impl TargetTrait for TargetType { | ||
/// type Assoc = SomeType; | ||
/// // ^^^^^^^^ this span | ||
/// } | ||
/// ``` | ||
/// | ||
/// Note that bounds that can be expressed as associated item bounds are **not** | ||
/// super predicates. This means that form 2 and 3 from above are only relevant if | ||
/// the [`GenericArgsRef`] of the projection type are not its identity arguments. | ||
fn extend_cause_with_original_assoc_item_obligation<'tcx>( | ||
tcx: TyCtxt<'tcx>, | ||
trait_ref: ty::TraitRef<'tcx>, | ||
item: Option<&hir::Item<'tcx>>, | ||
cause: &mut traits::ObligationCause<'tcx>, | ||
pred: ty::Predicate<'tcx>, | ||
) { | ||
debug!( | ||
"extended_cause_with_original_assoc_item_obligation {:?} {:?} {:?} {:?}", | ||
trait_ref, item, cause, pred | ||
); | ||
debug!(?item, ?cause, ?pred, "extended_cause_with_original_assoc_item_obligation"); | ||
let (items, impl_def_id) = match item { | ||
Some(hir::Item { kind: hir::ItemKind::Impl(impl_), owner_id, .. }) => { | ||
(impl_.items, *owner_id) | ||
} | ||
_ => return, | ||
}; | ||
let fix_span = | ||
|impl_item_ref: &hir::ImplItemRef| match tcx.hir().impl_item(impl_item_ref.id).kind { | ||
hir::ImplItemKind::Const(ty, _) | hir::ImplItemKind::Type(ty) => ty.span, | ||
_ => impl_item_ref.span, | ||
}; | ||
|
||
let ty_to_impl_span = |ty: Ty<'_>| { | ||
if let ty::Alias(ty::Projection, projection_ty) = ty.kind() | ||
&& let Some(&impl_item_id) = | ||
tcx.impl_item_implementor_ids(impl_def_id).get(&projection_ty.def_id) | ||
&& let Some(impl_item) = | ||
items.iter().find(|item| item.id.owner_id.to_def_id() == impl_item_id) | ||
{ | ||
Some(tcx.hir().impl_item(impl_item.id).expect_type().span) | ||
} else { | ||
None | ||
} | ||
}; | ||
|
||
// It is fine to skip the binder as we don't care about regions here. | ||
match pred.kind().skip_binder() { | ||
ty::PredicateKind::Clause(ty::ClauseKind::Projection(proj)) => { | ||
// The obligation comes not from the current `impl` nor the `trait` being implemented, | ||
// but rather from a "second order" obligation, where an associated type has a | ||
// projection coming from another associated type. See | ||
// `tests/ui/associated-types/point-at-type-on-obligation-failure.rs` and | ||
// `traits-assoc-type-in-supertrait-bad.rs`. | ||
if let Some(ty::Alias(ty::Projection, projection_ty)) = | ||
proj.term.ty().map(|ty| ty.kind()) | ||
&& let Some(&impl_item_id) = | ||
tcx.impl_item_implementor_ids(impl_def_id).get(&projection_ty.def_id) | ||
&& let Some(impl_item_span) = items | ||
.iter() | ||
.find(|item| item.id.owner_id.to_def_id() == impl_item_id) | ||
.map(fix_span) | ||
// Form 1: The obligation comes not from the current `impl` nor the `trait` being | ||
// implemented, but rather from a "second order" obligation, where an associated | ||
// type has a projection coming from another associated type. | ||
// See `tests/ui/traits/assoc-type-in-superbad.rs` for an example. | ||
if let Some(term_ty) = proj.term.ty() | ||
&& let Some(impl_item_span) = ty_to_impl_span(term_ty) | ||
{ | ||
cause.span = impl_item_span; | ||
} | ||
|
||
// Form 2: A projection obligation for an associated item failed to be met. | ||
// We overwrite the span from above to ensure that a bound like | ||
// `Self::Assoc1: Trait<OtherAssoc = Self::Assoc2>` gets the same | ||
// span for both obligations that it is lowered to. | ||
if let Some(impl_item_span) = ty_to_impl_span(proj.self_ty()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you intentionally overwrite the span from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, not intentional. I just didn't think about the case where form 1 and form 2 both apply, but with different assoc items. This can indeed happen and this PR changes the span for this case: trait OtherTrait {
type Assoc;
}
trait TargetTrait
where
Self::Assoc1<()>: OtherTrait<Assoc = Self::Assoc2>
{
type Assoc1<T>;
type Assoc2;
}
impl OtherTrait for () {
type Assoc = u8;
}
// ERROR: type mismatch resolving `<() as OtherTrait>::Assoc == u16`
impl TargetTrait for () {
type Assoc1<T> = ();
// ^^ with the current state of this PR the span points here
type Assoc2 = u16;
// ^^^ before this PR the span points here
} Ideally the error should probably be pointing at both of these, because either of them could be changed to fix it, but that would require changes to obligation spans in general. And pointing at either of these still seems to be better than just pointing at the trait. But we also emit a special diagnostic for the "second order" / form 1 case, so that would be an argument for preferring the form 1 span over the form 2 span, i.e. reversing the order of my current implementation:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤔 👍 don't have any strong opinion here and would need more time to read your comment to fully understand the impact. So I am fine with whatever you chose to do here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To make the error suppression work correctly, we have to ensure that when a bound like trait Super<T> {
type Assoc;
}
trait Sub<T>: Super<T, Assoc = T> {}
trait TargetTrait
where
Self::Assoc1<()>: Sub<Self::Assoc2>
{
type Assoc1<T>;
type Assoc2;
}
impl Super<u16> for () {
type Assoc = u8;
}
impl TargetTrait for u8 {
type Assoc1<T> = ();
type Assoc2 = u16;
} What the error should look like:
What the error would looks like if we made the obligation
So with that being the case and because Nobody Writes Code Like That Anyway[citation needed] I'd like to keep the current implementation, but add a comment and a test. |
||
cause.span = impl_item_span; | ||
} | ||
} | ||
|
||
ty::PredicateKind::Clause(ty::ClauseKind::Trait(pred)) => { | ||
// An associated item obligation born out of the `trait` failed to be met. An example | ||
// can be seen in `ui/associated-types/point-at-type-on-obligation-failure-2.rs`. | ||
// Form 3: A trait obligation for an associated item failed to be met. | ||
debug!("extended_cause_with_original_assoc_item_obligation trait proj {:?}", pred); | ||
if let ty::Alias(ty::Projection, ty::AliasTy { def_id, .. }) = *pred.self_ty().kind() | ||
&& let Some(&impl_item_id) = tcx.impl_item_implementor_ids(impl_def_id).get(&def_id) | ||
&& let Some(impl_item_span) = items | ||
.iter() | ||
.find(|item| item.id.owner_id.to_def_id() == impl_item_id) | ||
.map(fix_span) | ||
{ | ||
if let Some(impl_item_span) = ty_to_impl_span(pred.self_ty()) { | ||
cause.span = impl_item_span; | ||
} | ||
} | ||
|
@@ -355,9 +382,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> { | |
traits::ObligationCauseCode::DerivedObligation, | ||
); | ||
} | ||
extend_cause_with_original_assoc_item_obligation( | ||
tcx, trait_ref, item, &mut cause, predicate, | ||
); | ||
extend_cause_with_original_assoc_item_obligation(tcx, item, &mut cause, predicate); | ||
traits::Obligation::with_depth(tcx, cause, depth, param_env, 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.
😁