minor *dyn cast cleanup

[lukas-code]

Small follow-up to #130234 to remove a redundant check and clean up comments. No functional changes.

Also, explain why casts cannot drop the principal even though coercions can, and add a test because apparently we didn't have one already.

r? @WaffleLapkin or @compiler-errors

[compiler-errors]

@bors r+ rollup

[bors]

📌 Commit d9eb6d8 has been approved by compiler-errors

It is now in the queue for this repository.

[WaffleLapkin]

Why can they not change metadata?

And why would you need to change it anyway? Any vtable is valid for auto traits.

[compiler-errors]

=This may have just been an oversight of mine when authoring the dyn Trait -> dyn Auto PR.

[lukas-code]

Why can they not change metadata?

It's not that they inherently cannot, they just do not as they are currently implemented and would need to be changed for this to work.

And why would you need to change it anyway? Any vtable is valid for auto traits.

Because our UB checks currently require the vtable to match exactly. That's what the sentence above is trying to explain.

Currently, this is considered UB and Miri will report it as such:

#![feature(core_intrinsics, custom_mir)]

trait Trait {}
impl Trait for () {}

#[custom_mir(dialect = "built")]
fn cast(ptr: *const (dyn Trait + Send)) -> *const dyn Send {
    use std::intrinsics::mir::*;
    mir! {
        {
            RET = CastPtrToPtr(ptr);
            Return()
        }
    }
}

fn main() {
    let ptr: *const (dyn Trait + Send) = &();
    let ptr = cast(ptr);
}

error: Undefined Behavior: constructing invalid value: wrong trait in wide pointer vtable: expected `std::marker::Send`, but encountered `Trait + std::marker::Send`
  --> src/main.rs:11:13
   |
11 |             RET = CastPtrToPtr(ptr);
   |             ^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: wrong trait in wide pointer vtable: expected `std::marker::Send`, but encountered `Trait + std::marker::Send`
   |

So our options here are

• changing the opsem of a PtrToPtr cast in MIR
• lowering this HIR cast to something other than a single MIR cast
• declaring that the vtable must no longer match exactly (allowing dyn Trait vtables in *const dyn Auto) and adjusting the UB checks accordingly
  • this would also mean adjusting the *const (dyn Trait + Auto) to *const dyn Auto upcast coercions to no longer change the vtable (which they currently semantically do, even though it gets codegenned to a no-op)

Either way, this comment is trying to explain that we need to do more than just change this to return Ok(CastKind::PtrPtrCast) here.

[lukas-code]

=This may have just been an oversight of mine when authoring the dyn Trait -> dyn Auto PR.

@compiler-errors I'm not following, what oversight and PR are you talking about here?

[WaffleLapkin]

I'd say that we should relax the requirement. Either by allowing empty traits (!has_own_existential_vtable_entries) to have any vtable, or by making it actually compare vtables (something like own_existential_vtable_entries_iter(actual).chain(repeat(Vacant)).zip(own_existential_vtable_entries_iter(expected)).all(|a, b| a == b))..

[WaffleLapkin]

I would say that this is fine, while both casts guarantee to return a pointer with a valid vtable for dyn Send, there is no guarantee that it will or will not be valid for anything else.

But also, the upcast coercions don't change the vtable either.

[lukas-code]

But also, the upcast coercions don't change the vtable either.

They do in const eval / miri where this is currently considered UB:

trait Trait {}
impl Trait for () {}

fn main() {
    let ptr: *const (dyn Trait + Send) = &();

    let ptr2: *const dyn Send = ptr as *const dyn Send;
    let _: *const (dyn Trait + Send) = unsafe { std::mem::transmute(ptr2) };
}

error: Undefined Behavior: constructing invalid value: wrong trait in wide pointer vtable: expected `Trait + std::marker::Send`, but encountered `std::marker::Send`
 --> src/main.rs:8:49
  |
8 |     let _: *const (dyn Trait + Send) = unsafe { std::mem::transmute(ptr2) };
  |                                                 ^^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: wrong trait in wide pointer vtable: expected `Trait + std::marker::Send`, but encountered `std::marker::Send`
  |

(you need -Zextra-const-ub-checks to trigger the check in CTFE)

there is no guarantee that it will or will not be valid

For the sake of guarantees for stable code I think it makes sense to leave it unspecified whether this transmute is valid.

But for someone who writes unsafe code and tests it with miri I think it would be very confusing that the first case is UB, but the second case is not -- both of these are an as cast followed by a transmute, nothing here makes it obvious that the as cast actually does something different for each case.

---

Also we don't need to find a solution here: In this PR I'm just trying to add a comment to explain that there is a problem that needs to be solved before we can allow dropping the principal in non-coercion casts, and that allowing these casts won't be a one-line fix.

[lukas-code]

I've updated the comment -- please take another look whether it is any more understandable now and/or still missing something.

also cc @RalfJung can you please double-check whether my assumptions about MIR semantics are correct here (in 46cc5e9)? Thanks!

[RalfJung]

What's a "non-coercion cast"? I assumed that foo as type will trigger coercions as required to perform the cast.

[lukas-code]

Yes, a T as U cast will always trigger a coercion if possible. With "non-coercion" cast, I mean a cast that did not trigger a coercion, i.e. the case where it's not possible to coerce from T to U. Ferrocence calls these "specialized casts", but I don't think we have an official term for them?

[lukas-code]

@bors r- because the comment needs more clarification

[compiler-errors]

I think it's fine that we've documented the status quo.

@bors r+

[bors]

📌 Commit 46cc5e9 has been approved by compiler-errors

It is now in the queue for this repository.

[RalfJung]

Also see rust-lang/reference#1661