Ban ArrayToPointer and MutToConstPointer from runtime MIR

scottmcm · scottmcm · commit 55fc6bdcb344 · 2024-06-14T17:59:54.000-07:00
Apparently MIR borrowck cares about at least one of these for checking variance.

In runtime MIR, though, there's no need for them as `PtrToPtr` does the same thing.

(Banning them simplifies passes like GVN that no longer need to handle multiple cast possibilities.)
diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs
@@ -127,6 +127,9 @@ pub enum AnalysisPhase {
     /// * [`StatementKind::AscribeUserType`]
     /// * [`StatementKind::Coverage`] with [`CoverageKind::BlockMarker`] or [`CoverageKind::SpanMarker`]
     /// * [`Rvalue::Ref`] with `BorrowKind::Fake`
+    /// * [`CastKind::PointerCoercion`] with any of the following:
+    ///   * [`PointerCoercion::ArrayToPointer`]
+    ///   * [`PointerCoercion::MutToConstPointer`]
     ///
     /// Furthermore, `Deref` projections must be the first projection within any place (if they
     /// appear at all)
@@ -1281,8 +1284,7 @@ pub enum Rvalue<'tcx> {
     ///
     /// This allows for casts from/to a variety of types.
     ///
-    /// **FIXME**: Document exactly which `CastKind`s allow which types of casts. Figure out why
-    /// `ArrayToPointer` and `MutToConstPointer` are special.
+    /// **FIXME**: Document exactly which `CastKind`s allow which types of casts.
     Cast(CastKind, Operand<'tcx>, Ty<'tcx>),
 
     /// * `Offset` has the same semantics as [`offset`](pointer::offset), except that the second
@@ -1362,6 +1364,13 @@ pub enum CastKind {
     PointerWithExposedProvenance,
     /// Pointer related casts that are done by coercions. Note that reference-to-raw-ptr casts are
     /// translated into `&raw mut/const *r`, i.e., they are not actually casts.
+    ///
+    /// The following are allowed in [`AnalysisPhase::Initial`] as they're needed for borrowck,
+    /// but after that are forbidden (including in all phases of runtime MIR):
+    /// * [`PointerCoercion::ArrayToPointer`]
+    /// * [`PointerCoercion::MutToConstPointer`]
+    ///
+    /// Both are runtime nops, so should be [`CastKind::PtrToPtr`] instead in runtime MIR.
     PointerCoercion(PointerCoercion),
     /// Cast into a dyn* object.
     DynStar,
diff --git a/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs b/compiler/rustc_mir_transform/src/cleanup_post_borrowck.rs
@@ -18,7 +18,8 @@
 
 use crate::MirPass;
 use rustc_middle::mir::coverage::CoverageKind;
-use rustc_middle::mir::{Body, BorrowKind, Rvalue, StatementKind, TerminatorKind};
+use rustc_middle::mir::{Body, BorrowKind, CastKind, Rvalue, StatementKind, TerminatorKind};
+use rustc_middle::ty::adjustment::PointerCoercion;
 use rustc_middle::ty::TyCtxt;
 
 pub struct CleanupPostBorrowck;
@@ -36,6 +37,22 @@ impl<'tcx> MirPass<'tcx> for CleanupPostBorrowck {
                         CoverageKind::BlockMarker { .. } | CoverageKind::SpanMarker { .. },
                     )
                     | StatementKind::FakeRead(..) => statement.make_nop(),
+                    StatementKind::Assign(box (
+                        _,
+                        Rvalue::Cast(
+                            ref mut cast_kind @ CastKind::PointerCoercion(
+                                PointerCoercion::ArrayToPointer
+                                | PointerCoercion::MutToConstPointer,
+                            ),
+                            ..,
+                        ),
+                    )) => {
+                        // BorrowCk needed to track whether these cases were coercions or casts,
+                        // to know whether to check lifetimes in their pointees,
+                        // but from now on that distinction doesn't matter,
+                        // so just make them ordinary pointer casts instead.
+                        *cast_kind = CastKind::PtrToPtr;
+                    }
                     _ => (),
                 }
             }
diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs
@@ -572,11 +572,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                     let ret = self.ecx.ptr_to_ptr(&src, to).ok()?;
                     ret.into()
                 }
-                CastKind::PointerCoercion(
-                    ty::adjustment::PointerCoercion::MutToConstPointer
-                    | ty::adjustment::PointerCoercion::ArrayToPointer
-                    | ty::adjustment::PointerCoercion::UnsafeFnPointer,
-                ) => {
+                CastKind::PointerCoercion(ty::adjustment::PointerCoercion::UnsafeFnPointer) => {
                     let src = self.evaluated[value].as_ref()?;
                     let src = self.ecx.read_immediate(src).ok()?;
                     let to = self.ecx.layout_of(to).ok()?;
@@ -1165,10 +1161,10 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             }
         }
 
