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

arbitrary_self_type: insert implied Receiver bound on Deref #138952

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+327 −339
Open

arbitrary_self_type: insert implied Receiver bound on Deref #138952

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
44 changes: 41 additions & 3 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use rustc_hir::def::{DefKind, Res};
use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS;
use rustc_middle::ty::elaborate::ClauseWithSupertraitSpan;
use rustc_middle::ty::{
self, BottomUpFolder, DynKind, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeFoldable,
TypeVisitableExt, Upcast,
self, BottomUpFolder, DynKind, ExistentialPredicateStableCmpExt as _, Interner as _, Ty,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you importing Interner? There should be no reason to use that trait.

TyCtxt, TypeFoldable, TypeVisitableExt, Upcast,
};
use rustc_span::{ErrorGuaranteed, Span};
use rustc_trait_selection::error_reporting::traits::report_dyn_incompatibility;
Expand Down Expand Up @@ -112,9 +112,29 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// let _: &dyn Alias<Assoc = u32> = /* ... */;
// ```
let mut projection_bounds = FxIndexMap::default();
let mut auto_fill_bounds: SmallVec<[_; 4]> = smallvec![];
let mut push_auto_fill_receiver_bound = |pred: ty::Binder<'tcx, _>| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit overkill. Just take the projection and replace its def id with the one for Receiver::Target.

At that point, this doesn't need to be a callback, just an additional call to insert into the projection bounds map that you can duplicate twice.

let receiver_target_did =
tcx.require_lang_item(rustc_hir::LangItem::ReceiverTarget, None);
let receiver_trait_ref =
tcx.anonymize_bound_vars(pred.map_bound(|proj: ty::ProjectionPredicate<'_>| {
tcx.trait_ref_and_own_args_for_alias(
receiver_target_did,
proj.projection_term.args,
)
.0
}));
let receiver_pred = pred.map_bound(|mut pred| {
pred.projection_term.def_id = receiver_target_did;
pred
});
auto_fill_bounds.push((receiver_target_did, receiver_trait_ref, receiver_pred, span));
};
for (proj, proj_span) in elaborated_projection_bounds {
debug!(?proj, "elaborated proj bound");
let proj_did = proj.skip_binder().projection_term.def_id;
let key = (
proj.skip_binder().projection_term.def_id,
proj_did,
tcx.anonymize_bound_vars(
proj.map_bound(|proj| proj.projection_term.trait_ref(tcx)),
),
Expand Down Expand Up @@ -142,6 +162,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
)
.emit();
}
if tcx.is_lang_item(proj_did, rustc_hir::LangItem::DerefTarget) {
push_auto_fill_receiver_bound(proj);
}
}

let principal_trait = regular_traits.into_iter().next();
Expand Down Expand Up @@ -221,6 +244,14 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
if !projection_bounds.contains_key(&key) {
projection_bounds.insert(key, (pred, supertrait_span));
}
if tcx.is_lang_item(
pred.skip_binder().def_id(),
rustc_hir::LangItem::DerefTarget,
) {
// We need to fill in a `Receiver<Target = ?>` bound because
Copy link
Member

@compiler-errors compiler-errors Mar 26, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is missing the more important part that a supertrait like Deref: Receiver<Target = Self::Target> does not automatically elaborate into a projection bound for Reciever's Target type.

// neither `Receiver` nor `Deref` is an auto-trait.
push_auto_fill_receiver_bound(pred);
}
}

self.check_elaborated_projection_mentions_input_lifetimes(
Expand All @@ -233,6 +264,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}
}
}
for (did, trait_ref, pred, span) in auto_fill_bounds {
if !projection_bounds.contains_key(&(did, trait_ref)) {
projection_bounds.insert((did, trait_ref), (pred, span));
}
}

// `dyn Trait<Assoc = Foo>` desugars to (not Rust syntax) `dyn Trait where
// <Self as Trait>::Assoc = Foo`. So every `Projection` clause is an
Expand Down Expand Up @@ -264,6 +300,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}
})
.collect();
debug!(?missing_assoc_types, ?projection_bounds);

