Skip to content

Commit

Permalink
Auto merge of rust-lang#132289 - compiler-errors:vanquish-dyn-incompl…
Browse files Browse the repository at this point in the history
…eteness, r=<try>

Disqualify built-in trait impl if it seems likely to overlap in an unsound way with a blanket impl

cc rust-lang#57893

r? lcnr
  • Loading branch information
bors committed Nov 21, 2024
2 parents 2d0ea79 + 264f242 commit 4ce0d30
Show file tree
Hide file tree
Showing 22 changed files with 453 additions and 20 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
9 changes: 9 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1370,6 +1370,15 @@ rustc_queries! {
desc { |tcx| "checking if trait `{}` is dyn-compatible", tcx.def_path_str(trait_id) }
}

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 `dyn {}`",
tcx.def_path_str(key.0),
key.1.map_or(String::from("..."), |def_id| tcx.def_path_str(def_id)),
}
}

/// Gets the ParameterEnvironment for a given item; this environment
/// will be in "user-facing" mode, meaning that it is suitable for
/// type-checking etc, and it does not normalize specializable
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,14 @@ 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,
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 {
self.is_impl_trait_in_trait(def_id)
}
Expand Down
11 changes: 11 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 @@ -82,6 +82,17 @@ where
goal: Goal<I, Self>,
assumption: I::Clause,
) -> Result<Candidate<I>, NoSolution> {
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);
}

Self::probe_and_match_goal_against_assumption(ecx, source, goal, assumption, |ecx| {
let cx = ecx.cx();
let ty::Dynamic(bounds, _, _) = goal.predicate.self_ty().kind() else {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,7 @@ pub fn provide(providers: &mut Providers) {
specialization_graph_of: specialize::specialization_graph_provider,
specializes: specialize::specializes,
specialization_enabled_in: specialize::specialization_enabled_in,
trait_has_impl_which_may_shadow_dyn: specialize::trait_has_impl_which_may_shadow_dyn,
instantiate_and_check_impossible_predicates,
is_impossible_associated_item,
..*providers
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +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),
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 @@ -855,7 +855,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
"assemble_candidates_from_object_ty",
);

if !self.tcx().trait_def(obligation.predicate.def_id()).implement_via_object {
let tcx = self.tcx();
if !tcx.trait_def(obligation.predicate.def_id()).implement_via_object {
return;
}

Expand All @@ -876,9 +877,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

if let Some(principal) = data.principal() {
if !self.infcx.tcx.features().dyn_compatible_for_dispatch() {
principal.with_self_ty(self.tcx(), self_ty)
} else if self.tcx().is_dyn_compatible(principal.def_id()) {
principal.with_self_ty(self.tcx(), self_ty)
principal.with_self_ty(tcx, self_ty)
} else if tcx.is_dyn_compatible(principal.def_id()) {
principal.with_self_ty(tcx, self_ty)
} else {
return;
}
Expand All @@ -902,7 +903,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// correct trait, but also the correct type parameters.
// For example, we may be trying to upcast `Foo` to `Bar<i32>`,
// but `Foo` is declared as `trait Foo: Bar<u32>`.
let candidate_supertraits = util::supertraits(self.tcx(), principal_trait_ref)
let candidate_supertraits = util::supertraits(tcx, principal_trait_ref)
.enumerate()
.filter(|&(_, upcast_trait_ref)| {
self.infcx.probe(|_| {
Expand All @@ -914,6 +915,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
.is_ok()
})
})
.filter(|(_, trait_ref)| {
!tcx.trait_has_impl_which_may_shadow_dyn((
trait_ref.def_id(),
Some(principal_trait_ref.def_id()),
))
})
.map(|(idx, _)| ObjectCandidate(idx));

candidates.vec.extend(candidate_supertraits);
Expand Down
120 changes: 119 additions & 1 deletion compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,10 @@
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;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::infer::DefineOpaqueTypes;
use rustc_middle::bug;
Expand All @@ -24,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 @@ -480,3 +483,118 @@ fn report_conflicting_impls<'tcx>(
}
}
}

pub(super) fn trait_has_impl_which_may_shadow_dyn<'tcx>(
tcx: TyCtxt<'tcx>,
(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(target_trait_def_id)
.in_definition_order()
.any(|item| item.kind == ty::AssocKind::Type)
{
return false;
}

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;
}

let self_ty = tcx
.impl_trait_ref(impl_def_id)
.expect("impl must have trait ref")
.instantiate_identity()
.self_ty();

if simplify_type(tcx, self_ty, TreatParams::InstantiateWithInfer)
.is_some_and(|simp| simp != target_self_ty)
{
return;
}

for (pred, _) in
elaborate::elaborate(tcx, tcx.predicates_of(impl_def_id).instantiate_identity(tcx))
{
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_offending_impl = true;
});

has_offending_impl
}
6 changes: 6 additions & 0 deletions compiler/rustc_type_ir/src/interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,12 @@ 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,
principal_def_id: Option<Self::DefId>,
) -> bool;

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

fn delay_bug(self, msg: impl ToString) -> Self::ErrorGuaranteed;
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_type_ir/src/predicate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,10 @@ impl<I: Interner> ty::Binder<I, ExistentialProjection<I>> {
pub fn item_def_id(&self) -> I::DefId {
self.skip_binder().def_id
}

pub fn trait_def_id(self, interner: I) -> I::DefId {
interner.parent(self.skip_binder().def_id)
}
}

#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
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
}
Loading

0 comments on commit 4ce0d30

Please sign in to comment.