-        if let PtrToPtr | PointerCoercion(MutToConstPointer) = kind
+        if let PtrToPtr = kind
             && let Value::Cast { kind: inner_kind, value: inner_value, from: inner_from, to: _ } =
                 *self.get(value)
-            && let PtrToPtr | PointerCoercion(MutToConstPointer) = inner_kind
+            && let PtrToPtr = inner_kind
         {
             from = inner_from;
             value = inner_value;
diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs
@@ -49,11 +49,11 @@ mod abort_unwinding_calls;
 mod add_call_guards;
 mod add_moves_for_packed_drops;
 mod add_retag;
+mod add_subtyping_projections;
+mod check_alignment;
 mod check_const_item_mutation;
 mod check_packed_ref;
-mod remove_place_mention;
 // This pass is public to allow external drivers to perform MIR cleanup
-mod add_subtyping_projections;
 pub mod cleanup_post_borrowck;
 mod copy_prop;
 mod coroutine;
@@ -92,6 +92,7 @@ mod prettify;
 mod promote_consts;
 mod ref_prop;
 mod remove_noop_landing_pads;
+mod remove_place_mention;
 mod remove_storage_markers;
 mod remove_uninit_drops;
 mod remove_unneeded_drops;
@@ -101,7 +102,6 @@ mod reveal_all;
 mod shim;
 mod ssa;
 // This pass is public to allow external drivers to perform MIR cleanup
-mod check_alignment;
 pub mod simplify;
 mod simplify_branches;
 mod simplify_comparison_integral;
diff --git a/compiler/rustc_mir_transform/src/validate.rs b/compiler/rustc_mir_transform/src/validate.rs
@@ -1187,6 +1187,9 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                             "CastKind::{kind:?} output must be a raw const pointer, not {:?}",
                             ty::RawPtr(_, Mutability::Not)
                         );
+                        if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
+                            self.fail(location, format!("After borrowck, MIR disallows {kind:?}"));
+                        }
                     }
                     CastKind::PointerCoercion(PointerCoercion::ArrayToPointer) => {
                         // FIXME: Check pointee types
@@ -1200,6 +1203,9 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                             "CastKind::{kind:?} output must be a raw pointer, not {:?}",
                             ty::RawPtr(..)
                         );
+                        if self.mir_phase >= MirPhase::Analysis(AnalysisPhase::PostCleanup) {
+                            self.fail(location, format!("After borrowck, MIR disallows {kind:?}"));
+                        }
                     }
                     CastKind::PointerCoercion(PointerCoercion::Unsize) => {
                         // This is used for all `CoerceUnsized` types,
@@ -1211,7 +1217,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
                         if !input_valid || !target_valid {
                             self.fail(
                                 location,
-                                format!("Wrong cast kind {kind:?} for the type {op_ty}",),
+                                format!("Wrong cast kind {kind:?} for the type {op_ty}"),
                             );
                         }
                     }
diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff
@@ -80,7 +80,7 @@
   
       bb4: {
           StorageDead(_8);
--         _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
+-         _11 = _6 as *const [bool; 0] (PtrToPtr);
 -         _5 = NonNull::<[bool; 0]> { pointer: _11 };
 +         _11 = const {0x1 as *const [bool; 0]};
 +         _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff
@@ -84,7 +84,7 @@
   
       bb5: {
           StorageDead(_8);
--         _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
+-         _11 = _6 as *const [bool; 0] (PtrToPtr);
 -         _5 = NonNull::<[bool; 0]> { pointer: _11 };
 +         _11 = const {0x1 as *const [bool; 0]};
 +         _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff
@@ -80,7 +80,7 @@
   
       bb4: {
           StorageDead(_8);
--         _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
+-         _11 = _6 as *const [bool; 0] (PtrToPtr);
 -         _5 = NonNull::<[bool; 0]> { pointer: _11 };
 +         _11 = const {0x1 as *const [bool; 0]};
 +         _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
diff --git a/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff b/tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff
@@ -84,7 +84,7 @@
   
       bb5: {
           StorageDead(_8);
--         _11 = _6 as *const [bool; 0] (PointerCoercion(MutToConstPointer));
+-         _11 = _6 as *const [bool; 0] (PtrToPtr);
 -         _5 = NonNull::<[bool; 0]> { pointer: _11 };
 +         _11 = const {0x1 as *const [bool; 0]};
 +         _5 = const NonNull::<[bool; 0]> {{ pointer: {0x1 as *const [bool; 0]} }};
diff --git a/tests/mir-opt/instsimplify/casts.roundtrip.InstSimplify.diff b/tests/mir-opt/instsimplify/casts.roundtrip.InstSimplify.diff
@@ -14,7 +14,7 @@
           StorageLive(_4);
           _4 = _1;
           _3 = move _4 as *mut u8 (PtrToPtr);
-          _2 = move _3 as *const u8 (PointerCoercion(MutToConstPointer));
+          _2 = move _3 as *const u8 (PtrToPtr);
           StorageDead(_4);
           StorageDead(_3);
 -         _0 = move _2 as *const u8 (PtrToPtr);
diff --git a/tests/mir-opt/instsimplify/casts.rs b/tests/mir-opt/instsimplify/casts.rs
@@ -21,7 +21,7 @@ pub fn roundtrip(x: *const u8) -> *const u8 {
     // CHECK-LABEL: fn roundtrip(
     // CHECK: _4 = _1;
     // CHECK: _3 = move _4 as *mut u8 (PtrToPtr);
-    // CHECK: _2 = move _3 as *const u8 (PointerCoercion(MutToConstPointer));
+    // CHECK: _2 = move _3 as *const u8 (PtrToPtr);
     x as *mut u8 as *const u8
 }
 
diff --git a/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir
@@ -79,7 +79,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
         _7 = _4 as *mut T (PtrToPtr);
         _8 = Offset(_7, _3);
         StorageDead(_7);
-        _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
+        _9 = move _8 as *const T (PtrToPtr);
         StorageDead(_8);
         goto -> bb3;
     }
diff --git a/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-unwind.mir
@@ -79,7 +79,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
         _7 = _4 as *mut T (PtrToPtr);
         _8 = Offset(_7, _3);
         StorageDead(_7);
-        _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
+        _9 = move _8 as *const T (PtrToPtr);
         StorageDead(_8);
         goto -> bb3;
     }
diff --git a/tests/mir-opt/pre-codegen/slice_iter.forward_loop.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/slice_iter.forward_loop.PreCodegen.after.panic-abort.mir
@@ -71,7 +71,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
         _7 = _4 as *mut T (PtrToPtr);
         _8 = Offset(_7, _3);
         StorageDead(_7);
-        _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
+        _9 = move _8 as *const T (PtrToPtr);
         StorageDead(_8);
         goto -> bb3;
     }
diff --git a/tests/mir-opt/pre-codegen/slice_iter.forward_loop.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/slice_iter.forward_loop.PreCodegen.after.panic-unwind.mir
@@ -71,7 +71,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
         _7 = _4 as *mut T (PtrToPtr);
         _8 = Offset(_7, _3);
         StorageDead(_7);
-        _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
+        _9 = move _8 as *const T (PtrToPtr);
         StorageDead(_8);
         goto -> bb3;
     }
diff --git a/tests/mir-opt/pre-codegen/slice_iter.reverse_loop.PreCodegen.after.panic-abort.mir b/tests/mir-opt/pre-codegen/slice_iter.reverse_loop.PreCodegen.after.panic-abort.mir
@@ -79,7 +79,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
         _7 = _4 as *mut T (PtrToPtr);
         _8 = Offset(_7, _3);
         StorageDead(_7);
-        _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
+        _9 = move _8 as *const T (PtrToPtr);
         StorageDead(_8);
         goto -> bb3;
     }
diff --git a/tests/mir-opt/pre-codegen/slice_iter.reverse_loop.PreCodegen.after.panic-unwind.mir b/tests/mir-opt/pre-codegen/slice_iter.reverse_loop.PreCodegen.after.panic-unwind.mir
@@ -79,7 +79,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
         _7 = _4 as *mut T (PtrToPtr);
         _8 = Offset(_7, _3);
         StorageDead(_7);
-        _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));
+        _9 = move _8 as *const T (PtrToPtr);
         StorageDead(_8);
         goto -> bb3;
     }
