Fix dyn incompleteness

Author: compiler-errors

compiler/rustc_hir_analysis/src/hir_ty_lowering/dyn_compatibility.rs

@@ -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,
@@ -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,
@@ -80,12 +84,42 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             }
         }
 
-        let (trait_bounds, mut projection_bounds) =
+        let (expanded_trait_bounds, expanded_projection_bounds) =
             traits::expand_trait_aliases(tcx, user_written_bounds.clauses());
-        let (regular_traits, mut auto_traits): (Vec<_>, Vec<_>) = trait_bounds
+        let (regular_traits, mut auto_traits): (Vec<_>, Vec<_>) = expanded_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.
+        let mut projection_bounds = FxIndexMap::default();
+        for (proj, proj_span) in expanded_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 =
@@ -105,6 +139,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
@@ -134,14 +173,17 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                         );
                     }
                     ty::PredicateKind::Clause(ty::ClauseKind::Projection(pred)) => {
-                        let pred = bound_predicate.rebind(pred);
+                        let elaborated_projection = bound_predicate.rebind(pred);
                         // A `Self` within the original bound will be instantiated with a
                         // `trait_object_dummy_self`, so check for that.
-                        let references_self = match pred.skip_binder().term.unpack() {
-                            ty::TermKind::Ty(ty) => ty.walk().any(|arg| arg == dummy_self.into()),
-                            // FIXME(associated_const_equality): We should walk the const instead of not doing anything
-                            ty::TermKind::Const(_) => false,
-                        };
+                        let references_self =
+                            match elaborated_projection.skip_binder().term.unpack() {
+                                ty::TermKind::Ty(ty) => {
+                                    ty.walk().any(|arg| arg == dummy_self.into())
+                                }
+                                // FIXME(associated_const_equality): We should walk the const instead of not doing anything
+                                ty::TermKind::Const(_) => false,
+                            };
 
                         // If the projection output contains `Self`, force the user to
                         // elaborate it explicitly to avoid a lot of complexity.
@@ -162,11 +204,38 @@ 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(elaborated_projection);
+
+                            if let Some((user_written_projection, user_written_span)) =
+                                projection_bounds.shift_remove(&tcx.anonymize_bound_vars(
+                                    elaborated_projection.map_bound(|pred| pred.projection_term),
+                                ))
+                            {
+                                if tcx.anonymize_bound_vars(user_written_projection)
+                                    == tcx.anonymize_bound_vars(elaborated_projection)
+                                {
+                                    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,
+                                        elaborated_projection,
+                                        principal_trait.def_id(),
+                                        user_written_span,
+                                        supertrait_span,
+                                    );
+                                }
+                            }
                         }
 
                         self.check_elaborated_projection_mentions_input_lifetimes(
-                            pred,
+                            elaborated_projection,
                             *spans.first().unwrap(),
                             supertrait_span,
                         );
@@ -176,16 +245,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             }
         }
 
-        // `dyn Trait<Assoc = Foo>` desugars to (not Rust syntax) `dyn Trait where <Self as Trait>::Assoc = Foo`.
-        // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types` contains all associated
-        // types's `DefId`, so the following loop removes all the `DefIds` of the associated types that have a
-        // corresponding `Projection` clause
-        for (projection_bound, span) in &projection_bounds {
+        // `dyn Trait<Assoc = Foo>` desugars to something like `dyn Trait + (Trait::Assoc = Foo)`.
+        // So every `Projection` clause is an `Assoc = Foo` bound. `associated_types`
+        // contains all associated types's `DefId`, so the following loop removes all
+        // the `DefIds` of the associated types that have a corresponding `Projection`.
+        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,
@@ -195,6 +264,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()),
@@ -272,7 +348,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);
 
@@ -349,6 +425,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 implied by 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

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

@@ -722,7 +722,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
     /// emit a generic note suggesting using a `where` clause to constraint instead.
     pub(crate) fn check_for_required_assoc_tys(
         &self,
-        principal_span: Span,
+        spans: SmallVec<[Span; 1]>,
         missing_assoc_types: FxIndexSet<(DefId, ty::PolyTraitRef<'tcx>)>,
         potential_assoc_types: Vec<usize>,
         trait_bounds: &[hir::PolyTraitRef<'_>],

compiler/rustc_hir_typeck/src/closure.rs

@@ -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};
@@ -312,16 +312,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),
+                        DUMMY_SP,
+                    )
+                }),
+            ),
             ty::Infer(ty::TyVar(vid)) => self.deduce_closure_signature_from_predicates(
                 Ty::new_var(self.tcx, self.root_var(vid)),
                 closure_kind,

compiler/rustc_lint_defs/src/builtin.rs

@@ -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,
@@ -5113,6 +5115,20 @@ declare_lint! {
     crate_level_only
 }
 
+declare_lint! {
+    /// Hi
+    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.

compiler/rustc_middle/src/ty/relate.rs

@@ -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(),
@@ -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)
     }
 }

compiler/rustc_middle/src/ty/sty.rs

@@ -982,6 +982,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

compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

@@ -696,6 +696,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),
+                ));
+            }
         }
     }