From bb80bba427964866a7c46fc7b9bb2b7954e3de26 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 21 Nov 2024 03:48:07 +0000 Subject: [PATCH] Make the implementation better --- compiler/rustc_middle/src/query/keys.rs | 8 ++ compiler/rustc_middle/src/query/mod.rs | 7 +- compiler/rustc_middle/src/ty/context.rs | 8 +- .../src/solve/assembly/mod.rs | 9 +- .../src/traits/project.rs | 7 +- .../src/traits/select/candidate_assembly.rs | 5 +- .../src/traits/specialize/mod.rs | 112 ++++++++++++++---- compiler/rustc_type_ir/src/interner.rs | 6 +- ...rect-impl-blanket-doesnt-overlap-object.rs | 25 ++++ ...ndirect-impl-blanket-downstream-trait-2.rs | 43 +++++++ ...ect-impl-blanket-downstream-trait-2.stderr | 47 ++++++++ .../indirect-impl-blanket-downstream-trait.rs | 39 ++++++ ...irect-impl-blanket-downstream-trait.stderr | 22 ++++ .../coherence/overlap-dyn-impl-in-codegen.rs} | 3 +- .../overlap-dyn-impl-in-codegen.stderr | 19 +++ 15 files changed, 325 insertions(+), 35 deletions(-) create mode 100644 tests/ui/coherence/indirect-impl-blanket-doesnt-overlap-object.rs create mode 100644 tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.rs create mode 100644 tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.stderr create mode 100644 tests/ui/coherence/indirect-impl-blanket-downstream-trait.rs create mode 100644 tests/ui/coherence/indirect-impl-blanket-downstream-trait.stderr rename tests/{crashes/115435.rs => ui/coherence/overlap-dyn-impl-in-codegen.rs} (88%) create mode 100644 tests/ui/coherence/overlap-dyn-impl-in-codegen.stderr diff --git a/compiler/rustc_middle/src/query/keys.rs b/compiler/rustc_middle/src/query/keys.rs index ba7b57c891c5..1b5c4efd59d9 100644 --- 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) { + type Cache = DefaultCache; + + fn default_span(&self, tcx: TyCtxt<'_>) -> Span { + self.0.default_span(tcx) + } +} + impl<'tcx> Key for GenericArgsRef<'tcx> { type Cache = DefaultCache; diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index ac0a3ed63828..42d6b6cb5c1c 100644 --- 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)) -> 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 index 9d04f014e9f6..7eac9fb286de 100644 --- 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, + ) -> 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 index 3abc92e68fd5..6619a38a0f21 100644 --- 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, assumption: I::Clause, ) -> Result, 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 index b1955068cd82..d3da93c8845b 100644 --- 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 index 6c39b52f3832..1ef15a210291 100644 --- 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 index 44173493d090..826f9f22f55b 100644 --- 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), ) -> 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, + elaborated_supertraits: &FxHashSet, + seen_traits: &mut FxHashSet, + 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 index d00d68f07410..66751529aba8 100644 --- 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, + ) -> 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 new file mode 100644 index 000000000000..f4afb9bec4c3 --- /dev/null +++ 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 Service for S where S: SimpleService + ?Sized { + type Resp = ::Resp; +} + +fn implements_service(x: &(impl Service + ?Sized)) {} + +fn test(x: &dyn Service) { + 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 new file mode 100644 index 000000000000..a4c36d627e37 --- /dev/null +++ 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 Trait for S where S: Other + ?Sized { + type Assoc = &'static str; + fn generate(&self) -> Self::Assoc { "hi" } +} + +trait Downstream: Trait {} +impl 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(x: &T) { + let s: &str = x.generate(); + println!("{s}"); +} + +fn impl_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`, + // but the `Trait` 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 new file mode 100644 index 000000000000..95e7c1e37bab --- /dev/null +++ b/tests/ui/coherence/indirect-impl-blanket-downstream-trait-2.stderr @@ -0,0 +1,47 @@ +error[E0271]: type mismatch resolving `::Assoc == usize` + --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:36:10 + | +LL | test(x); // This call used to segfault. + | ---- ^ type mismatch resolving `::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 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(x: &T) { + | ^^^^^ required by this bound in `test` + +error[E0271]: type mismatch resolving `::Assoc == usize` + --> $DIR/indirect-impl-blanket-downstream-trait-2.rs:41:21 + | +LL | impl_downstream(x); + | --------------- ^ type mismatch resolving `::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(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 new file mode 100644 index 000000000000..bfc5bd553857 --- /dev/null +++ b/tests/ui/coherence/indirect-impl-blanket-downstream-trait.rs @@ -0,0 +1,39 @@ +trait Trait { + type Assoc; + fn generate(&self) -> Self::Assoc; +} + +trait Other {} + +impl Trait for S where S: Other + ?Sized { + type Assoc = &'static str; + fn generate(&self) -> Self::Assoc { "hi" } +} + +trait Downstream: Trait {} +impl Other for dyn Downstream {} + +struct Concrete; +impl Trait for Concrete { + type Assoc = usize; + fn generate(&self) -> Self::Assoc { 42 } +} +impl Downstream for Concrete {} + +fn test(x: &T) { + let s: &str = x.generate(); + println!("{s}"); +} + +fn impl_downstream(x: &T) {} + +fn main() { + let x: &dyn Downstream = &Concrete; + + test(x); // This call used to segfault. + + // This no longer holds since `Downstream: Trait`, + // but the `Trait` blanket impl now shadows. + impl_downstream(x); + //~^ ERROR type mismatch resolving +} diff --git a/tests/ui/coherence/indirect-impl-blanket-downstream-trait.stderr b/tests/ui/coherence/indirect-impl-blanket-downstream-trait.stderr new file mode 100644 index 000000000000..90f6a5040557 --- /dev/null +++ b/tests/ui/coherence/indirect-impl-blanket-downstream-trait.stderr @@ -0,0 +1,22 @@ +error[E0271]: type mismatch resolving `::Assoc == usize` + --> $DIR/indirect-impl-blanket-downstream-trait.rs:37:21 + | +LL | impl_downstream(x); + | --------------- ^ type mismatch resolving `::Assoc == usize` + | | + | required by a bound introduced by this call + | +note: expected this to be `usize` + --> $DIR/indirect-impl-blanket-downstream-trait.rs:9:18 + | +LL | type Assoc = &'static str; + | ^^^^^^^^^^^^ +note: required by a bound in `impl_downstream` + --> $DIR/indirect-impl-blanket-downstream-trait.rs:28:32 + | +LL | fn impl_downstream(x: &T) {} + | ^^^^^^^^^^ required by this bound in `impl_downstream` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0271`. diff --git a/tests/crashes/115435.rs b/tests/ui/coherence/overlap-dyn-impl-in-codegen.rs similarity index 88% rename from tests/crashes/115435.rs rename to tests/ui/coherence/overlap-dyn-impl-in-codegen.rs index c6e749867e88..a0ab970f7e35 100644 --- a/tests/crashes/115435.rs +++ b/tests/ui/coherence/overlap-dyn-impl-in-codegen.rs @@ -1,6 +1,6 @@ -//@ known-bug: #115435 //@ edition:2021 //@ compile-flags: -Copt-level=0 + trait MyTrait { type Target: ?Sized; } @@ -11,6 +11,7 @@ impl MyTrait for A { fn main() { bug_run::>(); + //~^ ERROR the size for values of type } fn bug_run() diff --git a/tests/ui/coherence/overlap-dyn-impl-in-codegen.stderr b/tests/ui/coherence/overlap-dyn-impl-in-codegen.stderr new file mode 100644 index 000000000000..87548ef2d1c1 --- /dev/null +++ b/tests/ui/coherence/overlap-dyn-impl-in-codegen.stderr @@ -0,0 +1,19 @@ +error[E0277]: the size for values of type `dyn MyTrait` cannot be known at compilation time + --> $DIR/overlap-dyn-impl-in-codegen.rs:13:15 + | +LL | bug_run::>(); + | ^^^^^^^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time + | + = help: the trait `Sized` is not implemented for `dyn MyTrait` +note: required by a bound in `bug_run` + --> $DIR/overlap-dyn-impl-in-codegen.rs:19:29 + | +LL | fn bug_run() + | ------- required by a bound in this function +LL | where +LL | ::Target: Sized, + | ^^^^^ required by this bound in `bug_run` + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`.