diff --git a/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-abort.mir b/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-abort.mir
@@ -64,7 +64,7 @@ fn array_casts() -> () {
         StorageLive(_4);
         _4 = &mut _1;
         _3 = &raw mut (*_4);
-        _2 = move _3 as *mut usize (PointerCoercion(ArrayToPointer));
+        _2 = move _3 as *mut usize (PtrToPtr);
         StorageDead(_3);
         StorageDead(_4);
         StorageLive(_5);
@@ -87,7 +87,7 @@ fn array_casts() -> () {
         StorageLive(_11);
         _11 = &_8;
         _10 = &raw const (*_11);
-        _9 = move _10 as *const usize (PointerCoercion(ArrayToPointer));
+        _9 = move _10 as *const usize (PtrToPtr);
         StorageDead(_10);
         StorageDead(_11);
         StorageLive(_12);
diff --git a/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-unwind.mir b/tests/mir-opt/retag.array_casts.SimplifyCfg-pre-optimizations.after.panic-unwind.mir
@@ -64,7 +64,7 @@ fn array_casts() -> () {
         StorageLive(_4);
         _4 = &mut _1;
         _3 = &raw mut (*_4);
-        _2 = move _3 as *mut usize (PointerCoercion(ArrayToPointer));
+        _2 = move _3 as *mut usize (PtrToPtr);
         StorageDead(_3);
         StorageDead(_4);
         StorageLive(_5);
@@ -87,7 +87,7 @@ fn array_casts() -> () {
         StorageLive(_11);
         _11 = &_8;
         _10 = &raw const (*_11);
-        _9 = move _10 as *const usize (PointerCoercion(ArrayToPointer));
+        _9 = move _10 as *const usize (PtrToPtr);
         StorageDead(_10);
         StorageDead(_11);
         StorageLive(_12);

Original file line number	Diff line number	Diff line change
`@@ -572,11 +572,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {`
`572`	`572`	`let ret = self.ecx.ptr_to_ptr(&src, to).ok()?;`
`573`	`573`	`ret.into()`
`574`	`574`	`}`
`575`		`- CastKind::PointerCoercion(`
`576`		`- ty::adjustment::PointerCoercion::MutToConstPointer`
`577`		`- \| ty::adjustment::PointerCoercion::ArrayToPointer`
`578`		`- \| ty::adjustment::PointerCoercion::UnsafeFnPointer,`
`579`		`- ) => {`
	`575`	`+ CastKind::PointerCoercion(ty::adjustment::PointerCoercion::UnsafeFnPointer) => {`
`580`	`576`	`let src = self.evaluated[value].as_ref()?;`
`581`	`577`	`let src = self.ecx.read_immediate(src).ok()?;`
`582`	`578`	`let to = self.ecx.layout_of(to).ok()?;`
`@@ -1165,10 +1161,10 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {`
`1165`	`1161`	`}`
`1166`	`1162`	`}`
`1167`	`1163`
`1168`		`- if let PtrToPtr \| PointerCoercion(MutToConstPointer) = kind`
	`1164`	`+ if let PtrToPtr = kind`
`1169`	`1165`	`&& let Value::Cast { kind: inner_kind, value: inner_value, from: inner_from, to: _ } =`
`1170`	`1166`	`*self.get(value)`
`1171`		`- && let PtrToPtr \| PointerCoercion(MutToConstPointer) = inner_kind`
	`1167`	`+ && let PtrToPtr = inner_kind`
`1172`	`1168`	`{`
`1173`	`1169`	`from = inner_from;`
`1174`	`1170`	`value = inner_value;`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ pub fn roundtrip(x: const u8) -> const u8 {`
`21`	`21`	`// CHECK-LABEL: fn roundtrip(`
`22`	`22`	`// CHECK: _4 = _1;`
`23`	`23`	`// CHECK: _3 = move _4 as *mut u8 (PtrToPtr);`
`24`		`- // CHECK: _2 = move _3 as *const u8 (PointerCoercion(MutToConstPointer));`
	`24`	`+ // CHECK: _2 = move _3 as *const u8 (PtrToPtr);`
`25`	`25`	`x as mut u8 as const u8`
`26`	`26`	`}`
`27`	`27`
Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {`
`79`	`79`	`_7 = _4 as *mut T (PtrToPtr);`
`80`	`80`	`_8 = Offset(_7, _3);`
`81`	`81`	`StorageDead(_7);`
`82`		`- _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));`
	`82`	`+ _9 = move _8 as *const T (PtrToPtr);`
`83`	`83`	`StorageDead(_8);`
`84`	`84`	`goto -> bb3;`
`85`	`85`	`}`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {`
`71`	`71`	`_7 = _4 as *mut T (PtrToPtr);`
`72`	`72`	`_8 = Offset(_7, _3);`
`73`	`73`	`StorageDead(_7);`
`74`		`- _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));`
	`74`	`+ _9 = move _8 as *const T (PtrToPtr);`
`75`	`75`	`StorageDead(_8);`
`76`	`76`	`goto -> bb3;`
`77`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {`
`79`	`79`	`_7 = _4 as *mut T (PtrToPtr);`
`80`	`80`	`_8 = Offset(_7, _3);`
`81`	`81`	`StorageDead(_7);`
`82`		`- _9 = move _8 as *const T (PointerCoercion(MutToConstPointer));`
	`82`	`+ _9 = move _8 as *const T (PtrToPtr);`
`83`	`83`	`StorageDead(_8);`
`84`	`84`	`goto -> bb3;`
`85`	`85`	`}`