From b89751b8f42d9c627ea65a0c2ea2d6f89eb9b461 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Fri, 18 Oct 2024 16:06:57 +0200 Subject: [PATCH 1/2] clean up `*dyn` casts (with principals) - remove a redundant check, because we always emit the "better diagnostic" now - clean up the comments a bit --- compiler/rustc_hir_typeck/src/cast.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs index 8fa6ab8503d08..7341d3fa3c4e2 100644 --- a/compiler/rustc_hir_typeck/src/cast.rs +++ b/compiler/rustc_hir_typeck/src/cast.rs @@ -876,20 +876,14 @@ impl<'a, 'tcx> CastCheck<'tcx> { // A + SrcAuto> -> B + DstAuto>. need to make sure // - `Src` and `Dst` traits are the same // - traits have the same generic arguments - // - `SrcAuto` is a superset of `DstAuto` - (Some(src_principal), Some(dst_principal)) => { + // - projections are the same + // - `SrcAuto` (+auto traits implied by `Src`) is a superset of `DstAuto` + // + // Note that trait upcasting goes through a different mechanism (`coerce_unsized`) + // and is unaffected by this check. + (Some(src_principal), Some(_)) => { let tcx = fcx.tcx; - // Check that the traits are actually the same. - // The `dyn Src = dyn Dst` check below would suffice, - // but this may produce a better diagnostic. - // - // Note that trait upcasting goes through a different mechanism (`coerce_unsized`) - // and is unaffected by this check. - if src_principal.def_id() != dst_principal.def_id() { - return Err(CastError::DifferingKinds { src_kind, dst_kind }); - } - // We need to reconstruct trait object types. // `m_src` and `m_dst` won't work for us here because they will potentially // contain wrappers, which we do not care about. @@ -912,8 +906,8 @@ impl<'a, 'tcx> CastCheck<'tcx> { ty::Dyn, )); - // `dyn Src = dyn Dst`, this checks for matching traits/generics - // This is `demand_eqtype`, but inlined to give a better error. + // `dyn Src = dyn Dst`, this checks for matching traits/generics/projections + // This is `fcx.demand_eqtype`, but inlined to give a better error. let cause = fcx.misc(self.span); if fcx .at(&cause, fcx.param_env) From 46cc5e9e962fda2f99d4956ef539bf4b79fead26 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Fri, 18 Oct 2024 16:47:19 +0200 Subject: [PATCH 2/2] elaborate why dropping principal in `*dyn` casts is non-trivial --- compiler/rustc_hir_typeck/src/cast.rs | 31 +++++++++++++++++-- .../cast/ptr-to-trait-obj-drop-principal.rs | 21 +++++++++++++ .../ptr-to-trait-obj-drop-principal.stderr | 11 +++++++ 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 tests/ui/cast/ptr-to-trait-obj-drop-principal.rs create mode 100644 tests/ui/cast/ptr-to-trait-obj-drop-principal.stderr diff --git a/compiler/rustc_hir_typeck/src/cast.rs b/compiler/rustc_hir_typeck/src/cast.rs index 7341d3fa3c4e2..86af6c97af574 100644 --- a/compiler/rustc_hir_typeck/src/cast.rs +++ b/compiler/rustc_hir_typeck/src/cast.rs @@ -959,8 +959,35 @@ impl<'a, 'tcx> CastCheck<'tcx> { // dyn Auto -> dyn Auto'? ok. (None, None) => Ok(CastKind::PtrPtrCast), - // dyn Trait -> dyn Auto? should be ok, but we used to not allow it. - // FIXME: allow this + // dyn Trait -> dyn Auto? not ok (for now). + // + // Although dropping the principal is already allowed for unsizing coercions + // (e.g. `*const (dyn Trait + Auto)` to `*const dyn Auto`), dropping it is + // currently **NOT** allowed for (non-coercion) ptr-to-ptr casts (e.g + // `*const Foo` to `*const Bar` where `Foo` has a `dyn Trait + Auto` tail + // and `Bar` has a `dyn Auto` tail), because the underlying MIR operations + // currently work very differently: + // + // * A MIR unsizing coercion on raw pointers to trait objects (`*const dyn Src` + // to `*const dyn Dst`) is currently equivalent to downcasting the source to + // the concrete sized type that it was originally unsized from first (via a + // ptr-to-ptr cast from `*const Src` to `*const T` with `T: Sized`) and then + // unsizing this thin pointer to the target type (unsizing `*const T` to + // `*const Dst`). In particular, this means that the pointer's metadata + // (vtable) will semantically change, e.g. for const eval and miri, even + // though the vtables will always be merged for codegen. + // + // * A MIR ptr-to-ptr cast is currently equivalent to a transmute and does not + // change the pointer metadata (vtable) at all. + // + // In addition to this potentially surprising difference between coercion and + // non-coercion casts, casting away the principal with a MIR ptr-to-ptr cast + // is currently considered undefined behavior: + // + // As a validity invariant of pointers to trait objects, we currently require + // that the principal of the vtable in the pointer metadata exactly matches + // the principal of the pointee type, where "no principal" is also considered + // a kind of principal. (Some(_), None) => Err(CastError::DifferingKinds { src_kind, dst_kind }), // dyn Auto -> dyn Trait? not ok. diff --git a/tests/ui/cast/ptr-to-trait-obj-drop-principal.rs b/tests/ui/cast/ptr-to-trait-obj-drop-principal.rs new file mode 100644 index 0000000000000..01dd91fc77dd7 --- /dev/null +++ b/tests/ui/cast/ptr-to-trait-obj-drop-principal.rs @@ -0,0 +1,21 @@ +//! Test that non-coercion casts aren't allowed to drop the principal, +//! because they cannot modify the pointer metadata. +//! +//! We test this in a const context to guard against UB if this is allowed +//! in the future. + +trait Trait {} +impl Trait for () {} + +struct Wrapper(T); + +const OBJECT: *const (dyn Trait + Send) = &(); + +// coercions are allowed +const _: *const dyn Send = OBJECT as _; + +// casts are **not** allowed +const _: *const Wrapper = OBJECT as _; +//~^ ERROR casting `*const (dyn Trait + Send + 'static)` as `*const Wrapper` is invalid + +fn main() {} diff --git a/tests/ui/cast/ptr-to-trait-obj-drop-principal.stderr b/tests/ui/cast/ptr-to-trait-obj-drop-principal.stderr new file mode 100644 index 0000000000000..719e0711f5b9b --- /dev/null +++ b/tests/ui/cast/ptr-to-trait-obj-drop-principal.stderr @@ -0,0 +1,11 @@ +error[E0606]: casting `*const (dyn Trait + Send + 'static)` as `*const Wrapper` is invalid + --> $DIR/ptr-to-trait-obj-drop-principal.rs:18:37 + | +LL | const _: *const Wrapper = OBJECT as _; + | ^^^^^^^^^^^ + | + = note: the trait objects may have different vtables + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0606`.