Make the implementation better

compiler-errors · compiler-errors · commit bb80bba42796 · 2024-11-21T03:48:25.000Z
diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs
@@ -319,6 +319,14 @@ impl Key for (DefId, SimplifiedType) {
     }
 }
 
+impl Key for (DefId, Option<DefId>) {
+    type Cache<V> = DefaultCache<Self, V>;
+
+    fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
+        self.0.default_span(tcx)
+    }
+}
+
 impl<'tcx> Key for GenericArgsRef<'tcx> {
     type Cache<V> = DefaultCache<Self, V>;
 
diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs
@@ -1371,11 +1371,12 @@ rustc_queries! {
         desc { |tcx| "checking if trait `{}` is dyn-compatible", tcx.def_path_str(trait_id) }
     }
 
-    query trait_has_impl_which_may_shadow_dyn(trait_def_id: DefId) -> bool {
+    query trait_has_impl_which_may_shadow_dyn(key: (DefId, Option<DefId>)) -> bool {
         desc {
             |tcx| "checking if trait `{}` has an impl which may overlap with \
-            the built-in impl for trait objects",
-            tcx.def_path_str(trait_def_id)
+            the built-in impl for `dyn {}`",
+            tcx.def_path_str(key.0),
+            key.1.map_or(String::from("..."), |def_id| tcx.def_path_str(def_id)),
         }
     }
 
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs
@@ -581,8 +581,12 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
         self.trait_def(trait_def_id).implement_via_object
     }
 
-    fn trait_has_impl_which_may_shadow_dyn(self, trait_def_id: DefId) -> bool {
-        self.trait_has_impl_which_may_shadow_dyn(trait_def_id)
+    fn trait_has_impl_which_may_shadow_dyn(
+        self,
+        trait_def_id: DefId,
+        principal_def_id: Option<DefId>,
+    ) -> bool {
+        self.trait_has_impl_which_may_shadow_dyn((trait_def_id, principal_def_id))
     }
 
     fn is_impl_trait_in_trait(self, def_id: DefId) -> bool {
diff --git a/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs b/compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
@@ -82,7 +82,14 @@ where
         goal: Goal<I, Self>,
         assumption: I::Clause,
     ) -> Result<Candidate<I>, NoSolution> {
-        if ecx.cx().trait_has_impl_which_may_shadow_dyn(goal.predicate.trait_def_id(ecx.cx())) {
+        let ty::Dynamic(data, _, _) = goal.predicate.self_ty().kind() else {
+            unreachable!();
+        };
+
+        if ecx.cx().trait_has_impl_which_may_shadow_dyn(
+            goal.predicate.trait_def_id(ecx.cx()),
+            data.principal_def_id(),
+        ) {
             return Err(NoSolution);
         }
 
diff --git a/compiler/rustc_trait_selection/src/traits/project.rs b/compiler/rustc_trait_selection/src/traits/project.rs
@@ -864,7 +864,12 @@ fn assemble_candidates_from_object_ty<'cx, 'tcx>(
     let env_predicates = data
         .projection_bounds()
         .filter(|bound| bound.item_def_id() == obligation.predicate.def_id)
-        .filter(|bound| !tcx.trait_has_impl_which_may_shadow_dyn(bound.trait_def_id(tcx)))
+        .filter(|bound| {
+            !tcx.trait_has_impl_which_may_shadow_dyn((
+                bound.trait_def_id(tcx),
+                data.principal_def_id(),
+            ))
+        })
         .map(|p| p.with_self_ty(tcx, object_ty).upcast(tcx));
 
     assemble_candidates_from_predicates(
diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
@@ -916,7 +916,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                         })
                     })
                     .filter(|(_, trait_ref)| {
-                        !tcx.trait_has_impl_which_may_shadow_dyn(trait_ref.def_id())
+                        !tcx.trait_has_impl_which_may_shadow_dyn((
+                            trait_ref.def_id(),
+                            Some(principal_trait_ref.def_id()),
+                        ))
                     })
                     .map(|(idx, _)| ObjectCandidate(idx));
 
diff --git a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs
@@ -11,7 +11,7 @@
 
 pub mod specialization_graph;
 
-use rustc_data_structures::fx::FxIndexSet;
+use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
 use rustc_errors::codes::*;
 use rustc_errors::{Diag, EmissionGuarantee};
 use rustc_hir::LangItem;
@@ -23,6 +23,8 @@ use rustc_middle::ty::print::PrintTraitRefExt as _;
 use rustc_middle::ty::{self, GenericArgsRef, ImplSubject, Ty, TyCtxt, TypeVisitableExt};
 use rustc_session::lint::builtin::{COHERENCE_LEAK_CHECK, ORDER_DEPENDENT_TRAIT_OBJECTS};
 use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, sym};
+use rustc_type_ir::elaborate;
+use rustc_type_ir::fast_reject::{SimplifiedType, TreatParams, simplify_type};
 use specialization_graph::GraphExt;
 use tracing::{debug, instrument};
 
@@ -482,20 +484,62 @@ fn report_conflicting_impls<'tcx>(
 
 pub(super) fn trait_has_impl_which_may_shadow_dyn<'tcx>(
     tcx: TyCtxt<'tcx>,
-    trait_def_id: DefId,
+    (target_trait_def_id, principal_def_id): (DefId, Option<DefId>),
 ) -> bool {
     // We only care about trait objects which have associated types.
     if !tcx
-        .associated_items(trait_def_id)
+        .associated_items(target_trait_def_id)
         .in_definition_order()
         .any(|item| item.kind == ty::AssocKind::Type)
     {
         return false;
     }
 
-    let mut has_impl = false;
-    tcx.for_each_impl(trait_def_id, |impl_def_id| {
-        if has_impl {
+    let target_self_ty =
+        principal_def_id.map_or(SimplifiedType::MarkerTraitObject, SimplifiedType::Trait);
+
+    let elaborated_supertraits =
+        principal_def_id.into_iter().flat_map(|def_id| tcx.supertrait_def_ids(def_id)).collect();
+
+    trait_has_impl_inner(
+        tcx,
+        target_trait_def_id,
+        target_self_ty,
+        &elaborated_supertraits,
+        &mut Default::default(),
+        true,
+    )
+}
+
+fn trait_has_impl_inner<'tcx>(
+    tcx: TyCtxt<'tcx>,
+    target_trait_def_id: DefId,
+    target_self_ty: SimplifiedType<DefId>,
+    elaborated_supertraits: &FxHashSet<DefId>,
+    seen_traits: &mut FxHashSet<DefId>,
+    first_generation: bool,
+) -> bool {
+    if tcx.is_lang_item(target_trait_def_id, LangItem::Sized) {
+        return false;
+    }
+
+    // If we've encountered a trait in a cycle, then let's just
+    // consider it to be implemented defensively.
+    if !seen_traits.insert(target_trait_def_id) {
+        return true;
+    }
+    // Since we don't pass in the set of auto traits, and just the principal,
+    // consider all auto traits implemented.
+    if tcx.trait_is_auto(target_trait_def_id) {
+        return true;
+    }
+    if !first_generation && elaborated_supertraits.contains(&target_trait_def_id) {
+        return true;
+    }
+
+    let mut has_offending_impl = false;
+    tcx.for_each_impl(target_trait_def_id, |impl_def_id| {
+        if has_offending_impl {
             return;
         }
 
@@ -504,33 +548,51 @@ pub(super) fn trait_has_impl_which_may_shadow_dyn<'tcx>(
             .expect("impl must have trait ref")
             .instantiate_identity()
             .self_ty();
-        if self_ty.is_known_rigid() {
-            return;
-        }
 
-        let sized_trait = tcx.require_lang_item(LangItem::Sized, None);
-        if tcx
-            .param_env(impl_def_id)
-            .caller_bounds()
-            .iter()
-            .filter_map(|clause| clause.as_trait_clause())
-            .any(|bound| bound.def_id() == sized_trait && bound.self_ty().skip_binder() == self_ty)
+        if simplify_type(tcx, self_ty, TreatParams::InstantiateWithInfer)
+            .is_some_and(|simp| simp != target_self_ty)
         {
             return;
         }
 
-        if let ty::Alias(ty::Projection, alias_ty) = self_ty.kind()
-            && tcx
-                .item_super_predicates(alias_ty.def_id)
-                .iter_identity()
-                .filter_map(|clause| clause.as_trait_clause())
-                .any(|bound| bound.def_id() == sized_trait)
+        for (pred, _) in
+            elaborate::elaborate(tcx, tcx.predicates_of(impl_def_id).instantiate_identity(tcx))
         {
-            return;
+            if let ty::ClauseKind::Trait(trait_pred) = pred.kind().skip_binder()
+                && trait_pred.self_ty() == self_ty
+                && !trait_has_impl_inner(
+                    tcx,
+                    trait_pred.def_id(),
+                    target_self_ty,
+                    elaborated_supertraits,
+                    seen_traits,
+                    false,
+                )
+            {
+                return;
+            }
+        }
+
+        if let ty::Alias(ty::Projection, alias_ty) = self_ty.kind() {
+            for pred in tcx.item_super_predicates(alias_ty.def_id).iter_identity() {
+                if let ty::ClauseKind::Trait(trait_pred) = pred.kind().skip_binder()
+                    && trait_pred.self_ty() == self_ty
+                    && !trait_has_impl_inner(
+                        tcx,
+                        trait_pred.def_id(),
+                        target_self_ty,
+                        elaborated_supertraits,
+                        seen_traits,
+                        false,
+                    )
+                {
+                    return;
+                }
+            }
         }
 
-        has_impl = true;
+        has_offending_impl = true;
     });
 
-    has_impl
+    has_offending_impl
 }
diff --git a/compiler/rustc_type_ir/src/interner.rs b/compiler/rustc_type_ir/src/interner.rs
@@ -274,7 +274,11 @@ pub trait Interner:
 
     fn trait_may_be_implemented_via_object(self, trait_def_id: Self::DefId) -> bool;
 
-    fn trait_has_impl_which_may_shadow_dyn(self, trait_def_id: Self::DefId) -> bool;
+    fn trait_has_impl_which_may_shadow_dyn(
+        self,
+        trait_def_id: Self::DefId,
+        principal_def_id: Option<Self::DefId>,
+    ) -> bool;
 
     fn is_impl_trait_in_trait(self, def_id: Self::DefId) -> bool;
 
diff --git a/tests/ui/coherence/indirect-impl-blanket-doesnt-overlap-object.rs b/tests/ui/coherence/indirect-impl-blanket-doesnt-overlap-object.rs
@@ -0,0 +1,25 @@
+//@ check-pass
+
+// Make sure that if we don't disqualify a built-in object impl
+// due to a blanket with a trait bound that will never apply to
+// the object.
+
+pub trait SimpleService {
+    type Resp;
+}
+
+trait Service {
+    type Resp;
+}
+
+impl<S> Service for S where S: SimpleService + ?Sized {
+    type Resp = <S as SimpleService>::Resp;
+}
+
+fn implements_service(x: &(impl Service<Resp = ()> + ?Sized)) {}
+
+fn test(x: &dyn Service<Resp = ()>) {
+    implements_service(x);
+}
+
+fn main() {}
diff --git a/tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.rs b/tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.rs
@@ -0,0 +1,43 @@
+trait Trait {
+    type Assoc;
+    fn generate(&self) -> Self::Assoc;
+}
+
+trait Other {}
+
+impl<S> Trait for S where S: Other + ?Sized {
+    type Assoc = &'static str;
+    fn generate(&self) -> Self::Assoc { "hi" }
+}
+
+trait Downstream: Trait<Assoc = usize> {}
+impl<T> Other for T where T: ?Sized + Downstream + OnlyDyn {}
+
+trait OnlyDyn {}
+impl OnlyDyn for dyn Downstream {}
+
+struct Concrete;
+impl Trait for Concrete {
+    type Assoc = usize;
+    fn generate(&self) -> Self::Assoc { 42 }
+}
+impl Downstream for Concrete {}
+
+fn test<T: ?Sized + Other>(x: &T) {
+    let s: &str = x.generate();
+    println!("{s}");
+}
+
+fn impl_downstream<T: ?Sized + Downstream>(x: &T) {}
+
+fn main() {
+    let x: &dyn Downstream = &Concrete;
+
+    test(x); // This call used to segfault.
+    //~^ ERROR type mismatch resolving
+
+    // This no longer holds since `Downstream: Trait<Assoc = usize>`,
+    // but the `Trait<Assoc = &'static str>` blanket impl now shadows.
+    impl_downstream(x);
+    //~^ ERROR type mismatch resolving
+}
diff --git a/tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.stderr b/tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.stderr
@@ -0,0 +1,47 @@
+error[E0271]: type mismatch resolving `<dyn Downstream as Trait>::Assoc == usize`
+  --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:36:10
+   |
+LL |     test(x); // This call used to segfault.
+   |     ---- ^ type mismatch resolving `<dyn Downstream as Trait>::Assoc == usize`
+   |     |
+   |     required by a bound introduced by this call
+   |
+note: expected this to be `usize`
+  --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:9:18
+   |
+LL |     type Assoc = &'static str;
+   |                  ^^^^^^^^^^^^
+note: required for `dyn Downstream` to implement `Other`
+  --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:14:9
+   |
+LL | impl<T> Other for T where T: ?Sized + Downstream + OnlyDyn {}
+   |         ^^^^^     ^                   ---------- unsatisfied trait bound introduced here
+   = note: associated types for the current `impl` cannot be restricted in `where` clauses
+note: required by a bound in `test`
+  --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:26:21
+   |
+LL | fn test<T: ?Sized + Other>(x: &T) {
+   |                     ^^^^^ required by this bound in `test`
+
+error[E0271]: type mismatch resolving `<dyn Downstream as Trait>::Assoc == usize`
+  --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:41:21
+   |
+LL |     impl_downstream(x);
+   |     --------------- ^ type mismatch resolving `<dyn Downstream as Trait>::Assoc == usize`
+   |     |
+   |     required by a bound introduced by this call
+   |
+note: expected this to be `usize`
+  --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:9:18
+   |
+LL |     type Assoc = &'static str;
+   |                  ^^^^^^^^^^^^
+note: required by a bound in `impl_downstream`
+  --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:31:32
+   |
+LL | fn impl_downstream<T: ?Sized + Downstream>(x: &T) {}
+   |                                ^^^^^^^^^^ required by this bound in `impl_downstream`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0271`.
diff --git a/tests/ui/coherence/indirect-impl-blanket-downstream-trait.rs b/tests/ui/coherence/indirect-impl-blanket-downstream-trait.rs
diff --git a/tests/ui/coherence/indirect-impl-blanket-downstream-trait.stderr b/tests/ui/coherence/indirect-impl-blanket-downstream-trait.stderr
diff --git a/tests/ui/coherence/overlap-dyn-impl-in-codegen.rs b/tests/ui/coherence/overlap-dyn-impl-in-codegen.rs
diff --git a/tests/ui/coherence/overlap-dyn-impl-in-codegen.stderr b/tests/ui/coherence/overlap-dyn-impl-in-codegen.stderr

Original file line number	Diff line number	Diff line change
`@@ -319,6 +319,14 @@ impl Key for (DefId, SimplifiedType) {`
`319`	`319`	`}`
`320`	`320`	`}`
`321`	`321`
	`322`	`+impl Key for (DefId, Option<DefId>) {`
	`323`	`+ type Cache<V> = DefaultCache<Self, V>;`
	`324`	`+`
	`325`	`+ fn default_span(&self, tcx: TyCtxt<'_>) -> Span {`
	`326`	`+ self.0.default_span(tcx)`
	`327`	`+ }`
	`328`	`+}`
	`329`	`+`
`322`	`330`	`impl<'tcx> Key for GenericArgsRef<'tcx> {`
`323`	`331`	`type Cache<V> = DefaultCache<Self, V>;`
`324`	`332`
Original file line number	Diff line number	Diff line change
`@@ -1371,11 +1371,12 @@ rustc_queries! {`
`1371`	`1371`	desc { \|tcx\| "checking if trait `{}` is dyn-compatible", tcx.def_path_str(trait_id) }
`1372`	`1372`	`}`
`1373`	`1373`
`1374`		`- query trait_has_impl_which_may_shadow_dyn(trait_def_id: DefId) -> bool {`
	`1374`	`+ query trait_has_impl_which_may_shadow_dyn(key: (DefId, Option<DefId>)) -> bool {`
`1375`	`1375`	`desc {`
`1376`	`1376`	\|tcx\| "checking if trait `{}` has an impl which may overlap with \
`1377`		`- the built-in impl for trait objects",`
`1378`		`- tcx.def_path_str(trait_def_id)`
	`1377`	+ the built-in impl for `dyn {}`",
	`1378`	`+ tcx.def_path_str(key.0),`
	`1379`	`+ key.1.map_or(String::from("..."), \|def_id\| tcx.def_path_str(def_id)),`
`1379`	`1380`	`}`
`1380`	`1381`	`}`
`1381`	`1382`
Original file line number	Diff line number	Diff line change
`@@ -581,8 +581,12 @@ impl<'tcx> Interner for TyCtxt<'tcx> {`
`581`	`581`	`self.trait_def(trait_def_id).implement_via_object`
`582`	`582`	`}`
`583`	`583`
`584`		`- fn trait_has_impl_which_may_shadow_dyn(self, trait_def_id: DefId) -> bool {`
`585`		`- self.trait_has_impl_which_may_shadow_dyn(trait_def_id)`
	`584`	`+ fn trait_has_impl_which_may_shadow_dyn(`
	`585`	`+ self,`
	`586`	`+ trait_def_id: DefId,`
	`587`	`+ principal_def_id: Option<DefId>,`
	`588`	`+ ) -> bool {`
	`589`	`+ self.trait_has_impl_which_may_shadow_dyn((trait_def_id, principal_def_id))`
`586`	`590`	`}`
`587`	`591`
`588`	`592`	`fn is_impl_trait_in_trait(self, def_id: DefId) -> bool {`