Add a debug assertion in codegen that unsize casts of the same principal trait def id are truly NOPs

Author: compiler-errors

compiler/rustc_codegen_cranelift/src/unsize.rs

@@ -34,7 +34,22 @@ pub(crate) fn unsized_info<'tcx>(
             let old_info =
                 old_info.expect("unsized_info: missing old info for trait upcasting coercion");
             if data_a.principal_def_id() == data_b.principal_def_id() {
-                // A NOP cast that doesn't actually change anything, should be allowed even with invalid vtables.
+                // Codegen takes advantage of the additional assumption, where if the
+                // principal trait def id of what's being casted doesn't change,
+                // then we don't need to adjust the vtable at all. This
+                // corresponds to the fact that `dyn Tr<A>: Unsize<dyn Tr<B>>`
+                // requires that `A = B`; we don't allow *upcasting* objects
+                // between the same trait with different args. If we, for
+                // some reason, were to relax the `Unsize` trait, it could become
+                // unsound, so let's assert here that the trait refs are *equal*.
+                //
+                // We can use `assert_eq` because the binders should have been anonymized,
+                // and because higher-ranked equality now requires the binders are equal.
+                debug_assert_eq!(
+                    data_a.principal(),
+                    data_b.principal(),
+                    "NOP unsize vtable changed principal trait ref: {data_a} -> {data_b}"
+                );
                 return old_info;
             }
 

compiler/rustc_codegen_ssa/src/base.rs

@@ -125,8 +125,28 @@ fn unsized_info<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             let old_info =
                 old_info.expect("unsized_info: missing old info for trait upcasting coercion");
             if data_a.principal_def_id() == data_b.principal_def_id() {
-                // A NOP cast that doesn't actually change anything, should be allowed even with
-                // invalid vtables.
+                // Codegen takes advantage of the additional assumption, where if the
+                // principal trait def id of what's being casted doesn't change,
+                // then we don't need to adjust the vtable at all. This
+                // corresponds to the fact that `dyn Tr<A>: Unsize<dyn Tr<B>>`
+                // requires that `A = B`; we don't allow *upcasting* objects
+                // between the same trait with different args. If we, for
+                // some reason, were to relax the `Unsize` trait, it could become
+                // unsound, so let's assert here that the trait refs are *equal*.
+                //
+                // We can use `assert_eq` because the binders should have been anonymized,
+                // and because higher-ranked equality now requires the binders are equal.
+                debug_assert_eq!(
+                    data_a.principal(),
+                    data_b.principal(),
+                    "NOP unsize vtable changed principal trait ref: {data_a} -> {data_b}"
+                );
+
+                // A NOP cast that doesn't actually change anything, let's avoid any
+                // unnecessary work. This relies on the assumption that if the principal
+                // traits are equal, then the associated type bounds (`dyn Trait<Assoc=T>`)
+                // are also equal, which is ensured by the fact that normalization is
+                // a function and we do not allow overlapping impls.
                 return old_info;
             }
 

compiler/rustc_mir_transform/src/validate.rs

@@ -1233,21 +1233,6 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                         )) {
                             self.fail(location, format!("Unsize coercion, but `{op_ty}` isn't coercible to `{target_type}`"));
                         }
-
-                        // FIXME: Codegen has an additional assumption, where if the
-                        // principal trait def id of what's being casted doesn't change,
-                        // then we don't need to adjust the vtable at all. This
-                        // corresponds to the fact that `dyn Tr<A>: Unsize<dyn Tr<B>>`
-                        // requires that `A = B`; we don't allow *upcasting* objects
-                        // between the same trait with different args. Nothing actually
-                        // validates this, though. While it's true right now, if we for
-                        // some reason were to relax the `Unsize` trait, it could become
-                        // unsound. We should eventually validate that, but it would
-                        // require peeling `&Box<Struct<.., dyn Tr<A>, ..>>` down to
-                        // the trait object that's being unsized, and that's rather
-                        // annoying, and also it would need to be opportunistic since
-                        // this MIR is not yet fully monomorphized, so we may bottom
-                        // out in an alias or a projection or something.
                     }
                     CastKind::PointerCoercion(PointerCoercion::DynStar, _) => {
                         // FIXME(dyn-star): make sure nothing needs to be done here.