From b7b1b3efe05f1ffd356c14c4c5c530578e28e899 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 20 Mar 2024 20:30:04 -0400 Subject: [PATCH] Item bounds can reference self projections and still be object safe --- .../src/traits/object_safety.rs | 122 ++++++++++++------ .../item-bounds-can-reference-self.rs | 11 ++ 2 files changed, 90 insertions(+), 43 deletions(-) create mode 100644 tests/ui/object-safety/item-bounds-can-reference-self.rs diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index f1611bd049de9..79b0ac73a32e8 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -12,16 +12,16 @@ use super::elaborate; use crate::infer::TyCtxtInferExt; use crate::traits::query::evaluate_obligation::InferCtxtExt; -use crate::traits::{self, Obligation, ObligationCause}; +use crate::traits::{Obligation, ObligationCause}; use rustc_errors::FatalError; use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_middle::query::Providers; +use rustc_middle::ty::GenericArgs; use rustc_middle::ty::{ self, EarlyBinder, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitor, }; -use rustc_middle::ty::{GenericArg, GenericArgs}; use rustc_middle::ty::{TypeVisitableExt, Upcast}; use rustc_span::symbol::Symbol; use rustc_span::Span; @@ -195,7 +195,13 @@ fn predicates_reference_self( .predicates .iter() .map(|&(predicate, sp)| (predicate.instantiate_supertrait(tcx, trait_ref), sp)) - .filter_map(|predicate| predicate_references_self(tcx, predicate)) + .filter_map(|(clause, sp)| { + // Super predicates cannot allow self projections, since they're + // impossible to make into existential bounds without eager resolution + // or something. + // e.g. `trait A: B`. + predicate_references_self(tcx, trait_def_id, clause, sp, AllowSelfProjections::No) + }) .collect() } @@ -204,20 +210,25 @@ fn bounds_reference_self(tcx: TyCtxt<'_>, trait_def_id: DefId) -> SmallVec<[Span .in_definition_order() .filter(|item| item.kind == ty::AssocKind::Type) .flat_map(|item| tcx.explicit_item_bounds(item.def_id).instantiate_identity_iter_copied()) - .filter_map(|c| predicate_references_self(tcx, c)) + .filter_map(|(clause, sp)| { + // Item bounds *can* have self projections, since they never get + // their self type erased. + predicate_references_self(tcx, trait_def_id, clause, sp, AllowSelfProjections::Yes) + }) .collect() } fn predicate_references_self<'tcx>( tcx: TyCtxt<'tcx>, - (predicate, sp): (ty::Clause<'tcx>, Span), + trait_def_id: DefId, + predicate: ty::Clause<'tcx>, + sp: Span, + allow_self_projections: AllowSelfProjections, ) -> Option { - let self_ty = tcx.types.self_param; - let has_self_ty = |arg: &GenericArg<'tcx>| arg.walk().any(|arg| arg == self_ty.into()); match predicate.kind().skip_binder() { ty::ClauseKind::Trait(ref data) => { // In the case of a trait predicate, we can skip the "self" type. - data.trait_ref.args[1..].iter().any(has_self_ty).then_some(sp) + data.trait_ref.args[1..].iter().any(|&arg| contains_illegal_self_type_reference(tcx, trait_def_id, arg, allow_self_projections)).then_some(sp) } ty::ClauseKind::Projection(ref data) => { // And similarly for projections. This should be redundant with @@ -235,9 +246,9 @@ fn predicate_references_self<'tcx>( // // This is ALT2 in issue #56288, see that for discussion of the // possible alternatives. - data.projection_term.args[1..].iter().any(has_self_ty).then_some(sp) + data.projection_term.args[1..].iter().any(|&arg| contains_illegal_self_type_reference(tcx, trait_def_id, arg, allow_self_projections)).then_some(sp) } - ty::ClauseKind::ConstArgHasType(_ct, ty) => has_self_ty(&ty.into()).then_some(sp), + ty::ClauseKind::ConstArgHasType(_ct, ty) => contains_illegal_self_type_reference(tcx, trait_def_id, ty, allow_self_projections).then_some(sp), ty::ClauseKind::WellFormed(..) | ty::ClauseKind::TypeOutlives(..) @@ -383,7 +394,12 @@ fn virtual_call_violations_for_method<'tcx>( let mut errors = Vec::new(); for (i, &input_ty) in sig.skip_binder().inputs().iter().enumerate().skip(1) { - if contains_illegal_self_type_reference(tcx, trait_def_id, sig.rebind(input_ty)) { + if contains_illegal_self_type_reference( + tcx, + trait_def_id, + sig.rebind(input_ty), + AllowSelfProjections::Yes, + ) { let span = if let Some(hir::Node::TraitItem(hir::TraitItem { kind: hir::TraitItemKind::Fn(sig, _), .. @@ -396,7 +412,12 @@ fn virtual_call_violations_for_method<'tcx>( errors.push(MethodViolationCode::ReferencesSelfInput(span)); } } - if contains_illegal_self_type_reference(tcx, trait_def_id, sig.output()) { + if contains_illegal_self_type_reference( + tcx, + trait_def_id, + sig.output(), + AllowSelfProjections::Yes, + ) { errors.push(MethodViolationCode::ReferencesSelfOutput); } if let Some(code) = contains_illegal_impl_trait_in_trait(tcx, method.def_id, sig.output()) { @@ -482,7 +503,7 @@ fn virtual_call_violations_for_method<'tcx>( return false; } - contains_illegal_self_type_reference(tcx, trait_def_id, pred) + contains_illegal_self_type_reference(tcx, trait_def_id, pred, AllowSelfProjections::Yes) }) { errors.push(MethodViolationCode::WhereClauseReferencesSelf); } @@ -711,10 +732,17 @@ fn receiver_is_dispatchable<'tcx>( infcx.predicate_must_hold_modulo_regions(&obligation) } +#[derive(Copy, Clone)] +enum AllowSelfProjections { + Yes, + No, +} + fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable>>( tcx: TyCtxt<'tcx>, trait_def_id: DefId, value: T, + allow_self_projections: AllowSelfProjections, ) -> bool { // This is somewhat subtle. In general, we want to forbid // references to `Self` in the argument and return types, @@ -759,6 +787,7 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable>>( tcx: TyCtxt<'tcx>, trait_def_id: DefId, supertraits: Option>, + allow_self_projections: AllowSelfProjections, } impl<'tcx> TypeVisitor> for IllegalSelfTypeVisitor<'tcx> { @@ -780,38 +809,40 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable>>( ControlFlow::Continue(()) } ty::Alias(ty::Projection, ref data) => { - // This is a projected type `::X`. - - // Compute supertraits of current trait lazily. - if self.supertraits.is_none() { - let trait_ref = - ty::Binder::dummy(ty::TraitRef::identity(self.tcx, self.trait_def_id)); - self.supertraits = Some( - traits::supertraits(self.tcx, trait_ref).map(|t| t.def_id()).collect(), - ); - } + match self.allow_self_projections { + AllowSelfProjections::Yes => { + // This is a projected type `::X`. + + // Compute supertraits of current trait lazily. + if self.supertraits.is_none() { + self.supertraits = + Some(self.tcx.supertrait_def_ids(self.trait_def_id).collect()); + } - // Determine whether the trait reference `Foo as - // SomeTrait` is in fact a supertrait of the - // current trait. In that case, this type is - // legal, because the type `X` will be specified - // in the object type. Note that we can just use - // direct equality here because all of these types - // are part of the formal parameter listing, and - // hence there should be no inference variables. - let is_supertrait_of_current_trait = self - .supertraits - .as_ref() - .unwrap() - .contains(&data.trait_ref(self.tcx).def_id); - - if is_supertrait_of_current_trait { - ControlFlow::Continue(()) // do not walk contained types, do not report error, do collect $200 - } else { - t.super_visit_with(self) // DO walk contained types, POSSIBLY reporting an error + // Determine whether the trait reference `Foo as + // SomeTrait` is in fact a supertrait of the + // current trait. In that case, this type is + // legal, because the type `X` will be specified + // in the object type. Note that we can just use + // direct equality here because all of these types + // are part of the formal parameter listing, and + // hence there should be no inference variables. + let is_supertrait_of_current_trait = self + .supertraits + .as_ref() + .unwrap() + .contains(&data.trait_ref(self.tcx).def_id); + + if is_supertrait_of_current_trait { + ControlFlow::Continue(()) + } else { + t.super_visit_with(self) + } + } + AllowSelfProjections::No => t.super_visit_with(self), } } - _ => t.super_visit_with(self), // walk contained types, if any + _ => t.super_visit_with(self), } } @@ -823,7 +854,12 @@ fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable>>( } value - .visit_with(&mut IllegalSelfTypeVisitor { tcx, trait_def_id, supertraits: None }) + .visit_with(&mut IllegalSelfTypeVisitor { + tcx, + trait_def_id, + supertraits: None, + allow_self_projections, + }) .is_break() } diff --git a/tests/ui/object-safety/item-bounds-can-reference-self.rs b/tests/ui/object-safety/item-bounds-can-reference-self.rs new file mode 100644 index 0000000000000..4ae982e8f951f --- /dev/null +++ b/tests/ui/object-safety/item-bounds-can-reference-self.rs @@ -0,0 +1,11 @@ +//@ check-pass + +pub trait Foo { + type X: PartialEq; + type Y: PartialEq; + type Z: PartialEq; +} + +fn uwu(x: &dyn Foo) {} + +fn main() {}