Skip to content

Commit 172cf9b

Browse files
compiler-errorscompiler-errors
committed
Fix unsoundness when associated types dont actually come from supertraits
1 parent 8edf9b8 commit 172cf9b

File tree

3 files changed

+269
-110
lines changed

3 files changed

+269
-110
lines changed

‎compiler/rustc_trait_selection/src/traits/object_safety.rs

+154-110
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,16 @@ use super::elaborate;
1212

1313
use crate::infer::TyCtxtInferExt;
1414
use crate::traits::query::evaluate_obligation::InferCtxtExt;
15-
use crate::traits::{Obligation, ObligationCause};
15+
use crate::traits::{util, Obligation, ObligationCause};
1616
use rustc_errors::FatalError;
1717
use rustc_hir as hir;
1818
use rustc_hir::def_id::DefId;
1919
use rustc_middle::query::Providers;
20-
use rustc_middle::ty::GenericArgs;
2120
use rustc_middle::ty::{
22-
self, EarlyBinder, ExistentialPredicateStableCmpExt as _, Ty, TyCtxt, TypeSuperVisitable,
23-
TypeVisitable, TypeVisitor,
21+
self, EarlyBinder, ExistentialPredicateStableCmpExt as _, GenericArgs, Ty, TyCtxt,
22+
TypeFoldable, TypeFolder, TypeSuperFoldable, TypeSuperVisitable, TypeVisitable,
23+
TypeVisitableExt, TypeVisitor, Upcast,
2424
};
25-
use rustc_middle::ty::{TypeVisitableExt, Upcast};
2625
use rustc_span::symbol::Symbol;
2726
use rustc_span::Span;
2827
use rustc_target::abi::Abi;
@@ -738,130 +737,175 @@ enum AllowSelfProjections {
738737
No,
739738
}
740739

740+
/// This is somewhat subtle. In general, we want to forbid
741+
/// references to `Self` in the argument and return types,
742+
/// since the value of `Self` is erased. However, there is one
743+
/// exception: it is ok to reference `Self` in order to access
744+
/// an associated type of the current trait, since we retain
745+
/// the value of those associated types in the object type
746+
/// itself.
747+
///
748+
/// ```rust,ignore (example)
749+
/// trait SuperTrait {
750+
/// type X;
751+
/// }
752+
///
753+
/// trait Trait : SuperTrait {
754+
/// type Y;
755+
/// fn foo(&self, x: Self) // bad
756+
/// fn foo(&self) -> Self // bad
757+
/// fn foo(&self) -> Option<Self> // bad
758+
/// fn foo(&self) -> Self::Y // OK, desugars to next example
759+
/// fn foo(&self) -> <Self as Trait>::Y // OK
760+
/// fn foo(&self) -> Self::X // OK, desugars to next example
761+
/// fn foo(&self) -> <Self as SuperTrait>::X // OK
762+
/// }
763+
/// ```
764+
///
765+
/// However, it is not as simple as allowing `Self` in a projected
766+
/// type, because there are illegal ways to use `Self` as well:
767+
///
768+
/// ```rust,ignore (example)
769+
/// trait Trait : SuperTrait {
770+
/// ...
771+
/// fn foo(&self) -> <Self as SomeOtherTrait>::X;
772+
/// }
773+
/// ```
774+
///
775+
/// Here we will not have the type of `X` recorded in the
776+
/// object type, and we cannot resolve `Self as SomeOtherTrait`
777+
/// without knowing what `Self` is.
741778
fn contains_illegal_self_type_reference<'tcx, T: TypeVisitable<TyCtxt<'tcx>>>(
742779
tcx: TyCtxt<'tcx>,
743780
trait_def_id: DefId,
744781
value: T,
745782
allow_self_projections: AllowSelfProjections,
746783
) -> bool {
747-
// This is somewhat subtle. In general, we want to forbid
748-
// references to `Self` in the argument and return types,
749-
// since the value of `Self` is erased. However, there is one
750-
// exception: it is ok to reference `Self` in order to access
751-
// an associated type of the current trait, since we retain
752-
// the value of those associated types in the object type
753-
// itself.
754-
//
755-
// ```rust
756-
// trait SuperTrait {
757-
// type X;
758-
// }
759-
//
760-
// trait Trait : SuperTrait {
761-
// type Y;
762-
// fn foo(&self, x: Self) // bad
763-
// fn foo(&self) -> Self // bad
764-
// fn foo(&self) -> Option<Self> // bad
765-
// fn foo(&self) -> Self::Y // OK, desugars to next example
766-
// fn foo(&self) -> <Self as Trait>::Y // OK
767-
// fn foo(&self) -> Self::X // OK, desugars to next example
768-
// fn foo(&self) -> <Self as SuperTrait>::X // OK
769-
// }
770-
// ```
771-
//
772-
// However, it is not as simple as allowing `Self` in a projected
773-
// type, because there are illegal ways to use `Self` as well:
774-
//
775-
// ```rust
776-
// trait Trait : SuperTrait {
777-
// ...
778-
// fn foo(&self) -> <Self as SomeOtherTrait>::X;
779-
// }
780-
// ```
781-
//
782-
// Here we will not have the type of `X` recorded in the
783-
// object type, and we cannot resolve `Self as SomeOtherTrait`
784-
// without knowing what `Self` is.
785-
786-
struct IllegalSelfTypeVisitor<'tcx> {
787-
tcx: TyCtxt<'tcx>,
788-
trait_def_id: DefId,
789-
supertraits: Option<Vec<DefId>>,
790-
allow_self_projections: AllowSelfProjections,
791-
}
784+
value
785+
.visit_with(&mut IllegalSelfTypeVisitor {
786+
tcx,
787+
trait_def_id,
788+
supertraits: None,
789+
allow_self_projections,
790+
})
791+
.is_break()
792+
}
792793

