diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs index 1c6993bdd3729..ec19cf2766816 100644 --- a/compiler/rustc_trait_selection/src/traits/object_safety.rs +++ b/compiler/rustc_trait_selection/src/traits/object_safety.rs @@ -12,17 +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::{util, 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::{ - self, EarlyBinder, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeSuperVisitable, - TypeVisitable, TypeVisitor, + self, EarlyBinder, ExistentialPredicateStableCmpExt as _, GenericArgs, Ty, TyCtxt, + TypeFoldable, TypeFolder, TypeSuperFoldable, TypeSuperVisitable, TypeVisitable, + TypeVisitableExt, TypeVisitor, Upcast, }; -use rustc_middle::ty::{GenericArg, GenericArgs}; -use rustc_middle::ty::{TypeVisitableExt, Upcast}; use rustc_span::symbol::Symbol; use rustc_span::Span; use rustc_target::abi::Abi; @@ -195,7 +194,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 +209,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).iter_identity_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 +245,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 +393,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 +411,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 +502,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,121 +731,181 @@ fn receiver_is_dispatchable<'tcx>( infcx.predicate_must_hold_modulo_regions(&obligation) } +#[derive(Copy, Clone)] +enum AllowSelfProjections { + Yes, + No, +} + +/// This is somewhat subtle. In general, we want to forbid +/// references to `Self` in the argument and return types, +/// since the value of `Self` is erased. However, there is one +/// exception: it is ok to reference `Self` in order to access +/// an associated type of the current trait, since we retain +/// the value of those associated types in the object type +/// itself. +/// +/// ```rust,ignore (example) +/// trait SuperTrait { +/// type X; +/// } +/// +/// trait Trait : SuperTrait { +/// type Y; +/// fn foo(&self, x: Self) // bad +/// fn foo(&self) -> Self // bad +/// fn foo(&self) -> Option // bad +/// fn foo(&self) -> Self::Y // OK, desugars to next example +/// fn foo(&self) -> ::Y // OK +/// fn foo(&self) -> Self::X // OK, desugars to next example +/// fn foo(&self) -> ::X // OK +/// } +/// ``` +/// +/// However, it is not as simple as allowing `Self` in a projected +/// type, because there are illegal ways to use `Self` as well: +/// +/// ```rust,ignore (example) +/// trait Trait : SuperTrait { +/// ... +/// fn foo(&self) -> ::X; +/// } +/// ``` +/// +/// Here we will not have the type of `X` recorded in the +/// object type, and we cannot resolve `Self as SomeOtherTrait` +/// without knowing what `Self` is. 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, - // since the value of `Self` is erased. However, there is one - // exception: it is ok to reference `Self` in order to access - // an associated type of the current trait, since we retain - // the value of those associated types in the object type - // itself. - // - // ```rust - // trait SuperTrait { - // type X; - // } - // - // trait Trait : SuperTrait { - // type Y; - // fn foo(&self, x: Self) // bad - // fn foo(&self) -> Self // bad - // fn foo(&self) -> Option // bad - // fn foo(&self) -> Self::Y // OK, desugars to next example - // fn foo(&self) -> ::Y // OK - // fn foo(&self) -> Self::X // OK, desugars to next example - // fn foo(&self) -> ::X // OK - // } - // ``` - // - // However, it is not as simple as allowing `Self` in a projected - // type, because there are illegal ways to use `Self` as well: - // - // ```rust - // trait Trait : SuperTrait { - // ... - // fn foo(&self) -> ::X; - // } - // ``` - // - // Here we will not have the type of `X` recorded in the - // object type, and we cannot resolve `Self as SomeOtherTrait` - // without knowing what `Self` is. - - struct IllegalSelfTypeVisitor<'tcx> { - tcx: TyCtxt<'tcx>, - trait_def_id: DefId, - supertraits: Option>, - } + value + .visit_with(&mut IllegalSelfTypeVisitor { + tcx, + trait_def_id, + supertraits: None, + allow_self_projections, + }) + .is_break() +} - impl<'tcx> TypeVisitor> for IllegalSelfTypeVisitor<'tcx> { - type Result = ControlFlow<()>; +struct IllegalSelfTypeVisitor<'tcx> { + tcx: TyCtxt<'tcx>, + trait_def_id: DefId, + supertraits: Option>>, + allow_self_projections: AllowSelfProjections, +} - fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { - match t.kind() { - ty::Param(_) => { - if t == self.tcx.types.self_param { - ControlFlow::Break(()) - } else { - ControlFlow::Continue(()) - } - } - ty::Alias(ty::Projection, ref data) - if self.tcx.is_impl_trait_in_trait(data.def_id) => - { - // We'll deny these later in their own pass +impl<'tcx> TypeVisitor> for IllegalSelfTypeVisitor<'tcx> { + type Result = ControlFlow<()>; + + fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { + match t.kind() { + ty::Param(_) => { + if t == self.tcx.types.self_param { + ControlFlow::Break(()) + } else { 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(), - ); - } + } + ty::Alias(ty::Projection, ref data) if self.tcx.is_impl_trait_in_trait(data.def_id) => { + // We'll deny these later in their own pass + ControlFlow::Continue(()) + } + ty::Alias(ty::Projection, ref data) => { + 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( + util::supertraits( + self.tcx, + ty::Binder::dummy(ty::TraitRef::identity( + self.tcx, + self.trait_def_id, + )), + ) + .map(|trait_ref| { + self.tcx.erase_regions( + self.tcx.instantiate_bound_regions_with_erased(trait_ref), + ) + }) + .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); - - // only walk contained types if it's not a super trait - if is_supertrait_of_current_trait { - ControlFlow::Continue(()) - } else { - t.super_visit_with(self) // 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).fold_with( + &mut EraseEscapingBoundRegions { + tcx: self.tcx, + binder: ty::INNERMOST, + }, + ), + ); + + // only walk contained types if it's not a super trait + if is_supertrait_of_current_trait { + ControlFlow::Continue(()) + } else { + t.super_visit_with(self) // POSSIBLY reporting an error + } } + AllowSelfProjections::No => t.super_visit_with(self), } - _ => t.super_visit_with(self), // walk contained types, if any } + _ => t.super_visit_with(self), } + } - fn visit_const(&mut self, ct: ty::Const<'tcx>) -> Self::Result { - // Constants can only influence object safety if they are generic and reference `Self`. - // This is only possible for unevaluated constants, so we walk these here. - self.tcx.expand_abstract_consts(ct).super_visit_with(self) - } + fn visit_const(&mut self, ct: ty::Const<'tcx>) -> Self::Result { + // Constants can only influence object safety if they are generic and reference `Self`. + // This is only possible for unevaluated constants, so we walk these here. + self.tcx.expand_abstract_consts(ct).super_visit_with(self) } +} - value - .visit_with(&mut IllegalSelfTypeVisitor { tcx, trait_def_id, supertraits: None }) - .is_break() +struct EraseEscapingBoundRegions<'tcx> { + tcx: TyCtxt<'tcx>, + binder: ty::DebruijnIndex, +} + +impl<'tcx> TypeFolder> for EraseEscapingBoundRegions<'tcx> { + fn cx(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn fold_binder(&mut self, t: ty::Binder<'tcx, T>) -> ty::Binder<'tcx, T> + where + T: TypeFoldable>, + { + self.binder.shift_in(1); + let result = t.super_fold_with(self); + self.binder.shift_out(1); + result + } + + fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> { + if let ty::ReBound(debruijn, _) = *r + && debruijn < self.binder + { + r + } else { + self.tcx.lifetimes.re_erased + } + } } pub fn contains_illegal_impl_trait_in_trait<'tcx>( diff --git a/tests/ui/object-safety/almost-supertrait-associated-type.rs b/tests/ui/object-safety/almost-supertrait-associated-type.rs new file mode 100644 index 0000000000000..963cdff526ee6 --- /dev/null +++ b/tests/ui/object-safety/almost-supertrait-associated-type.rs @@ -0,0 +1,60 @@ +// Test for fixed unsoundness in #126079. +// Enforces that the associated types that are object safe + +use std::marker::PhantomData; + +fn transmute(t: T) -> U { + (&PhantomData:: as &dyn Foo).transmute(t) + //~^ ERROR the trait `Foo` cannot be made into an object + //~| ERROR the trait `Foo` cannot be made into an object +} + +struct ActuallySuper; +struct NotActuallySuper; +trait Super { + type Assoc; +} + +trait Dyn { + type Out; +} +impl Dyn for dyn Foo + '_ { +//~^ ERROR the trait `Foo` cannot be made into an object + type Out = U; +} +impl + ?Sized, U> Super for S { + type Assoc = U; +} + +trait Foo: Super +where + ::Assoc: Super +{ + fn transmute(&self, t: T) -> >::Assoc; +} + +trait Mirror { + type Assoc: ?Sized; +} +impl Mirror for T { + type Assoc = T; +} + +impl Foo for PhantomData { + fn transmute(&self, t: T) -> T { + t + } +} +impl Super for PhantomData { + type Assoc = T; +} +impl Super for PhantomData { + type Assoc = T; +} + +fn main() { + let x = String::from("hello, world"); + let s = transmute::<&str, &'static str>(x.as_str()); + drop(x); + println!("> {s}"); +} diff --git a/tests/ui/object-safety/almost-supertrait-associated-type.stderr b/tests/ui/object-safety/almost-supertrait-associated-type.stderr new file mode 100644 index 0000000000000..97a51c2f38164 --- /dev/null +++ b/tests/ui/object-safety/almost-supertrait-associated-type.stderr @@ -0,0 +1,55 @@ +error[E0038]: the trait `Foo` cannot be made into an object + --> $DIR/almost-supertrait-associated-type.rs:21:20 + | +LL | impl Dyn for dyn Foo + '_ { + | ^^^^^^^^^^^^^^^^^^ `Foo` cannot be made into an object + | +note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit + --> $DIR/almost-supertrait-associated-type.rs:33:34 + | +LL | trait Foo: Super + | --- this trait cannot be made into an object... +... +LL | fn transmute(&self, t: T) -> >::Assoc; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...because method `transmute` references the `Self` type in its return type + = help: consider moving `transmute` to another trait + = help: only type `std::marker::PhantomData` implements the trait, consider using it directly instead + +error[E0038]: the trait `Foo` cannot be made into an object + --> $DIR/almost-supertrait-associated-type.rs:7:27 + | +LL | (&PhantomData:: as &dyn Foo).transmute(t) + | ^^^^^^^^^^^^^^ `Foo` cannot be made into an object + | +note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit + --> $DIR/almost-supertrait-associated-type.rs:33:34 + | +LL | trait Foo: Super + | --- this trait cannot be made into an object... +... +LL | fn transmute(&self, t: T) -> >::Assoc; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...because method `transmute` references the `Self` type in its return type + = help: consider moving `transmute` to another trait + = help: only type `std::marker::PhantomData` implements the trait, consider using it directly instead + +error[E0038]: the trait `Foo` cannot be made into an object + --> $DIR/almost-supertrait-associated-type.rs:7:6 + | +LL | (&PhantomData:: as &dyn Foo).transmute(t) + | ^^^^^^^^^^^^^^^^^ `Foo` cannot be made into an object + | +note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit + --> $DIR/almost-supertrait-associated-type.rs:33:34 + | +LL | trait Foo: Super + | --- this trait cannot be made into an object... +... +LL | fn transmute(&self, t: T) -> >::Assoc; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...because method `transmute` references the `Self` type in its return type + = help: consider moving `transmute` to another trait + = help: only type `std::marker::PhantomData` implements the trait, consider using it directly instead + = note: required for the cast from `&PhantomData` to `&dyn Foo` + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0038`. 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() {}