Draft: Miri/CTFE engine: Unions are not scalar

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -118,8 +118,9 @@ pub(super) fn op_to_const<'tcx>(
     // instead allow `ConstValue::Scalar` to store `ScalarMaybeUninit`, but that would affect all
     // the usual cases of extracting e.g. a `usize`, without there being a real use case for the
     // `Undef` situation.
+    // FIXME: This is all a horrible hack and should go away once we are properly using valtrees.
     let try_as_immediate = match op.layout.abi {
-        Abi::Scalar(..) => true,
+        Abi::Scalar(..) if crate::interpret::type_is_scalar(ecx.tcx.tcx, op.layout.ty) => true,
         Abi::ScalarPair(..) => match op.layout.ty.kind() {
             ty::Ref(_, inner, _) => match *inner.kind() {
                 ty::Slice(elem) => elem == ecx.tcx.types.u8,

compiler/rustc_const_eval/src/interpret/mod.rs

@@ -30,4 +30,5 @@ pub use self::validity::{CtfeValidationMode, RefTracking};
 pub use self::visitor::{MutValueVisitor, ValueVisitor};
 
 crate use self::intrinsics::eval_nullary_intrinsic;
+crate use self::operand::type_is_scalar;
 use eval_context::{from_known_layout, mir_assign_valid_types};

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -9,7 +9,7 @@ use rustc_hir::def::Namespace;
 use rustc_macros::HashStable;
 use rustc_middle::ty::layout::{LayoutOf, PrimitiveExt, TyAndLayout};
 use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter, Printer};
-use rustc_middle::ty::{ConstInt, Ty};
+use rustc_middle::ty::{ConstInt, Ty, TyCtxt};
 use rustc_middle::{mir, ty};
 use rustc_target::abi::{Abi, HasDataLayout, Size, TagEncoding};
 use rustc_target::abi::{VariantIdx, Variants};
@@ -246,6 +246,55 @@ impl<'tcx, Tag: Provenance> ImmTy<'tcx, Tag> {
     }
 }
 
+/// See `try_read_immediate_from_mplace` for what this function is about.
+/// Exposed to the rest of the crate since some hacky code in `eval_queries.rs` (that shouldn't
+/// exist) also needs it.
+pub(crate) fn type_is_scalar<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
+    match ty.kind() {
+        // Primitive types. We *have to* handle these since the primitive operations that
+        // act on them require it.
+        ty::Bool
+        | ty::Char
+        | ty::Float(_)
+        | ty::Int(_)
+        | ty::Uint(_)
+        | ty::RawPtr(..)
+        | ty::Ref(..)
+        | ty::FnPtr(..)
+        | ty::FnDef(..)
+        | ty::Never => true,
+        // Compound types. We have to exclude unions here.
+        ty::Adt(adt_def, substs) => {
+            if adt_def.is_union() {
+                // Unions are explicitly allowed to be partially initialized, so we have to
+                // exclude them here.
+                false
+            } else {
+                // Check all fields of all variants, to make sure there is no union hiding here.
+                adt_def.all_fields().all(|field| type_is_scalar(tcx, field.ty(tcx, substs)))
+            }
+        }
+        ty::Tuple(..) => {
+            // If all fields are scalar, we are good.
+            ty.tuple_fields().iter().all(|field| type_is_scalar(tcx, field))
+        }
+        // FIXME: can these ever have Scalar ABI?
+        ty::Closure(..) | ty::Generator(..) => false,
+        // Types that don't have scalar layout to begin with.
+        ty::Array(..) | ty::Slice(..) | ty::Str | ty::Dynamic(..) | ty::Foreign(..) => false,
+        // Types we should not uusally see here, but when called from CTFE op_to_const these can
+        // actually happen.
+        ty::Error(_)
+        | ty::Infer(..)
+        | ty::Placeholder(..)
+        | ty::Bound(..)
+        | ty::Param(..)
+        | ty::Opaque(..)
+        | ty::Projection(..)
+        | ty::GeneratorWitness(..) => false,
+    }
+}
+
 impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Try reading an immediate in memory; this is interesting particularly for `ScalarPair`.
     /// Returns `None` if the layout does not permit loading this as a value.
@@ -266,12 +315,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }));
         };
 
+        // It may seem like all types with `Scalar` or `ScalarPair` ABI are fair game at this point.
+        // However, `MaybeUninit<u64>` is considered a `Scalar` as far as its layout is concerned --
+        // and yet cannot be represented by an interpreter `Scalar`, since we have to handle the
+        // case where some of the bytes are initialized and others are not. So, we need an extra
+        // check that walks over the type of `mplace` to make sure it is truly correct to treat this
+        // like a `Scalar` (or `ScalarPair`).
         match mplace.layout.abi {
-            Abi::Scalar(..) => {
+            Abi::Scalar(..) if type_is_scalar(self.tcx.tcx, mplace.layout.ty) => {
                 let scalar = alloc.read_scalar(alloc_range(Size::ZERO, mplace.layout.size))?;
                 Ok(Some(ImmTy { imm: scalar.into(), layout: mplace.layout }))
             }
-            Abi::ScalarPair(a, b) => {
+            Abi::ScalarPair(a, b) if type_is_scalar(self.tcx.tcx, mplace.layout.ty) => {
                 // We checked `ptr_align` above, so all fields will have the alignment they need.
                 // We would anyway check against `ptr_align.restrict_for_offset(b_offset)`,
                 // which `ptr.offset(b_offset)` cannot possibly fail to satisfy.

src/test/ui/consts/issue-69488.rs

@@ -0,0 +1,33 @@
+// run-pass
+
+#![feature(const_ptr_write)]
+#![feature(const_fn_trait_bound)]
+#![feature(const_mut_refs)]
+
+// Or, equivalently: `MaybeUninit`.
+pub union BagOfBits<T: Copy> {
+    uninit: (),
+    _storage: T,
+}
+
+pub const fn make_1u8_bag<T: Copy>() -> BagOfBits<T> {
+    assert!(core::mem::size_of::<T>() >= 1);
+    let mut bag = BagOfBits { uninit: () };
+    unsafe { (&mut bag as *mut _ as *mut u8).write(1); };
+    bag
+}
+
+pub fn check_bag<T: Copy>(bag: &BagOfBits<T>) {
+    let val = unsafe { (bag as *const _ as *const u8).read() };
+    assert_eq!(val, 1);
+}
+
+fn main() {
+    check_bag(&make_1u8_bag::<[usize; 1]>()); // Fine
+    check_bag(&make_1u8_bag::<usize>()); // Fine
+
+    const CONST_ARRAY_BAG: BagOfBits<[usize; 1]> = make_1u8_bag();
+    check_bag(&CONST_ARRAY_BAG); // Fine.
+    const CONST_USIZE_BAG: BagOfBits<usize> = make_1u8_bag();
+    check_bag(&CONST_USIZE_BAG); // Panics
+}

src/test/ui/consts/issue-94371.rs

@@ -0,0 +1,16 @@
+// check-pass
+
+#![feature(const_swap)]
+#![feature(const_mut_refs)]
+
+#[repr(C)]
+struct Demo(u64, bool, u64, u32, u64, u64, u64);
+
+const C: (Demo, Demo) = {
+    let mut x = Demo(1, true, 3, 4, 5, 6, 7);
+    let mut y = Demo(10, false, 12, 13, 14, 15, 16);
+    std::mem::swap(&mut x, &mut y);
+    (x, y)
+};
+
+fn main() {}