Skip to content

Commit 10da3d7

Browse files
Make the implementation better
1 parent 0d5a5e6 commit 10da3d7

File tree

15 files changed

+335
-35
lines changed

15 files changed

+335
-35
lines changed

compiler/rustc_middle/src/query/keys.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,14 @@ impl Key for (DefId, SimplifiedType) {
320320
}
321321
}
322322

323+
impl Key for (DefId, Option<DefId>) {
324+
type Cache<V> = DefaultCache<Self, V>;
325+
326+
fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
327+
self.0.default_span(tcx)
328+
}
329+
}
330+
323331
impl<'tcx> Key for GenericArgsRef<'tcx> {
324332
type Cache<V> = DefaultCache<Self, V>;
325333

compiler/rustc_middle/src/query/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,11 +1373,12 @@ rustc_queries! {
13731373
desc { |tcx| "checking if trait `{}` is dyn-compatible", tcx.def_path_str(trait_id) }
13741374
}
13751375

1376-
query trait_has_impl_which_may_shadow_dyn(trait_def_id: DefId) -> bool {
1376+
query trait_has_impl_which_may_shadow_dyn(key: (DefId, Option<DefId>)) -> bool {
13771377
desc {
13781378
|tcx| "checking if trait `{}` has an impl which may overlap with \
1379-
the built-in impl for trait objects",
1380-
tcx.def_path_str(trait_def_id)
1379+
the built-in impl for `dyn {}`",
1380+
tcx.def_path_str(key.0),
1381+
key.1.map_or(String::from("..."), |def_id| tcx.def_path_str(def_id)),
13811382
}
13821383
}
13831384

compiler/rustc_middle/src/ty/context.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -589,8 +589,12 @@ impl<'tcx> Interner for TyCtxt<'tcx> {
589589
self.trait_def(trait_def_id).safety == hir::Safety::Unsafe
590590
}
591591

592-
fn trait_has_impl_which_may_shadow_dyn(self, trait_def_id: DefId) -> bool {
593-
self.trait_has_impl_which_may_shadow_dyn(trait_def_id)
592+
fn trait_has_impl_which_may_shadow_dyn(
593+
self,
594+
trait_def_id: DefId,
595+
principal_def_id: Option<DefId>,
596+
) -> bool {
597+
self.trait_has_impl_which_may_shadow_dyn((trait_def_id, principal_def_id))
594598
}
595599

596600
fn is_impl_trait_in_trait(self, def_id: DefId) -> bool {

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,14 @@ where
8282
goal: Goal<I, Self>,
8383
assumption: I::Clause,
8484
) -> Result<Candidate<I>, NoSolution> {
85-
if ecx.cx().trait_has_impl_which_may_shadow_dyn(goal.predicate.trait_def_id(ecx.cx())) {
85+
let ty::Dynamic(data, _, _) = goal.predicate.self_ty().kind() else {
86+
unreachable!();
87+
};
88+
89+
if ecx.cx().trait_has_impl_which_may_shadow_dyn(
90+
goal.predicate.trait_def_id(ecx.cx()),
91+
data.principal_def_id(),
92+
) {
8693
return Err(NoSolution);
8794
}
8895

compiler/rustc_trait_selection/src/traits/project.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,12 @@ fn assemble_candidates_from_object_ty<'cx, 'tcx>(
817817
let env_predicates = data
818818
.projection_bounds()
819819
.filter(|bound| bound.item_def_id() == obligation.predicate.def_id)
820-
.filter(|bound| !tcx.trait_has_impl_which_may_shadow_dyn(bound.trait_def_id(tcx)))
820+
.filter(|bound| {
821+
!tcx.trait_has_impl_which_may_shadow_dyn((
822+
bound.trait_def_id(tcx),
823+
data.principal_def_id(),
824+
))
825+
})
821826
.map(|p| p.with_self_ty(tcx, object_ty).upcast(tcx));
822827

823828
assemble_candidates_from_predicates(

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
893893
})
894894
})
895895
.filter(|(_, trait_ref)| {
896-
!tcx.trait_has_impl_which_may_shadow_dyn(trait_ref.def_id())
896+
!tcx.trait_has_impl_which_may_shadow_dyn((
897+
trait_ref.def_id(),
898+
Some(principal_trait_ref.def_id()),
899+
))
897900
})
898901
.map(|(idx, _)| ObjectCandidate(idx));
899902

compiler/rustc_trait_selection/src/traits/specialize/mod.rs

Lines changed: 97 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
1212
pub mod specialization_graph;
1313

14-
use rustc_data_structures::fx::FxIndexSet;
14+
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
1515
use rustc_errors::codes::*;
1616
use rustc_errors::{Diag, EmissionGuarantee};
1717
use rustc_hir::LangItem;
@@ -23,6 +23,8 @@ use rustc_middle::ty::print::PrintTraitRefExt as _;
2323
use rustc_middle::ty::{self, GenericArgsRef, Ty, TyCtxt, TypeVisitableExt, TypingMode};
2424
use rustc_session::lint::builtin::{COHERENCE_LEAK_CHECK, ORDER_DEPENDENT_TRAIT_OBJECTS};
2525
use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span, sym};
26+
use rustc_type_ir::elaborate;
27+
use rustc_type_ir::fast_reject::{SimplifiedType, TreatParams, simplify_type};
2628
use rustc_type_ir::solve::NoSolution;
2729
use specialization_graph::GraphExt;
2830
use tracing::{debug, instrument};
@@ -599,20 +601,72 @@ fn report_conflicting_impls<'tcx>(
599601

600602
pub(super) fn trait_has_impl_which_may_shadow_dyn<'tcx>(
601603
tcx: TyCtxt<'tcx>,
602-
trait_def_id: DefId,
604+
(target_trait_def_id, principal_def_id): (DefId, Option<DefId>),
603605
) -> bool {
604606
// We only care about trait objects which have associated types.
605607
if !tcx
606-
.associated_items(trait_def_id)
608+
.associated_items(target_trait_def_id)
607609
.in_definition_order()
608610
.any(|item| item.kind == ty::AssocKind::Type)
609611
{
610612
return false;
611613
}
612614

613-
let mut has_impl = false;
614-
tcx.for_each_impl(trait_def_id, |impl_def_id| {
615-
if has_impl {
615+
let target_self_ty =
616+
principal_def_id.map_or(SimplifiedType::MarkerTraitObject, SimplifiedType::Trait);
617+
618+
let elaborated_supertraits =
619+
principal_def_id.into_iter().flat_map(|def_id| tcx.supertrait_def_ids(def_id)).collect();
620+
621+
trait_has_impl_inner(
622+
tcx,
623+
target_trait_def_id,
624+
target_self_ty,
625+
&elaborated_supertraits,
626+
&mut Default::default(),
627+
false,
628+
)
629+
}
630+
631+
fn trait_has_impl_inner<'tcx>(
632+
tcx: TyCtxt<'tcx>,
633+
target_trait_def_id: DefId,
634+
target_self_ty: SimplifiedType<DefId>,
635+
elaborated_supertraits: &FxHashSet<DefId>,
636+
seen_traits: &mut FxHashSet<DefId>,
637+
for_nested_impl: bool,
638+
) -> bool {
639+
if tcx.is_lang_item(target_trait_def_id, LangItem::Sized) {
640+
return false;
641+
}
642+
643+
// If we've encountered a trait in a cycle, then let's just
644+
// consider it to be implemented defensively.
645+
if !seen_traits.insert(target_trait_def_id) {
646+
return true;
647+
}
648+
// Since we don't pass in the set of auto traits, and just the principal,
649+
// consider all auto traits implemented.
650+
if tcx.trait_is_auto(target_trait_def_id) {
651+
return true;
652+
}
653+
// When checking the outermost call of this recursive function, we don't care
654+
// about elaborated supertraits or built-in impls. This is intuitively because
655+
// `dyn Trait: Trait` doesn't overlap with itself, and all built-in impls have
656+
// `rustc_deny_explicit_impl(implement_via_object = true)`.
657+
if for_nested_impl {
658+
if elaborated_supertraits.contains(&target_trait_def_id) {
659+
return true;
660+
}
661+
// Lazy heuristic that any lang item is potentially a built-in impl.
662+
if tcx.as_lang_item(target_trait_def_id).is_some() {
663+
return true;
664+
}
665+
}
666+
667+
let mut has_offending_impl = false;
668+
tcx.for_each_impl(target_trait_def_id, |impl_def_id| {
669+
if has_offending_impl {
616670
return;
617671
}
618672

@@ -621,33 +675,51 @@ pub(super) fn trait_has_impl_which_may_shadow_dyn<'tcx>(
621675
.expect("impl must have trait ref")
622676
.instantiate_identity()
623677
.self_ty();
624-
if self_ty.is_known_rigid() {
625-
return;
626-
}
627678

628-
let sized_trait = tcx.require_lang_item(LangItem::Sized, None);
629-
if tcx
630-
.param_env(impl_def_id)
631-
.caller_bounds()
632-
.iter()
633-
.filter_map(|clause| clause.as_trait_clause())
634-
.any(|bound| bound.def_id() == sized_trait && bound.self_ty().skip_binder() == self_ty)
679+
if simplify_type(tcx, self_ty, TreatParams::InstantiateWithInfer)
680+
.is_some_and(|simp| simp != target_self_ty)
635681
{
636682
return;
637683
}
638684

639-
if let ty::Alias(ty::Projection, alias_ty) = self_ty.kind()
640-
&& tcx
641-
.item_super_predicates(alias_ty.def_id)
642-
.iter_identity()
643-
.filter_map(|clause| clause.as_trait_clause())
644-
.any(|bound| bound.def_id() == sized_trait)
685+
for (pred, _) in
686+
elaborate::elaborate(tcx, tcx.predicates_of(impl_def_id).instantiate_identity(tcx))
645687
{
646-
return;
688+
if let ty::ClauseKind::Trait(trait_pred) = pred.kind().skip_binder()
689+
&& trait_pred.self_ty() == self_ty
690+
&& !trait_has_impl_inner(
691+
tcx,
692+
trait_pred.def_id(),
693+
target_self_ty,
694+
elaborated_supertraits,
695+
seen_traits,
696+
true,
697+
)
698+
{
699+
return;
700+
}
701+
}
702+
703+
if let ty::Alias(ty::Projection, alias_ty) = self_ty.kind() {
704+
for pred in tcx.item_super_predicates(alias_ty.def_id).iter_identity() {
705+
if let ty::ClauseKind::Trait(trait_pred) = pred.kind().skip_binder()
706+
&& trait_pred.self_ty() == self_ty
707+
&& !trait_has_impl_inner(
708+
tcx,
709+
trait_pred.def_id(),
710+
target_self_ty,
711+
elaborated_supertraits,
712+
seen_traits,
713+
false,
714+
)
715+
{
716+
return;
717+
}
718+
}
647719
}
648720

649-
has_impl = true;
721+
has_offending_impl = true;
650722
});
651723

652-
has_impl
724+
has_offending_impl
653725
}

compiler/rustc_type_ir/src/interner.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,11 @@ pub trait Interner:
273273
/// Returns `true` if this is an `unsafe trait`.
274274
fn trait_is_unsafe(self, trait_def_id: Self::DefId) -> bool;
275275

276-
fn trait_has_impl_which_may_shadow_dyn(self, trait_def_id: Self::DefId) -> bool;
276+
fn trait_has_impl_which_may_shadow_dyn(
277+
self,
278+
trait_def_id: Self::DefId,
279+
principal_def_id: Option<Self::DefId>,
280+
) -> bool;
277281

278282
fn is_impl_trait_in_trait(self, def_id: Self::DefId) -> bool;
279283

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//@ check-pass
2+
3+
// Make sure that if we don't disqualify a built-in object impl
4+
// due to a blanket with a trait bound that will never apply to
5+
// the object.
6+
7+
pub trait SimpleService {
8+
type Resp;
9+
}
10+
11+
trait Service {
12+
type Resp;
13+
}
14+
15+
impl<S> Service for S where S: SimpleService + ?Sized {
16+
type Resp = <S as SimpleService>::Resp;
17+
}
18+
19+
fn implements_service(x: &(impl Service<Resp = ()> + ?Sized)) {}
20+
21+
fn test(x: &dyn Service<Resp = ()>) {
22+
implements_service(x);
23+
}
24+
25+
fn main() {}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
trait Trait {
2+
type Assoc;
3+
fn generate(&self) -> Self::Assoc;
4+
}
5+
6+
trait Other {}
7+
8+
impl<S> Trait for S where S: Other + ?Sized {
9+
type Assoc = &'static str;
10+
fn generate(&self) -> Self::Assoc { "hi" }
11+
}
12+
13+
trait Downstream: Trait<Assoc = usize> {}
14+
impl<T> Other for T where T: ?Sized + Downstream + OnlyDyn {}
15+
16+
trait OnlyDyn {}
17+
impl OnlyDyn for dyn Downstream {}
18+
19+
struct Concrete;
20+
impl Trait for Concrete {
21+
type Assoc = usize;
22+
fn generate(&self) -> Self::Assoc { 42 }
23+
}
24+
impl Downstream for Concrete {}
25+
26+
fn test<T: ?Sized + Other>(x: &T) {
27+
let s: &str = x.generate();
28+
println!("{s}");
29+
}
30+
31+
fn impl_downstream<T: ?Sized + Downstream>(x: &T) {}
32+
33+
fn main() {
34+
let x: &dyn Downstream = &Concrete;
35+
36+
test(x); // This call used to segfault.
37+
//~^ ERROR type mismatch resolving
38+
39+
// This no longer holds since `Downstream: Trait<Assoc = usize>`,
40+
// but the `Trait<Assoc = &'static str>` blanket impl now shadows.
41+
impl_downstream(x);
42+
//~^ ERROR type mismatch resolving
43+
}

0 commit comments

Comments
 (0)