fix some issues around ZST handling

compiler/rustc_abi/src/layout.rs

@@ -157,8 +157,10 @@ pub trait LayoutCalculator {
         // for non-ZST uninhabited data (mostly partial initialization).
         let absent = |fields: &IndexSlice<FieldIdx, Layout<'_>>| {
             let uninhabited = fields.iter().any(|f| f.abi().is_uninhabited());
-            let is_zst = fields.iter().all(|f| f.0.is_zst());
-            uninhabited && is_zst
+            // We cannot ignore alignment; that might lead us to entirely discard a variant and
+            // produce an enum that is less aligned than it should be!
+            let is_1zst = fields.iter().all(|f| f.0.is_1zst());
+            uninhabited && is_1zst
         };
         let (present_first, present_second) = {
             let mut present_variants = variants
@@ -357,10 +359,8 @@ pub trait LayoutCalculator {
                 // It'll fit, but we need to make some adjustments.
                 match layout.fields {
                     FieldsShape::Arbitrary { ref mut offsets, .. } => {
-                        for (j, offset) in offsets.iter_enumerated_mut() {
-                            if !variants[i][j].0.is_zst() {
-                                *offset += this_offset;
-                            }
+                        for offset in offsets.iter_mut() {
+                            *offset += this_offset;
                         }
                     }
                     _ => {
@@ -504,7 +504,7 @@ pub trait LayoutCalculator {
                 // to make room for a larger discriminant.
                 for field_idx in st.fields.index_by_increasing_offset() {
                     let field = &field_layouts[FieldIdx::from_usize(field_idx)];
-                    if !field.0.is_zst() || field.align().abi.bytes() != 1 {
+                    if !field.0.is_1zst() {
                         start_align = start_align.min(field.align().abi);
                         break;
                     }
@@ -603,12 +603,15 @@ pub trait LayoutCalculator {
             abi = Abi::Scalar(tag);
         } else {
             // Try to use a ScalarPair for all tagged enums.
+            // That's possible only if we can find a common primitive type for all variants.
             let mut common_prim = None;
             let mut common_prim_initialized_in_all_variants = true;
             for (field_layouts, layout_variant) in iter::zip(variants, &layout_variants) {
                 let FieldsShape::Arbitrary { ref offsets, .. } = layout_variant.fields else {
                     panic!();
                 };
+                // We skip *all* ZST here and later check if we are good in terms of alignment.
+                // This lets us handle some cases involving aligned ZST.
                 let mut fields = iter::zip(field_layouts, offsets).filter(|p| !p.0.0.is_zst());
                 let (field, offset) = match (fields.next(), fields.next()) {
                     (None, None) => {
@@ -954,9 +957,6 @@ fn univariant(
                         };
 
                         (
-                            // Place ZSTs first to avoid "interesting offsets", especially with only one
-                            // or two non-ZST fields. This helps Scalar/ScalarPair layouts.
-                            !f.0.is_zst(),
                             // Then place largest alignments first.
                             cmp::Reverse(alignment_group_key(f)),
                             // Then prioritize niche placement within alignment group according to
@@ -1073,9 +1073,10 @@ fn univariant(
     let size = min_size.align_to(align.abi);
     let mut layout_of_single_non_zst_field = None;
     let mut abi = Abi::Aggregate { sized };
-    // Unpack newtype ABIs and find scalar pairs.
+    // Try to make this a Scalar/ScalarPair.
     if sized && size.bytes() > 0 {
-        // All other fields must be ZSTs.
+        // We skip *all* ZST here and later check if we are good in terms of alignment.
+        // This lets us handle some cases involving aligned ZST.
         let mut non_zst_fields = fields.iter_enumerated().filter(|&(_, f)| !f.0.is_zst());
 
         match (non_zst_fields.next(), non_zst_fields.next(), non_zst_fields.next()) {

compiler/rustc_abi/src/lib.rs

@@ -1660,15 +1660,25 @@ pub struct PointeeInfo {
 
 impl LayoutS {
     /// Returns `true` if the layout corresponds to an unsized type.
+    #[inline]
     pub fn is_unsized(&self) -> bool {
         self.abi.is_unsized()
     }
 
+    #[inline]
     pub fn is_sized(&self) -> bool {
         self.abi.is_sized()
     }
 
+    /// Returns `true` if the type is sized and a 1-ZST (meaning it has size 0 and alignment 1).
+    pub fn is_1zst(&self) -> bool {
+        self.is_sized() && self.size.bytes() == 0 && self.align.abi.bytes() == 1
+    }
+
     /// Returns `true` if the type is a ZST and not unsized.
+    ///
+    /// Note that this does *not* imply that the type is irrelevant for layout! It can still have
+    /// non-trivial alignment constraints. You probably want to use `is_1zst` instead.
     pub fn is_zst(&self) -> bool {
         match self.abi {
             Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } => false,

compiler/rustc_codegen_cranelift/src/unsize.rs

@@ -88,7 +88,8 @@ fn unsize_ptr<'tcx>(
                 let src_f = src_layout.field(fx, i);
                 assert_eq!(src_layout.fields.offset(i).bytes(), 0);
                 assert_eq!(dst_layout.fields.offset(i).bytes(), 0);
-                if src_f.is_zst() {
+                if src_f.is_1zst() {
+                    // We are looking for the one non-1-ZST field; this is not it.
                     continue;
                 }
                 assert_eq!(src_layout.size, src_f.size);
@@ -151,6 +152,7 @@ pub(crate) fn coerce_unsized_into<'tcx>(
                 let dst_f = dst.place_field(fx, FieldIdx::new(i));
 
                 if dst_f.layout().is_zst() {
+                    // No data here, nothing to copy/coerce.
                     continue;
                 }
 

compiler/rustc_codegen_cranelift/src/vtable.rs

@@ -51,8 +51,8 @@ pub(crate) fn get_ptr_and_method_ref<'tcx>(
             'descend_newtypes: while !arg.layout().ty.is_unsafe_ptr() && !arg.layout().ty.is_ref() {
                 for i in 0..arg.layout().fields.count() {
                     let field = arg.value_field(fx, FieldIdx::new(i));
-                    if !field.layout().is_zst() {
-                        // we found the one non-zero-sized field that is allowed
+                    if !field.layout().is_1zst() {
+                        // we found the one non-1-ZST field that is allowed
                         // now find *its* non-zero-sized field, or stop if it's a
                         // pointer
                         arg = field;

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

@@ -445,9 +445,9 @@ pub fn type_di_node<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll D
         ty::RawPtr(ty::TypeAndMut { ty: pointee_type, .. }) | ty::Ref(_, pointee_type, _) => {
             build_pointer_or_reference_di_node(cx, t, pointee_type, unique_type_id)
         }
-        // Box<T, A> may have a non-ZST allocator A. In that case, we
+        // Box<T, A> may have a non-1-ZST allocator A. In that case, we
         // cannot treat Box<T, A> as just an owned alias of `*mut T`.
-        ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_zst() => {
+        ty::Adt(def, args) if def.is_box() && cx.layout_of(args.type_at(1)).is_1zst() => {
             build_pointer_or_reference_di_node(cx, t, t.boxed_ty(), unique_type_id)
         }
         ty::FnDef(..) | ty::FnPtr(_) => build_subroutine_type_di_node(cx, unique_type_id),

compiler/rustc_codegen_ssa/src/base.rs

@@ -202,7 +202,7 @@ pub fn unsize_ptr<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             (src, unsized_info(bx, a, b, old_info))
         }
         (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
-            assert_eq!(def_a, def_b);
+            assert_eq!(def_a, def_b); // implies same number of fields
             let src_layout = bx.cx().layout_of(src_ty);
             let dst_layout = bx.cx().layout_of(dst_ty);
             if src_ty == dst_ty {
@@ -211,7 +211,8 @@ pub fn unsize_ptr<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             let mut result = None;
             for i in 0..src_layout.fields.count() {
                 let src_f = src_layout.field(bx.cx(), i);
-                if src_f.is_zst() {
+                if src_f.is_1zst() {
+                    // We are looking for the one non-1-ZST field; this is not it.
                     continue;
                 }
 
@@ -272,13 +273,14 @@ pub fn coerce_unsized_into<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
         }
 
         (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
-            assert_eq!(def_a, def_b);
+            assert_eq!(def_a, def_b); // implies same number of fields
 
             for i in def_a.variant(FIRST_VARIANT).fields.indices() {
                 let src_f = src.project_field(bx, i.as_usize());
                 let dst_f = dst.project_field(bx, i.as_usize());
 
                 if dst_f.layout.is_zst() {
+                    // No data here, nothing to copy/coerce.
                     continue;
                 }
 

compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -933,8 +933,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         {
                             for i in 0..op.layout.fields.count() {
                                 let field = op.extract_field(bx, i);
-                                if !field.layout.is_zst() {
-                                    // we found the one non-zero-sized field that is allowed
+                                if !field.layout.is_1zst() {
+                                    // we found the one non-1-ZST field that is allowed
                                     // now find *its* non-zero-sized field, or stop if it's a
                                     // pointer
                                     op = field;
@@ -975,10 +975,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
                         {
                             for i in 0..op.layout.fields.count() {
                                 let field = op.extract_field(bx, i);
-                                if !field.layout.is_zst() {
-                                    // we found the one non-zero-sized field that is allowed
-                                    // now find *its* non-zero-sized field, or stop if it's a
-                                    // pointer
+                                if !field.layout.is_1zst() {
+                                    // We found the one non-1-ZST field that is allowed. (The rules
+                                    // for `DispatchFromDyn` ensure there's exactly one such field.)
+                                    // Now find *its* non-zero-sized field, or stop if it's a
+                                    // pointer.
                                     op = field;
                                     continue 'descend_newtypes;
                                 }

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -145,7 +145,7 @@ impl<'tcx, V: CodegenObject> LocalRef<'tcx, V> {
         if layout.is_zst() {
             // Zero-size temporaries aren't always initialized, which
             // doesn't matter because they don't contain data, but
-            // we need something in the operand.
+            // we need something sufficiently aligned in the operand.
             LocalRef::Operand(OperandRef::zero_sized(layout))
         } else {
             LocalRef::PendingOperand

compiler/rustc_codegen_ssa/src/mir/operand.rs

@@ -50,7 +50,8 @@ pub enum OperandValue<V> {
     /// from [`ConstMethods::const_poison`].
     ///
     /// An `OperandValue` *must* be this variant for any type for which
-    /// `is_zst` on its `Layout` returns `true`.
+    /// `is_zst` on its `Layout` returns `true`. Note however that
+    /// these values can still require alignment.
     ZeroSized,
 }
 

compiler/rustc_codegen_ssa/src/mir/place.rs

@@ -114,7 +114,8 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
                     bx.struct_gep(ty, self.llval, 1)
                 }
                 Abi::Scalar(_) | Abi::ScalarPair(..) | Abi::Vector { .. } if field.is_zst() => {
-                    // ZST fields are not included in Scalar, ScalarPair, and Vector layouts, so manually offset the pointer.
+                    // ZST fields (even some that require alignment) are not included in Scalar,
+                    // ScalarPair, and Vector layouts, so manually offset the pointer.
                     bx.gep(bx.cx().type_i8(), self.llval, &[bx.const_usize(offset.bytes())])
                 }
                 Abi::Scalar(_) | Abi::ScalarPair(..) => {

compiler/rustc_codegen_ssa/src/mir/rvalue.rs

@@ -1004,6 +1004,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             mir::Rvalue::Aggregate(..) => {
                 let ty = rvalue.ty(self.mir, self.cx.tcx());
                 let ty = self.monomorphize(ty);
+                // For ZST this can be `OperandValueKind::ZeroSized`.
                 self.cx.spanned_layout_of(ty, span).is_zst()
             }
         }

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -410,21 +410,25 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 self.unsize_into_ptr(src, dest, *s, *c)
             }
             (&ty::Adt(def_a, _), &ty::Adt(def_b, _)) => {
-                assert_eq!(def_a, def_b);
+                assert_eq!(def_a, def_b); // implies same number of fields
 
-                // unsizing of generic struct with pointer fields
-                // Example: `Arc<T>` -> `Arc<Trait>`
-                // here we need to increase the size of every &T thin ptr field to a fat ptr
+                // Unsizing of generic struct with pointer fields, like `Arc<T>` -> `Arc<Trait>`.
+                // There can be extra fields as long as they don't change their type or are 1-ZST.
+                // There might also be no field that actually needs unsizing.
+                let mut found_cast_field = false;
                 for i in 0..src.layout.fields.count() {
                     let cast_ty_field = cast_ty.field(self, i);
-                    if cast_ty_field.is_zst() {
-                        continue;
-                    }
                     let src_field = self.project_field(src, i)?;
                     let dst_field = self.project_field(dest, i)?;
-                    if src_field.layout.ty == cast_ty_field.ty {
+                    if src_field.layout.is_1zst() && cast_ty_field.is_1zst() {
+                        // Skip 1-ZST fields.
+                    } else if src_field.layout.ty == cast_ty_field.ty {
                         self.copy_op(&src_field, &dst_field, /*allow_transmute*/ false)?;
                     } else {
+                        if found_cast_field {
+                            span_bug!(self.cur_span(), "unsize_into: more than one field to cast");
+                        }
+                        found_cast_field = true;
                         self.unsize_into(&src_field, cast_ty_field, &dst_field)?;
                     }
                 }

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -239,6 +239,7 @@ impl<'tcx, Prov: Provenance> ImmTy<'tcx, Prov> {
             // if the entire value is uninit, then so is the field (can happen in ConstProp)
             (Immediate::Uninit, _) => Immediate::Uninit,
             // the field contains no information, can be left uninit
+            // (Scalar/ScalarPair can contain even aligned ZST, not just 1-ZST)
             _ if layout.is_zst() => Immediate::Uninit,
             // some fieldless enum variants can have non-zero size but still `Aggregate` ABI... try
             // to detect those here and also give them no data

compiler/rustc_const_eval/src/interpret/terminator.rs

@@ -684,23 +684,23 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                         }
                         _ => {
                             // Not there yet, search for the only non-ZST field.
+                            // (The rules for `DispatchFromDyn` ensure there's exactly one such field.)
                             let mut non_zst_field = None;
                             for i in 0..receiver.layout.fields.count() {
                                 let field = self.project_field(&receiver, i)?;
-                                let zst =
-                                    field.layout.is_zst() && field.layout.align.abi.bytes() == 1;
+                                let zst = field.layout.is_1zst();
                                 if !zst {
                                     assert!(
                                         non_zst_field.is_none(),
-                                        "multiple non-ZST fields in dyn receiver type {}",
+                                        "multiple non-1-ZST fields in dyn receiver type {}",
                                         receiver.layout.ty
                                     );
                                     non_zst_field = Some(field);
                                 }
                             }
                             receiver = non_zst_field.unwrap_or_else(|| {
                                 panic!(
-                                    "no non-ZST fields in dyn receiver type {}",
+                                    "no non-1-ZST fields in dyn receiver type {}",
                                     receiver.layout.ty
                                 )
                             });

tests/ui/layout/debug.stderr

@@ -117,13 +117,13 @@ error: layout_of(S) = Layout {
            fields: Arbitrary {
                offsets: [
                    Size(0 bytes),
-                   Size(0 bytes),
+                   Size(8 bytes),
                    Size(4 bytes),
                ],
                memory_index: [
-                   1,
                    0,
                    2,
+                   1,
                ],
            },
            largest_niche: None,

tests/ui/layout/enum.rs

@@ -0,0 +1,18 @@
+// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
+//! Various enum layout tests.
+
+#![feature(rustc_attrs)]
+#![feature(never_type)]
+#![crate_type = "lib"]
+
+#[rustc_layout(align)]
+enum UninhabitedVariantAlign { //~ERROR: abi: Align(2 bytes)
+    A([u8; 32]),
+    B([u16; 0], !), // make sure alignment in uninhabited fields is respected
+}
+
+#[rustc_layout(size)]
+enum UninhabitedVariantSpace { //~ERROR: size: Size(16 bytes)
+    A,
+    B([u8; 15], !), // make sure there is space being reserved for this field.
+}

tests/ui/layout/enum.stderr

@@ -0,0 +1,14 @@
+error: align: AbiAndPrefAlign { abi: Align(2 bytes), pref: $PREF_ALIGN }
+  --> $DIR/enum.rs:9:1
+   |
+LL | enum UninhabitedVariantAlign {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: size: Size(16 bytes)
+  --> $DIR/enum.rs:15:1
+   |
+LL | enum UninhabitedVariantSpace {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+

tests/ui/layout/struct.rs

@@ -0,0 +1,12 @@
+// normalize-stderr-test "pref: Align\([1-8] bytes\)" -> "pref: $$PREF_ALIGN"
+//! Various struct layout tests.
+
+#![feature(rustc_attrs)]
+#![feature(never_type)]
+#![crate_type = "lib"]
+
+#[rustc_layout(abi)]
+struct AlignedZstPreventsScalar(i16, [i32; 0]); //~ERROR: abi: Aggregate
+
+#[rustc_layout(abi)]
+struct AlignedZstButStillScalar(i32, [i16; 0]); //~ERROR: abi: Scalar

tests/ui/layout/struct.stderr

@@ -0,0 +1,14 @@
+error: abi: Aggregate { sized: true }
+  --> $DIR/struct.rs:9:1
+   |
+LL | struct AlignedZstPreventsScalar(i16, [i32; 0]);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: abi: Scalar(Initialized { value: Int(I32, true), valid_range: 0..=4294967295 })
+  --> $DIR/struct.rs:12:1
+   |
+LL | struct AlignedZstButStillScalar(i32, [i16; 0]);
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+