Skip to content

Fix dyn incompleteness with multiple supertraits with different substitutions #133397

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_cranelift/example/issue-59326.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// Based on https://github.com/rust-lang/rust/blob/689511047a75a30825e367d4fd45c74604d0b15e/tests/ui/issues/issue-59326.rs#L1
// check-pass

#![allow(dyn_assoc_redundant, dyn_assoc_shadowed)]

trait Service {
type S;
}
Expand Down
143 changes: 135 additions & 8 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_errors::codes::*;
use rustc_errors::struct_span_code_err;
use rustc_hir as hir;
use rustc_hir::HirId;
use rustc_hir::def::{DefKind, Res};
use rustc_lint_defs::builtin::UNUSED_ASSOCIATED_TYPE_BOUNDS;
use rustc_hir::def_id::DefId;
use rustc_lint_defs::builtin::{
DYN_ASSOC_REDUNDANT, DYN_ASSOC_SHADOWED, UNUSED_ASSOCIATED_TYPE_BOUNDS,
};
use rustc_middle::ty::fold::BottomUpFolder;
use rustc_middle::ty::{
self, DynKind, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeFoldable,
Expand All @@ -28,7 +32,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
pub(super) fn lower_trait_object_ty(
&self,
span: Span,
hir_id: hir::HirId,
hir_id: HirId,
hir_bounds: &[hir::PolyTraitRef<'tcx>],
lifetime: &hir::Lifetime,
representation: DynKind,
Expand Down Expand Up @@ -59,12 +63,49 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}
}

let (trait_bounds, mut projection_bounds) =
let (trait_bounds, elaborated_projection_bounds) =
traits::expand_trait_aliases(tcx, user_written_bounds.clauses());
let (regular_traits, mut auto_traits): (Vec<_>, Vec<_>) = trait_bounds
.into_iter()
.partition(|(trait_ref, _)| !tcx.trait_is_auto(trait_ref.def_id()));

// Map the projection bounds onto a key that makes it easy to remove redundant
// bounds that are constrained by supertraits of the principal def id.
//
// Also make sure we detect conflicting bounds from expanding a trait alias and
// also specifying it manually, like:
// ```
// type Alias = Trait<Assoc = i32>;
// let _: &dyn Alias<Assoc = u32> = /* ... */;
// ```
let mut projection_bounds = FxIndexMap::default();
for (proj, proj_span) in elaborated_projection_bounds {
if let Some((old_proj, old_proj_span)) = projection_bounds.insert(
tcx.anonymize_bound_vars(proj.map_bound(|proj| proj.projection_term)),
(proj, proj_span),
) && tcx.anonymize_bound_vars(proj) != tcx.anonymize_bound_vars(old_proj)
{
let item = tcx.item_name(proj.item_def_id());
self.dcx()
.struct_span_err(
span,
format!(
"conflicting associated type bounds for `{item}` when \
expanding trait alias"
),
)
.with_span_label(
old_proj_span,
format!("`{item}` is specified to be `{}` here", old_proj.term()),
)
.with_span_label(
proj_span,
format!("`{item}` is specified to be `{}` here", proj.term()),
)
.emit();
}
}

// We don't support empty trait objects.
if regular_traits.is_empty() && auto_traits.is_empty() {
let guar =
Expand Down Expand Up @@ -105,6 +146,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
let principal_trait = regular_traits.into_iter().next();

let mut needed_associated_types = FxIndexSet::default();

// These are the projection bounds that we get from supertraits that
// don't mention the dyn trait recursively. See comment below.
let mut implied_projection_bounds = vec![];

if let Some((principal_trait, spans)) = &principal_trait {
let pred: ty::Predicate<'tcx> = (*principal_trait).upcast(tcx);
for ClauseWithSupertraitSpan { pred, supertrait_span } in
Expand Down Expand Up @@ -162,7 +208,34 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// the discussion in #56288 for alternatives.
if !references_self {
// Include projections defined on supertraits.
projection_bounds.push((pred, supertrait_span));
implied_projection_bounds.push(pred);

if let Some((user_written_projection, user_written_span)) =
projection_bounds.shift_remove(&tcx.anonymize_bound_vars(
pred.map_bound(|pred| pred.projection_term),
))
{
if tcx.anonymize_bound_vars(user_written_projection)
== tcx.anonymize_bound_vars(pred)
{
self.lint_redundant_projection(
hir_id,
user_written_projection,
principal_trait.def_id(),
user_written_span,
supertrait_span,
);
} else {
self.lint_shadowed_projection(
hir_id,
user_written_projection,
pred,
principal_trait.def_id(),
user_written_span,
supertrait_span,
);
}
}
}

self.check_elaborated_projection_mentions_input_lifetimes(
Expand All @@ -182,12 +255,12 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// types that we expect to be provided by the user, so the following loop
// removes all the associated types that have a corresponding `Projection`
// clause, either from expanding trait aliases or written by the user.
for &(projection_bound, span) in &projection_bounds {
for &(projection_bound, span) in projection_bounds.values() {
let def_id = projection_bound.item_def_id();
let trait_ref = tcx.anonymize_bound_vars(
projection_bound.map_bound(|p| p.projection_term.trait_ref(tcx)),
);
needed_associated_types.swap_remove(&(def_id, trait_ref));
needed_associated_types.shift_remove(&(def_id, trait_ref));
if tcx.generics_require_sized_self(def_id) {
tcx.emit_node_span_lint(
UNUSED_ASSOCIATED_TYPE_BOUNDS,
Expand All @@ -197,6 +270,13 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
);
}
}
for projection_bound in &implied_projection_bounds {
let def_id = projection_bound.item_def_id();
let trait_ref = tcx.anonymize_bound_vars(
projection_bound.map_bound(|p| p.projection_term.trait_ref(tcx)),
);
needed_associated_types.swap_remove(&(def_id, trait_ref));
}

if let Err(guar) = self.check_for_required_assoc_tys(
principal_trait.as_ref().map_or(smallvec![], |(_, spans)| spans.clone()),
Expand Down Expand Up @@ -266,7 +346,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
})
});

let existential_projections = projection_bounds.iter().map(|(bound, _)| {
let existential_projections = projection_bounds.values().map(|(bound, _)| {
bound.map_bound(|mut b| {
assert_eq!(b.projection_term.self_ty(), dummy_self);

Expand Down Expand Up @@ -343,6 +423,53 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
Ty::new_dynamic(tcx, existential_predicates, region_bound, representation)
}

fn lint_shadowed_projection(
&self,
hir_id: HirId,
user_written_projection: ty::PolyProjectionPredicate<'tcx>,
elaborated_projection: ty::PolyProjectionPredicate<'tcx>,
principal_def_id: DefId,
user_written_span: Span,
supertrait_span: Span,
) {
let tcx = self.tcx();
let assoc = tcx.item_name(user_written_projection.item_def_id());
let principal = tcx.item_name(principal_def_id);
self.tcx().node_span_lint(DYN_ASSOC_SHADOWED, hir_id, user_written_span, |diag| {
diag.primary_message(format!(
"associated type bound for `{assoc}` in `dyn {principal}` differs from \
associated type bound from supertrait",
));
diag.span_label(user_written_span, "this bound has no effect and will be ignored");
diag.note(format!(
"`{assoc} = {}` was implied by a supertrait and shadows any user-written bounds, \
so `{assoc} = {}` will be ignored",
elaborated_projection.term(),
user_written_projection.term(),
));
diag.span_label(supertrait_span, "shadowed due to this supertrait bound");
});
}

fn lint_redundant_projection(
&self,
hir_id: HirId,
user_written_projection: ty::PolyProjectionPredicate<'tcx>,
principal_def_id: DefId,
user_written_span: Span,
supertrait_span: Span,
) {
let tcx = self.tcx();
let assoc = tcx.item_name(user_written_projection.item_def_id());
let principal = tcx.item_name(principal_def_id);
self.tcx().node_span_lint(DYN_ASSOC_REDUNDANT, hir_id, user_written_span, |diag| {
diag.primary_message(format!(
"associated type bound for `{assoc}` in `dyn {principal}` is redundant",
));
diag.span_label(supertrait_span, "redundant due to this supertrait bound");
});
}

/// Check that elaborating the principal of a trait ref doesn't lead to projections
/// that are unconstrained. This can happen because an otherwise unconstrained
/// *type variable* can be substituted with a type that has late-bound regions. See
Expand Down
26 changes: 15 additions & 11 deletions compiler/rustc_hir_typeck/src/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_span::def_id::LocalDefId;
use rustc_span::{DUMMY_SP, Span};
use rustc_trait_selection::error_reporting::traits::ArgKind;
use rustc_trait_selection::traits;
use rustc_type_ir::ClosureKind;
use rustc_type_ir::{ClosureKind, Upcast as _};
use tracing::{debug, instrument, trace};

use super::{CoroutineTypes, Expectation, FnCtxt, check_fn};
Expand Down Expand Up @@ -61,6 +61,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(ty) => self.deduce_closure_signature(
self.try_structurally_resolve_type(expr_span, ty),
closure.kind,
expr_span,
),
None => (None, None),
};
Expand Down Expand Up @@ -301,6 +302,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
expected_ty: Ty<'tcx>,
closure_kind: hir::ClosureKind,
span: Span,
) -> (Option<ExpectedSig<'tcx>>, Option<ty::ClosureKind>) {
match *expected_ty.kind() {
ty::Alias(ty::Opaque, ty::AliasTy { def_id, args, .. }) => self
Expand All @@ -312,16 +314,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.iter_instantiated_copied(self.tcx, args)
.map(|(c, s)| (c.as_predicate(), s)),
),
ty::Dynamic(object_type, ..) => {
let sig = object_type.projection_bounds().find_map(|pb| {
let pb = pb.with_self_ty(self.tcx, self.tcx.types.trait_object_dummy_self);
self.deduce_sig_from_projection(None, closure_kind, pb)
});
let kind = object_type
.principal_def_id()
.and_then(|did| self.tcx.fn_trait_kind_from_def_id(did));
(sig, kind)
}
ty::Dynamic(data, ..) => self.deduce_closure_signature_from_predicates(
self.tcx.types.trait_object_dummy_self,
closure_kind,
data.iter().map(|bound| {
(
bound
.with_self_ty(self.tcx, self.tcx.types.trait_object_dummy_self)
.upcast(self.tcx),
span,
)
}),
),
ty::Infer(ty::TyVar(vid)) => self.deduce_closure_signature_from_predicates(
Ty::new_var(self.tcx, self.root_var(vid)),
closure_kind,
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ declare_lint_pass! {
DEPRECATED_SAFE_2024,
DEPRECATED_WHERE_CLAUSE_LOCATION,
DUPLICATE_MACRO_ATTRIBUTES,
DYN_ASSOC_REDUNDANT,
DYN_ASSOC_SHADOWED,
ELIDED_LIFETIMES_IN_ASSOCIATED_CONSTANT,
ELIDED_LIFETIMES_IN_PATHS,
ELIDED_NAMED_LIFETIMES,
Expand Down Expand Up @@ -5120,6 +5122,20 @@ declare_lint! {
crate_level_only
}

declare_lint! {
/// Hi
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right I need lint docs.

pub DYN_ASSOC_REDUNDANT,
Warn,
"oops",
}

declare_lint! {
/// Hi
pub DYN_ASSOC_SHADOWED,
Deny,
"oops",
}

declare_lint! {
/// The `abi_unsupported_vector_types` lint detects function definitions and calls
/// whose ABI depends on enabling certain target features, but those features are not enabled.
Expand Down
16 changes: 4 additions & 12 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,12 @@ impl<'tcx> Relate<TyCtxt<'tcx>> for &'tcx ty::List<ty::PolyExistentialPredicate<
) -> RelateResult<'tcx, Self> {
let tcx = relation.cx();

// FIXME: this is wasteful, but want to do a perf run to see how slow it is.
// We need to perform this deduplication as we sometimes generate duplicate projections
// in `a`.
let mut a_v: Vec<_> = a.into_iter().collect();
let mut b_v: Vec<_> = b.into_iter().collect();
a_v.dedup();
b_v.dedup();
if a_v.len() != b_v.len() {
if a.len() != b.len() {
return Err(TypeError::ExistentialMismatch(ExpectedFound::new(a, b)));
}

let v = iter::zip(a_v, b_v).map(|(ep_a, ep_b)| {
match (ep_a.skip_binder(), ep_b.skip_binder()) {
let v =
iter::zip(a, b).map(|(ep_a, ep_b)| match (ep_a.skip_binder(), ep_b.skip_binder()) {
(ty::ExistentialPredicate::Trait(a), ty::ExistentialPredicate::Trait(b)) => {
Ok(ep_a.rebind(ty::ExistentialPredicate::Trait(
relation.relate(ep_a.rebind(a), ep_b.rebind(b))?.skip_binder(),
Expand All @@ -109,8 +102,7 @@ impl<'tcx> Relate<TyCtxt<'tcx>> for &'tcx ty::List<ty::PolyExistentialPredicate<
ty::ExistentialPredicate::AutoTrait(b),
) if a == b => Ok(ep_a.rebind(ty::ExistentialPredicate::AutoTrait(a))),
_ => Err(TypeError::ExistentialMismatch(ExpectedFound::new(a, b))),
}
});
});
tcx.mk_poly_existential_predicates_from_iter(v)
}
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,10 @@ impl<'tcx> rustc_type_ir::inherent::Ty<TyCtxt<'tcx>> for Ty<'tcx> {
fn has_unsafe_fields(self) -> bool {
Ty::has_unsafe_fields(self)
}

fn is_self_param(self) -> bool {
self.is_param(0)
}
}

/// Type utilities
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,15 @@ where
assumption.upcast(cx),
));
}

for assumption in elaborate::implied_supertrait_projections(cx, principal) {
candidates.extend(G::probe_and_consider_object_bound_candidate(
self,
CandidateSource::BuiltinImpl(BuiltinImplSource::Misc),
goal,
assumption.with_self_ty(cx, self_ty).upcast(cx),
));
}
}
}

Expand Down
Loading
Loading