Skip to content

Commit

Permalink
Make the implementation better
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Nov 21, 2024
1 parent 1820863 commit 264f242
Show file tree
Hide file tree
Showing 15 changed files with 325 additions and 35 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/query/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,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>;

Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1370,11 +1370,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)),
}
}

Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,8 +578,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 {
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,7 +842,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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
112 changes: 87 additions & 25 deletions compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,6 +25,8 @@ use rustc_middle::ty::{
};
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};

Expand Down Expand Up @@ -484,20 +486,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;
}

Expand All @@ -506,33 +550,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
}
6 changes: 5 additions & 1 deletion compiler/rustc_type_ir/src/interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,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;

Expand Down
25 changes: 25 additions & 0 deletions tests/ui/coherence/indirect-impl-blanket-doesnt-overlap-object.rs
Original file line number Diff line number Diff line change
@@ -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() {}
43 changes: 43 additions & 0 deletions tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.rs
Original file line number Diff line number Diff line change
@@ -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
}
47 changes: 47 additions & 0 deletions tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Loading

0 comments on commit 264f242

Please sign in to comment.