Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

minor *dyn cast cleanup #131898

Merged
merged 2 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 37 additions & 16 deletions compiler/rustc_hir_typeck/src/cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,20 +876,14 @@ impl<'a, 'tcx> CastCheck<'tcx> {
// A<dyn Src<...> + SrcAuto> -> B<dyn Dst<...> + 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.
Expand All @@ -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)
Expand Down Expand Up @@ -965,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.
Expand Down
21 changes: 21 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-drop-principal.rs
Original file line number Diff line number Diff line change
@@ -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: ?Sized>(T);

const OBJECT: *const (dyn Trait + Send) = &();

// coercions are allowed
const _: *const dyn Send = OBJECT as _;

// casts are **not** allowed
const _: *const Wrapper<dyn Send> = OBJECT as _;
//~^ ERROR casting `*const (dyn Trait + Send + 'static)` as `*const Wrapper<dyn Send>` is invalid

fn main() {}
11 changes: 11 additions & 0 deletions tests/ui/cast/ptr-to-trait-obj-drop-principal.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0606]: casting `*const (dyn Trait + Send + 'static)` as `*const Wrapper<dyn Send>` is invalid
--> $DIR/ptr-to-trait-obj-drop-principal.rs:18:37
|
LL | const _: *const Wrapper<dyn Send> = 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`.
Loading