Skip to content

Commit

Permalink
Rollup merge of #121863 - lukas-code:silence-mismatched-super-project…
Browse files Browse the repository at this point in the history
…ions, r=lcnr

silence mismatched types errors for implied projections

Currently, if a trait bound is not satisfied, then we suppress any errors for the trait's supertraits not being satisfied, but still report errors for super projections not being satisfied.

For example:
```rust
trait Super {
    type Assoc;
}
trait Sub: Super<Assoc = ()> {}
```
Before this PR, if `T: Sub` is not satisfied, then errors for `T: Super` are suppressed, but errors for `<T as Super>::Assoc == ()` are still shown. This PR makes it so that errors about super projections not being satisfied are also suppressed.

The errors are only suppressed if the span of the trait obligation matches the span of the super predicate obligation to avoid silencing error that are not related. This PR removes some differences between the spans of supertraits and super projections to make the suppression work correctly.

This PR fixes the majority of the diagnostics fallout when making `Thin` a supertrait of `Sized` (in a future PR).
cc #120354 (comment)
cc `@lcnr`
  • Loading branch information
GuillaumeGomez authored Mar 7, 2024
2 parents 8c9a75b + db48b93 commit 0e37648
Show file tree
Hide file tree
Showing 10 changed files with 310 additions and 102 deletions.
46 changes: 28 additions & 18 deletions compiler/rustc_hir_analysis/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,31 +298,31 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) -> Result<()
hir::ItemKind::Const(ty, ..) => {
check_item_type(tcx, def_id, ty.span, UnsizedHandling::Forbid)
}
hir::ItemKind::Struct(_, ast_generics) => {
hir::ItemKind::Struct(_, hir_generics) => {
let res = check_type_defn(tcx, item, false);
check_variances_for_type_defn(tcx, item, ast_generics);
check_variances_for_type_defn(tcx, item, hir_generics);
res
}
hir::ItemKind::Union(_, ast_generics) => {
hir::ItemKind::Union(_, hir_generics) => {
let res = check_type_defn(tcx, item, true);
check_variances_for_type_defn(tcx, item, ast_generics);
check_variances_for_type_defn(tcx, item, hir_generics);
res
}
hir::ItemKind::Enum(_, ast_generics) => {
hir::ItemKind::Enum(_, hir_generics) => {
let res = check_type_defn(tcx, item, true);
check_variances_for_type_defn(tcx, item, ast_generics);
check_variances_for_type_defn(tcx, item, hir_generics);
res
}
hir::ItemKind::Trait(..) => check_trait(tcx, item),
hir::ItemKind::TraitAlias(..) => check_trait(tcx, item),
// `ForeignItem`s are handled separately.
hir::ItemKind::ForeignMod { .. } => Ok(()),
hir::ItemKind::TyAlias(hir_ty, ast_generics) => {
hir::ItemKind::TyAlias(hir_ty, hir_generics) => {
if tcx.type_alias_is_lazy(item.owner_id) {
// Bounds of lazy type aliases and of eager ones that contain opaque types are respected.
// E.g: `type X = impl Trait;`, `type X = (impl Trait, Y);`.
let res = check_item_type(tcx, def_id, hir_ty.span, UnsizedHandling::Allow);
check_variances_for_type_defn(tcx, item, ast_generics);
check_variances_for_type_defn(tcx, item, hir_generics);
res
} else {
Ok(())
Expand Down Expand Up @@ -1277,25 +1277,26 @@ fn check_item_type(
})
}

#[instrument(level = "debug", skip(tcx, ast_self_ty, ast_trait_ref))]
#[instrument(level = "debug", skip(tcx, hir_self_ty, hir_trait_ref))]
fn check_impl<'tcx>(
tcx: TyCtxt<'tcx>,
item: &'tcx hir::Item<'tcx>,
ast_self_ty: &hir::Ty<'_>,
ast_trait_ref: &Option<hir::TraitRef<'_>>,
hir_self_ty: &hir::Ty<'_>,
hir_trait_ref: &Option<hir::TraitRef<'_>>,
) -> Result<(), ErrorGuaranteed> {
enter_wf_checking_ctxt(tcx, item.span, item.owner_id.def_id, |wfcx| {
match ast_trait_ref {
Some(ast_trait_ref) => {
match hir_trait_ref {
Some(hir_trait_ref) => {
// `#[rustc_reservation_impl]` impls are not real impls and
// therefore don't need to be WF (the trait's `Self: Trait` predicate
// won't hold).
let trait_ref = tcx.impl_trait_ref(item.owner_id).unwrap().instantiate_identity();
// Avoid bogus "type annotations needed `Foo: Bar`" errors on `impl Bar for Foo` in case
// other `Foo` impls are incoherent.
tcx.ensure().coherent_trait(trait_ref.def_id)?;
let trait_span = hir_trait_ref.path.span;
let trait_ref = wfcx.normalize(
ast_trait_ref.path.span,
trait_span,
Some(WellFormedLoc::Ty(item.hir_id().expect_owner().def_id)),
trait_ref,
);
Expand All @@ -1306,14 +1307,23 @@ fn check_impl<'tcx>(
wfcx.param_env,
wfcx.body_def_id,
trait_pred,
ast_trait_ref.path.span,
trait_span,
item,
);
for obligation in &mut obligations {
if obligation.cause.span != trait_span {
// We already have a better span.
continue;
}
if let Some(pred) = obligation.predicate.to_opt_poly_trait_pred()
&& pred.self_ty().skip_binder() == trait_ref.self_ty()
&& pred.skip_binder().self_ty() == trait_ref.self_ty()
{
obligation.cause.span = hir_self_ty.span;
}
if let Some(pred) = obligation.predicate.to_opt_poly_projection_pred()
&& pred.skip_binder().self_ty() == trait_ref.self_ty()
{
obligation.cause.span = ast_self_ty.span;
obligation.cause.span = hir_self_ty.span;
}
}
debug!(?obligations);
Expand All @@ -1327,7 +1337,7 @@ fn check_impl<'tcx>(
self_ty,
);
wfcx.register_wf_obligation(
ast_self_ty.span,
hir_self_ty.span,
Some(WellFormedLoc::Ty(item.hir_id().expect_owner().def_id)),
self_ty.into(),
);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/relate/higher_ranked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ impl<'tcx> InferCtxt<'tcx> {
// that name placeholders created in this function. Nested goals from type relations can
// also contain placeholders created by this function.
let value = self.enter_forall_and_leak_universe(forall);
debug!("?value");
debug!(?value);
f(value)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1431,45 +1431,64 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {

#[extension(pub(super) trait InferCtxtPrivExt<'tcx>)]
impl<'tcx> TypeErrCtxt<'_, 'tcx> {
fn can_match_trait(
&self,
goal: ty::TraitPredicate<'tcx>,
assumption: ty::PolyTraitPredicate<'tcx>,
) -> bool {
if goal.polarity != assumption.polarity() {
return false;
}

let trait_goal = goal.trait_ref;
let trait_assumption = self.instantiate_binder_with_fresh_vars(
DUMMY_SP,
infer::BoundRegionConversionTime::HigherRankedType,
assumption.to_poly_trait_ref(),
);

self.can_eq(ty::ParamEnv::empty(), trait_goal, trait_assumption)
}

fn can_match_projection(
&self,
goal: ty::ProjectionPredicate<'tcx>,
assumption: ty::PolyProjectionPredicate<'tcx>,
) -> bool {
let assumption = self.instantiate_binder_with_fresh_vars(
DUMMY_SP,
infer::BoundRegionConversionTime::HigherRankedType,
assumption,
);

let param_env = ty::ParamEnv::empty();
self.can_eq(param_env, goal.projection_ty, assumption.projection_ty)
&& self.can_eq(param_env, goal.term, assumption.term)
}

// returns if `cond` not occurring implies that `error` does not occur - i.e., that
// `error` occurring implies that `cond` occurs.
#[instrument(level = "debug", skip(self), ret)]
fn error_implies(&self, cond: ty::Predicate<'tcx>, error: ty::Predicate<'tcx>) -> bool {
if cond == error {
return true;
}

// FIXME: It should be possible to deal with `ForAll` in a cleaner way.
let bound_error = error.kind();
let (cond, error) = match (cond.kind().skip_binder(), bound_error.skip_binder()) {
(
ty::PredicateKind::Clause(ty::ClauseKind::Trait(..)),
ty::PredicateKind::Clause(ty::ClauseKind::Trait(error)),
) => (cond, bound_error.rebind(error)),
_ => {
// FIXME: make this work in other cases too.
return false;
}
};

for pred in elaborate(self.tcx, std::iter::once(cond)) {
let bound_predicate = pred.kind();
if let ty::PredicateKind::Clause(ty::ClauseKind::Trait(implication)) =
bound_predicate.skip_binder()
{
let error = error.to_poly_trait_ref();
let implication = bound_predicate.rebind(implication.trait_ref);
// FIXME: I'm just not taking associated types at all here.
// Eventually I'll need to implement param-env-aware
// `Γ₁ ⊦ φ₁ => Γ₂ ⊦ φ₂` logic.
let param_env = ty::ParamEnv::empty();
if self.can_sub(param_env, error, implication) {
debug!("error_implies: {:?} -> {:?} -> {:?}", cond, error, implication);
return true;
}
}
if let Some(error) = error.to_opt_poly_trait_pred() {
self.enter_forall(error, |error| {
elaborate(self.tcx, std::iter::once(cond))
.filter_map(|implied| implied.to_opt_poly_trait_pred())
.any(|implied| self.can_match_trait(error, implied))
})
} else if let Some(error) = error.to_opt_poly_projection_pred() {
self.enter_forall(error, |error| {
elaborate(self.tcx, std::iter::once(cond))
.filter_map(|implied| implied.to_opt_poly_projection_pred())
.any(|implied| self.can_match_projection(error, implied))
})
} else {
false
}

false
}

#[instrument(skip(self), level = "debug")]
Expand Down
95 changes: 60 additions & 35 deletions compiler/rustc_trait_selection/src/traits/wf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
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;
}
}
Expand Down Expand Up @@ -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)
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ where
}

impl<T: Copy + std::ops::Deref> UnsafeCopy<'_, T> for T {
//~^ type mismatch resolving `<T as Deref>::Target == T`
type Item = T;
//~^ type mismatch resolving `<T as Deref>::Target == T`
}

pub fn main() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
error[E0271]: type mismatch resolving `<T as Deref>::Target == T`
--> $DIR/hr-associated-type-projection-1.rs:13:33
--> $DIR/hr-associated-type-projection-1.rs:14:17
|
LL | impl<T: Copy + std::ops::Deref> UnsafeCopy<'_, T> for T {
| - ^^^^^^^^^^^^^^^^^ expected type parameter `T`, found associated type
| |
| expected this type parameter
| - expected this type parameter
LL | type Item = T;
| ^ expected type parameter `T`, found associated type
|
= note: expected type parameter `T`
found associated type `<T as Deref>::Target`
Expand Down
1 change: 0 additions & 1 deletion tests/ui/issues/issue-33941.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,4 @@ use std::collections::HashMap;
fn main() {
for _ in HashMap::new().iter().cloned() {} //~ ERROR expected `Iter<'_, _, _>` to be an iterator that yields `&_`, but it yields `(&_, &_)`
//~^ ERROR expected `Iter<'_, _, _>` to be an iterator that yields `&_`, but it yields `(&_, &_)`
//~| ERROR expected `Iter<'_, _, _>` to be an iterator that yields `&_`, but it yields `(&_, &_)`
}
12 changes: 1 addition & 11 deletions tests/ui/issues/issue-33941.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,6 @@ LL | for _ in HashMap::new().iter().cloned() {}
= note: required for `Cloned<std::collections::hash_map::Iter<'_, _, _>>` to implement `Iterator`
= note: required for `Cloned<std::collections::hash_map::Iter<'_, _, _>>` to implement `IntoIterator`

error[E0271]: expected `Iter<'_, _, _>` to be an iterator that yields `&_`, but it yields `(&_, &_)`
--> $DIR/issue-33941.rs:6:14
|
LL | for _ in HashMap::new().iter().cloned() {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `(&_, &_)`, found `&_`
|
= note: expected tuple `(&_, &_)`
found reference `&_`
= note: required for `Cloned<std::collections::hash_map::Iter<'_, _, _>>` to implement `Iterator`

error: aborting due to 3 previous errors
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0271`.
Loading

0 comments on commit 0e37648

Please sign in to comment.