about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMichael Goulet <michael@errs.io>2024-06-06 14:40:56 -0400
committerMichael Goulet <michael@errs.io>2024-07-15 14:17:32 -0400
commit172cf9bef3411d479a0be98827647150cce48afd (patch)
treece2f3c3ef9c9895a4b44a4fe066e80fada18e973
parent8edf9b808427dfbe4425b49d213351f907f58b40 (diff)
Fix unsoundness when associated types dont actually come from supertraits
-rw-r--r--compiler/rustc_trait_selection/src/traits/object_safety.rs264
-rw-r--r--tests/ui/object-safety/almost-supertrait-associated-type.rs60
-rw-r--r--tests/ui/object-safety/almost-supertrait-associated-type.stderr55
3 files changed, 269 insertions, 110 deletions
diff --git a/compiler/rustc_trait_selection/src/traits/object_safety.rs b/compiler/rustc_trait_selection/src/traits/object_safety.rs
index 29c18512e6d..ec19cf27668 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::{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::GenericArgs;
 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::{TypeVisitableExt, Upcast};
 use rustc_span::symbol::Symbol;
 use rustc_span::Span;
 use rustc_target::abi::Abi;
@@ -738,130 +737,175 @@ enum AllowSelfProjections {
     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<Self> // bad
+///     fn foo(&self) -> Self::Y // OK, desugars to next example
+///     fn foo(&self) -> <Self as Trait>::Y // OK
+///     fn foo(&self) -> Self::X // OK, desugars to next example
+///     fn foo(&self) -> <Self as SuperTrait>::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) -> <Self as SomeOtherTrait>::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<TyCtxt<'tcx>>>(
     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<Self> // bad
-    //     fn foo(&self) -> Self::Y // OK, desugars to next example
-    //     fn foo(&self) -> <Self as Trait>::Y // OK
-    //     fn foo(&self) -> Self::X // OK, desugars to next example
-    //     fn foo(&self) -> <Self as SuperTrait>::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) -> <Self as SomeOtherTrait>::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<Vec<DefId>>,
-        allow_self_projections: AllowSelfProjections,
-    }
+    value
+        .visit_with(&mut IllegalSelfTypeVisitor {
+            tcx,
+            trait_def_id,
+            supertraits: None,
+            allow_self_projections,
+        })
+        .is_break()
+}
 
-    impl<'tcx> TypeVisitor<TyCtxt<'tcx>> for IllegalSelfTypeVisitor<'tcx> {
-        type Result = ControlFlow<()>;
+struct IllegalSelfTypeVisitor<'tcx> {
+    tcx: TyCtxt<'tcx>,
+    trait_def_id: DefId,
+    supertraits: Option<Vec<ty::TraitRef<'tcx>>>,
+    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<TyCtxt<'tcx>> 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) => {
-                    match self.allow_self_projections {
-                        AllowSelfProjections::Yes => {
-                            // This is a projected type `<Foo as SomeTrait>::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());
-                            }
+            }
+            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 `<Foo as SomeTrait>::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),
                     }
+                    AllowSelfProjections::No => t.super_visit_with(self),
                 }
-                _ => t.super_visit_with(self),
             }
+            _ => 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,
-            allow_self_projections,
-        })
-        .is_break()
+struct EraseEscapingBoundRegions<'tcx> {
+    tcx: TyCtxt<'tcx>,
+    binder: ty::DebruijnIndex,
+}
+
+impl<'tcx> TypeFolder<TyCtxt<'tcx>> for EraseEscapingBoundRegions<'tcx> {
+    fn cx(&self) -> TyCtxt<'tcx> {
+        self.tcx
+    }
+
+    fn fold_binder<T>(&mut self, t: ty::Binder<'tcx, T>) -> ty::Binder<'tcx, T>
+    where
+        T: TypeFoldable<TyCtxt<'tcx>>,
+    {
+        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 00000000000..963cdff526e
--- /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, U>(t: T) -> U {
+    (&PhantomData::<T> as &dyn Foo<T, U>).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<Q> {
+    type Assoc;
+}
+
+trait Dyn {
+    type Out;
+}
+impl<T, U> Dyn for dyn Foo<T, U> + '_ {
+//~^ ERROR the trait `Foo` cannot be made into an object
+    type Out = U;
+}
+impl<S: Dyn<Out = U> + ?Sized, U> Super<NotActuallySuper> for S {
+    type Assoc = U;
+}
+
+trait Foo<T, U>: Super<ActuallySuper, Assoc = T>
+where
+    <Self as Mirror>::Assoc: Super<NotActuallySuper>
+{
+    fn transmute(&self, t: T) -> <Self as Super<NotActuallySuper>>::Assoc;
+}
+
+trait Mirror {
+    type Assoc: ?Sized;
+}
+impl<T: ?Sized> Mirror for T {
+    type Assoc = T;
+}
+
+impl<T, U> Foo<T, U> for PhantomData<T> {
+    fn transmute(&self, t: T) -> T {
+        t
+    }
+}
+impl<T> Super<ActuallySuper> for PhantomData<T> {
+    type Assoc = T;
+}
+impl<T> Super<NotActuallySuper> for PhantomData<T> {
+    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 00000000000..97a51c2f381
--- /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<T, U> Dyn for dyn Foo<T, U> + '_ {
+   |                    ^^^^^^^^^^^^^^^^^^ `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 <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
+  --> $DIR/almost-supertrait-associated-type.rs:33:34
+   |
+LL | trait Foo<T, U>: Super<ActuallySuper, Assoc = T>
+   |       --- this trait cannot be made into an object...
+...
+LL |     fn transmute(&self, t: T) -> <Self as Super<NotActuallySuper>>::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<T>` 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::<T> as &dyn Foo<T, U>).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 <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
+  --> $DIR/almost-supertrait-associated-type.rs:33:34
+   |
+LL | trait Foo<T, U>: Super<ActuallySuper, Assoc = T>
+   |       --- this trait cannot be made into an object...
+...
+LL |     fn transmute(&self, t: T) -> <Self as Super<NotActuallySuper>>::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<T>` 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::<T> as &dyn Foo<T, U>).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 <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
+  --> $DIR/almost-supertrait-associated-type.rs:33:34
+   |
+LL | trait Foo<T, U>: Super<ActuallySuper, Assoc = T>
+   |       --- this trait cannot be made into an object...
+...
+LL |     fn transmute(&self, t: T) -> <Self as Super<NotActuallySuper>>::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<T>` implements the trait, consider using it directly instead
+   = note: required for the cast from `&PhantomData<T>` to `&dyn Foo<T, U>`
+
+error: aborting due to 3 previous errors
+
+For more information about this error, try `rustc --explain E0038`.