Replace #[rustc_do_not_implement_via_object] with #[rustc_dyn_incompatible_trait] by zachs18 · Pull Request #148637 · rust-lang/rust (original) (raw)

Background: #[rustc_do_not_implement_via_object] on a trait currently still allows dyn Trait to exist (if the trait is otherwise dyn-compatible), it just means that dyn Trait does not automatically implement Trait via the normal object candidate. For some traits, this means that dyn Trait does not implement Trait at all (e.g. Unsize and Tuple). For some traits, this means that dyn Trait implements Trait, but with different associated types (e.g. Pointee, DiscriminantKind). Both of these cases can can cause issues with codegen , as seen in #148089 (and #148089 (comment) ), because codegen assumes that if dyn Trait does not implement Trait (including if dyn Trait<Assoc = T> does not implement Trait with Assoc == T), then dyn Trait cannot be constructed, so vtable accesses on dyn Trait are unreachable, but this is not the case if dyn Trait has multiple supertraits: one which is #[rustc_do_not_implement_via_object], and one which we are doing the vtable access to call a method from.

This PR replaces #[rustc_do_not_implement_via_object] with #[rustc_dyn_incompatible_trait], which makes the marked trait dyn-incompatible, making dyn Trait not well-formed, instead of it being well-formed but not implementing Trait. This resolves #148089 by making it not compile.

May fix #148615

The traits that are currently marked #[rustc_do_not_implement_via_object] are: Sized, MetaSized, PointeeSized, TransmuteFrom, Unsize, BikeshedGuaranteedNoDrop, DiscriminantKind, Destruct, Tuple, FnPtr, Pointee. Of these:

• Sized and FnPtr are already not dyn-compatible (FnPtr: Copy)
• MetaSized
  • Removed #[rustc_do_not_implement_via_object]. Still dyn-compatible after this change. See question 1 below
• PointeeSized
  • Removed #[rustc_do_not_implement_via_object]. It doesn't seem to have been doing anything anyway (playground), since PointeeSized is removed before trait solving(?).
• Pointee, DiscriminantKind, Unsize, and Tuple being dyn-compatible without having dyn Trait: Trait (with same assoc tys) can be observed to cause codegen issues (Segfault on nightly with feature Pointer Metadata #148089) so should be made dyn-incompatible
• Destruct, TransmuteFrom, and BikeshedGuaranteedNoDrop I'm not sure if would be useful as object types, but they can be relaxed to being dyn-compatible later if it is determined they should be.

Questions before merge:

1. dyn MetaSized: MetaSized having both SizedCandidate and ObjectCandidate
   1. I'm not sure if the checks in compiler/rustc_trait_selection/src/traits/project.rs and compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs were "load-bearing" for MetaSized (which is the only trait that was previously #[rustc_do_not_implement_via_object] that is still dyn-compatible after this change). Is it fine to just remove them? Removing them (as I did in the second commit) doesn't change any UI test results.
   2. IIUC, dyn MetaSized could get its MetaSized implementation in two ways: the object candidate (the normal dyn Trait: Trait) that was supressed by #[rustc_do_not_implement_via_object], and the SizedCandidate (that all dyn Trait get for dyn Trait: MetaSized). Given that MetaSized has no associated types or methods, is it fine that these both exist now? Or is it better to only have the SizedCandidate and leave these checks in (i.e. drop the second commit, and remove the FIXMEs)?
2. Diagnostics improvements?
   1. The diagnostics are kinda bad. If you have a trait Foo: Pointee {}, you now get a note that reads like Foo "opted out of dyn-compatbility", when really Pointee did that.
3. compiler/rustc_hir/src/attrs/encode_cross_crate.rs: Should DynIncompatibleTrait attribute be encoded cross crate? I'm not sure what this actually does. diagnostic example

#![feature(ptr_metadata)]

trait C: std::ptr::Pointee {}

fn main() { let y: &dyn C; }

error[E0038]: the trait C is not dyn compatible --> c.rs:6:17 | 6 | let y: &dyn C; | ^ C is not dyn compatible | note: for a trait to be dyn compatible it needs to allow building a vtable for more information, visit https://doc.rust-lang.org/reference/items/traits.html#dyn-compatibility --> /home/zachary/opt_mount/zachary/Programming/rust-compiler-2/library/core/src/ptr/metadata.rs:57:1 | 57 | #[rustc_dyn_incompatible_trait] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ...because it opted out of dyn-compatbility | ::: c.rs:3:7 | 3 | trait C: std::ptr::Pointee {} | - this trait is not dyn compatible...

error: aborting due to 1 previous error

For more information about this error, try rustc --explain E0038.