if let Err(guar) = self.check_for_required_assoc_tys(
principal_trait.as_ref().map_or(smallvec![], |(_, spans)| spans.clone()),
Expand Down Expand Up @@ -378,6 +415,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
.collect::<SmallVec<[_; 8]>>();
v.sort_by(|a, b| a.skip_binder().stable_cmp(tcx, &b.skip_binder()));
let existential_predicates = tcx.mk_poly_existential_predicates(&v);
debug!(?existential_predicates);

// Use explicitly-specified region bound, unless the bound is missing.
let region_bound = if !lifetime.is_elided() {
Expand Down
164 changes: 91 additions & 73 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1036,90 +1036,108 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
};
debug!(?bound);

if let Some(bound2) = matching_candidates.next() {
debug!(?bound2);
let Some(bound2) = matching_candidates.next() else { return Ok(bound) };
let mut matching_candidates = matching_candidates.peekable();

let is_receiver_target = |def_id| tcx.is_lang_item(def_id, rustc_hir::LangItem::Receiver);
let is_deref_target = |def_id| tcx.is_lang_item(def_id, rustc_hir::LangItem::Deref);
// Since `Deref::Target` and `Receiver::Target` are forced to be the same,
// the `Deref::Target` should be taken.
if assoc_name.name == sym::Target && matching_candidates.peek().is_none() {
if is_deref_target(bound.skip_binder().def_id)
&& is_receiver_target(bound2.skip_binder().def_id)
{
return Ok(bound);
}
if is_receiver_target(bound.skip_binder().def_id)
&& is_deref_target(bound2.skip_binder().def_id)
{
return Ok(bound2);
}
}

let assoc_kind_str = errors::assoc_kind_str(assoc_kind);
let qself_str = qself.to_string(tcx);
let mut err = self.dcx().create_err(crate::errors::AmbiguousAssocItem {
span,
assoc_kind: assoc_kind_str,
assoc_name,
qself: &qself_str,
});
// Provide a more specific error code index entry for equality bindings.
err.code(
if let Some(constraint) = constraint
&& let hir::AssocItemConstraintKind::Equality { .. } = constraint.kind
{
E0222
} else {
E0221
},
);
debug!(?bound2);

// FIXME(#97583): Print associated item bindings properly (i.e., not as equality predicates!).
// FIXME: Turn this into a structured, translateable & more actionable suggestion.
let mut where_bounds = vec![];
for bound in [bound, bound2].into_iter().chain(matching_candidates) {
let bound_id = bound.def_id();
let bound_span = tcx
.associated_items(bound_id)
.find_by_name_and_kind(tcx, assoc_name, assoc_kind, bound_id)
.and_then(|item| tcx.hir().span_if_local(item.def_id));

if let Some(bound_span) = bound_span {
err.span_label(
bound_span,
format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),),
);
if let Some(constraint) = constraint {
match constraint.kind {
hir::AssocItemConstraintKind::Equality { term } => {
let term: ty::Term<'_> = match term {
hir::Term::Ty(ty) => self.lower_ty(ty).into(),
hir::Term::Const(ct) => {
self.lower_const_arg(ct, FeedConstTy::No).into()
}
};
if term.references_error() {
continue;
let assoc_kind_str = errors::assoc_kind_str(assoc_kind);
let qself_str = qself.to_string(tcx);
let mut err = self.dcx().create_err(crate::errors::AmbiguousAssocItem {
span,
assoc_kind: assoc_kind_str,
assoc_name,
qself: &qself_str,
});
// Provide a more specific error code index entry for equality bindings.
err.code(
if let Some(constraint) = constraint
&& let hir::AssocItemConstraintKind::Equality { .. } = constraint.kind
{
E0222
} else {
E0221
},
);

// FIXME(#97583): Print associated item bindings properly (i.e., not as equality predicates!).
// FIXME: Turn this into a structured, translateable & more actionable suggestion.
let mut where_bounds = vec![];
for bound in [bound, bound2].into_iter().chain(matching_candidates) {
let bound_id = bound.def_id();
let bound_span = tcx
.associated_items(bound_id)
.find_by_name_and_kind(tcx, assoc_name, assoc_kind, bound_id)
.and_then(|item| tcx.hir().span_if_local(item.def_id));

if let Some(bound_span) = bound_span {
err.span_label(
bound_span,
format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),),
);
if let Some(constraint) = constraint {
match constraint.kind {
hir::AssocItemConstraintKind::Equality { term } => {
let term: ty::Term<'_> = match term {
hir::Term::Ty(ty) => self.lower_ty(ty).into(),
hir::Term::Const(ct) => {
self.lower_const_arg(ct, FeedConstTy::No).into()
}
// FIXME(#97583): This isn't syntactically well-formed!
where_bounds.push(format!(
" T: {trait}::{assoc_name} = {term}",
trait = bound.print_only_trait_path(),
));
};
if term.references_error() {
continue;
}
// FIXME: Provide a suggestion.
hir::AssocItemConstraintKind::Bound { bounds: _ } => {}
// FIXME(#97583): This isn't syntactically well-formed!
where_bounds.push(format!(
" T: {trait}::{assoc_name} = {term}",
trait = bound.print_only_trait_path(),
));
}
} else {
err.span_suggestion_verbose(
span.with_hi(assoc_name.span.lo()),
"use fully-qualified syntax to disambiguate",
format!("<{qself_str} as {}>::", bound.print_only_trait_path()),
Applicability::MaybeIncorrect,
);
// FIXME: Provide a suggestion.
hir::AssocItemConstraintKind::Bound { bounds: _ } => {}
}
} else {
err.note(format!(
"associated {assoc_kind_str} `{assoc_name}` could derive from `{}`",
bound.print_only_trait_path(),
));
err.span_suggestion_verbose(
span.with_hi(assoc_name.span.lo()),
"use fully-qualified syntax to disambiguate",
format!("<{qself_str} as {}>::", bound.print_only_trait_path()),
Applicability::MaybeIncorrect,
);
}
}
if !where_bounds.is_empty() {
err.help(format!(
"consider introducing a new type parameter `T` and adding `where` constraints:\
\n where\n T: {qself_str},\n{}",
where_bounds.join(",\n"),
} else {
err.note(format!(
"associated {assoc_kind_str} `{assoc_name}` could derive from `{}`",
bound.print_only_trait_path(),
));
let reported = err.emit();
return Err(reported);
}
err.emit();
}
if !where_bounds.is_empty() {
err.help(format!(
"consider introducing a new type parameter `T` and adding `where` constraints:\
\n where\n T: {qself_str},\n{}",
where_bounds.join(",\n"),
));
let reported = err.emit();
return Err(reported);
}
err.emit();

Ok(bound)
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_middle/src/ty/print/pretty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,8 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
if let Some(bound_principal) = predicates.principal() {
self.wrap_binder(&bound_principal, WrapBinderMode::ForAll, |principal, cx| {
define_scoped_cx!(cx);
let principal_deref_trait =
cx.tcx().is_lang_item(principal.def_id, LangItem::Deref);
p!(print_def_path(principal.def_id, &[]));

let mut resugared = false;
Expand Down Expand Up @@ -1451,7 +1453,11 @@ pub trait PrettyPrinter<'tcx>: Printer<'tcx> + fmt::Write {
let super_proj = cx.tcx().anonymize_bound_vars(super_proj);

proj == super_proj
});
}) || (principal_deref_trait
&& cx.tcx().is_lang_item(
proj.skip_binder().def_id,
LangItem::ReceiverTarget,
));
!proj_is_implied
})
.map(|proj| {
Expand Down
35 changes: 16 additions & 19 deletions compiler/rustc_middle/src/ty/sty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -721,26 +721,23 @@ impl<'tcx> Ty<'tcx> {
) -> Ty<'tcx> {
if cfg!(debug_assertions) {
let projection_count = obj.projection_bounds().count();
let expected_count: usize = obj
.principal_def_id()
.into_iter()
.flat_map(|principal_def_id| {
// NOTE: This should agree with `needed_associated_types` in
// dyn trait lowering, or else we'll have ICEs.
elaborate::supertraits(
tcx,
ty::Binder::dummy(ty::TraitRef::identity(tcx, principal_def_id)),
)
.map(|principal| {
tcx.associated_items(principal.def_id())
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Type)
.filter(|item| !item.is_impl_trait_in_trait())
.filter(|item| !tcx.generics_require_sized_self(item.def_id))
.count()
})
let expected_count = obj.principal_def_id().map_or(0, |principal_def_id| {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this changed?

// NOTE: This should agree with `needed_associated_types` in
// dyn trait lowering, or else we'll have ICEs.
elaborate::supertraits(
tcx,
ty::Binder::dummy(ty::TraitRef::identity(tcx, principal_def_id)),
)
.map(|principal| {
tcx.associated_items(principal.def_id())
.in_definition_order()
.filter(|item| item.kind == ty::AssocKind::Type)
.filter(|item| !item.is_impl_trait_in_trait())
.filter(|item| !tcx.generics_require_sized_self(item.def_id))
.count()
})
.sum();
.sum()
});
assert_eq!(
projection_count, expected_count,
"expected {obj:?} to have {expected_count} projections, \
Expand Down
Loading
Loading