793-
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for IllegalSelfTypeVisitor<'tcx> {
794-
type Result = ControlFlow<()>;
794+
struct IllegalSelfTypeVisitor<'tcx> {
795+
tcx: TyCtxt<'tcx>,
796+
trait_def_id: DefId,
797+
supertraits: Option<Vec<ty::TraitRef<'tcx>>>,
798+
allow_self_projections: AllowSelfProjections,
799+
}
795800

796-
fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
797-
match t.kind() {
798-
ty::Param(_) => {
799-
if t == self.tcx.types.self_param {
800-
ControlFlow::Break(())
801-
} else {
802-
ControlFlow::Continue(())
803-
}
804-
}
805-
ty::Alias(ty::Projection, ref data)
806-
if self.tcx.is_impl_trait_in_trait(data.def_id) =>
807-
{
808-
// We'll deny these later in their own pass
801+
impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for IllegalSelfTypeVisitor<'tcx> {
802+
type Result = ControlFlow<()>;
803+
804+
fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result {
805+
match t.kind() {
806+
ty::Param(_) => {
807+
if t == self.tcx.types.self_param {
808+
ControlFlow::Break(())
809+
} else {
809810
ControlFlow::Continue(())
810811
}
811-
ty::Alias(ty::Projection, ref data) => {
812-
match self.allow_self_projections {
813-
AllowSelfProjections::Yes => {
814-
// This is a projected type `<Foo as SomeTrait>::X`.
815-
816-
// Compute supertraits of current trait lazily.
817-
if self.supertraits.is_none() {
818-
self.supertraits =
819-
Some(self.tcx.supertrait_def_ids(self.trait_def_id).collect());
820-
}
812+
}
813+
ty::Alias(ty::Projection, ref data) if self.tcx.is_impl_trait_in_trait(data.def_id) => {
814+
// We'll deny these later in their own pass
815+
ControlFlow::Continue(())
816+
}
817+
ty::Alias(ty::Projection, ref data) => {
818+
match self.allow_self_projections {
819+
AllowSelfProjections::Yes => {
820+
// This is a projected type `<Foo as SomeTrait>::X`.
821+
822+
// Compute supertraits of current trait lazily.
823+
if self.supertraits.is_none() {
824+
self.supertraits = Some(
825+
util::supertraits(
826+
self.tcx,
827+
ty::Binder::dummy(ty::TraitRef::identity(
828+
self.tcx,
829+
self.trait_def_id,
830+
)),
831+
)
832+
.map(|trait_ref| {
833+
self.tcx.erase_regions(
834+
self.tcx.instantiate_bound_regions_with_erased(trait_ref),
835+
)
836+
})
837+
.collect(),
838+
);
839+
}
821840

822-
// Determine whether the trait reference `Foo as
823-
// SomeTrait` is in fact a supertrait of the
824-
// current trait. In that case, this type is
825-
// legal, because the type `X` will be specified
826-
// in the object type. Note that we can just use
827-
// direct equality here because all of these types
828-
// are part of the formal parameter listing, and
829-
// hence there should be no inference variables.
830-
let is_supertrait_of_current_trait = self
831-
.supertraits
832-
.as_ref()
833-
.unwrap()
834-
.contains(&data.trait_ref(self.tcx).def_id);
835-
836-
// only walk contained types if it's not a super trait
837-
if is_supertrait_of_current_trait {
838-
ControlFlow::Continue(())
839-
} else {
840-
t.super_visit_with(self) // POSSIBLY reporting an error
841-
}
841+
// Determine whether the trait reference `Foo as
842+
// SomeTrait` is in fact a supertrait of the
843+
// current trait. In that case, this type is
844+
// legal, because the type `X` will be specified
845+
// in the object type. Note that we can just use
846+
// direct equality here because all of these types
847+
// are part of the formal parameter listing, and
848+
// hence there should be no inference variables.
849+
let is_supertrait_of_current_trait =
850+
self.supertraits.as_ref().unwrap().contains(
851+
&data.trait_ref(self.tcx).fold_with(
852+
&mut EraseEscapingBoundRegions {
853+
tcx: self.tcx,
854+
binder: ty::INNERMOST,
855+
},
856+
),
857+
);
858+
859+
// only walk contained types if it's not a super trait
860+
if is_supertrait_of_current_trait {
861+
ControlFlow::Continue(())
862+
} else {
863+
t.super_visit_with(self) // POSSIBLY reporting an error
842864
}
843-
AllowSelfProjections::No => t.super_visit_with(self),
844865
}
866+
AllowSelfProjections::No => t.super_visit_with(self),
845867
}
846-
_ => t.super_visit_with(self),
847868
}
869+
_ => t.super_visit_with(self),
848870
}
871+
}
849872

850-
fn visit_const(&mut self, ct: ty::Const<'tcx>) -> Self::Result {
851-
// Constants can only influence object safety if they are generic and reference `Self`.
852-
// This is only possible for unevaluated constants, so we walk these here.
853-
self.tcx.expand_abstract_consts(ct).super_visit_with(self)
854-
}
873+
fn visit_const(&mut self, ct: ty::Const<'tcx>) -> Self::Result {
874+
// Constants can only influence object safety if they are generic and reference `Self`.
875+
// This is only possible for unevaluated constants, so we walk these here.
876+
self.tcx.expand_abstract_consts(ct).super_visit_with(self)
855877
}
878+
}
856879

857-
value
858-
.visit_with(&mut IllegalSelfTypeVisitor {
859-
tcx,
860-
trait_def_id,
861-
supertraits: None,
862-
allow_self_projections,
863-
})
864-
.is_break()
880+
struct EraseEscapingBoundRegions<'tcx> {
881+
tcx: TyCtxt<'tcx>,
882+
binder: ty::DebruijnIndex,
883+
}
884+
885+
impl<'tcx> TypeFolder<TyCtxt<'tcx>> for EraseEscapingBoundRegions<'tcx> {
886+
fn cx(&self) -> TyCtxt<'tcx> {
887+
self.tcx
888+
}
889+
890+
fn fold_binder<T>(&mut self, t: ty::Binder<'tcx, T>) -> ty::Binder<'tcx, T>
891+
where
892+
T: TypeFoldable<TyCtxt<'tcx>>,
893+
{
894+
self.binder.shift_in(1);
895+
let result = t.super_fold_with(self);
896+
self.binder.shift_out(1);
897+
result
898+
}
899+
900+
fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
901+
if let ty::ReBound(debruijn, _) = *r
902+
&& debruijn < self.binder
903+
{
904+
r
905+
} else {
906+
self.tcx.lifetimes.re_erased
907+
}
908+
}
865909
}
866910

867911
pub fn contains_illegal_impl_trait_in_trait<'tcx>(

‎tests/ui/object-safety/almost-supertrait-associated-type.rs

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Test for fixed unsoundness in #126079.
2+
// Enforces that the associated types that are object safe
3+
4+
use std::marker::PhantomData;
5+
6+
fn transmute<T, U>(t: T) -> U {
7+
(&PhantomData::<T> as &dyn Foo<T, U>).transmute(t)
8+
//~^ ERROR the trait `Foo` cannot be made into an object
9+
//~| ERROR the trait `Foo` cannot be made into an object
10+
}
11+
12+
struct ActuallySuper;
13+
struct NotActuallySuper;
14+
trait Super<Q> {
15+
type Assoc;
16+
}
17+
18+
trait Dyn {
19+
type Out;
20+
}
21+
impl<T, U> Dyn for dyn Foo<T, U> + '_ {
22+
//~^ ERROR the trait `Foo` cannot be made into an object
23+
type Out = U;
24+
}
25+
impl<S: Dyn<Out = U> + ?Sized, U> Super<NotActuallySuper> for S {
26+
type Assoc = U;
27+
}
28+
29+
trait Foo<T, U>: Super<ActuallySuper, Assoc = T>
30+
where
31+
<Self as Mirror>::Assoc: Super<NotActuallySuper>
32+
{
33+
fn transmute(&self, t: T) -> <Self as Super<NotActuallySuper>>::Assoc;
34+
}
35+
36+
trait Mirror {
37+
type Assoc: ?Sized;
38+
}
39+
impl<T: ?Sized> Mirror for T {
40+
type Assoc = T;
41+
}
42+
43+
impl<T, U> Foo<T, U> for PhantomData<T> {
44+
fn transmute(&self, t: T) -> T {
45+
t
46+
}
47+
}
48+
impl<T> Super<ActuallySuper> for PhantomData<T> {
49+
type Assoc = T;
50+
}
51+
impl<T> Super<NotActuallySuper> for PhantomData<T> {
52+
type Assoc = T;
53+
}
54+
55+
fn main() {
56+
let x = String::from("hello, world");
57+
let s = transmute::<&str, &'static str>(x.as_str());
58+
drop(x);
59+
println!("> {s}");
60+
}

0 commit comments

Comments
 (0)