Track exactness in NaiveLayout and use it for SizeSkeleton checks

Author: moulins

compiler/rustc_middle/src/ty/layout.rs

@@ -15,7 +15,7 @@ use rustc_target::abi::call::FnAbi;
 use rustc_target::abi::*;
 use rustc_target::spec::{abi::Abi as SpecAbi, HasTargetSpec, PanicStrategy, Target};
 
-use std::cmp;
+use std::cmp::{self, Ordering};
 use std::fmt;
 use std::num::NonZeroUsize;
 use std::ops::Bound;
@@ -313,7 +313,16 @@ impl<'tcx> SizeSkeleton<'tcx> {
     ) -> Result<SizeSkeleton<'tcx>, &'tcx LayoutError<'tcx>> {
         debug_assert!(!ty.has_non_region_infer());
 
-        // First try computing a static layout.
+        // First, try computing an exact naive layout (this covers simple types with generic
+        // references, where a full static layout would fail).
+        if let Ok(layout) = tcx.naive_layout_of(param_env.and(ty)) {
+            if layout.is_exact {
+                return Ok(SizeSkeleton::Known(layout.min_size));
+            }
+        }
+
+        // Second, try computing a full static layout (this covers cases when the naive layout
+        // wasn't smart enough, but cannot deal with generic references).
         let err = match tcx.layout_of(param_env.and(ty)) {
             Ok(layout) => {
                 return Ok(SizeSkeleton::Known(layout.size));
@@ -327,6 +336,7 @@ impl<'tcx> SizeSkeleton<'tcx> {
             ) => return Err(e),
         };
 
+        // Third, fall back to ad-hoc cases.
         match *ty.kind() {
             ty::Ref(_, pointee, _) | ty::RawPtr(ty::TypeAndMut { ty: pointee, .. }) => {
                 let non_zero = !ty.is_unsafe_ptr();
@@ -645,25 +655,36 @@ impl std::ops::DerefMut for TyAndNaiveLayout<'_> {
 pub struct NaiveLayout {
     pub min_size: Size,
     pub min_align: Align,
+    // If `true`, `min_size` and `min_align` are guaranteed to be exact.
+    pub is_exact: bool,
 }
 
 impl NaiveLayout {
-    pub const EMPTY: Self = Self { min_size: Size::ZERO, min_align: Align::ONE };
+    pub const UNKNOWN: Self = Self { min_size: Size::ZERO, min_align: Align::ONE, is_exact: false };
+    pub const EMPTY: Self = Self { min_size: Size::ZERO, min_align: Align::ONE, is_exact: true };
+
+    pub fn is_compatible_with(&self, layout: Layout<'_>) -> bool {
+        let cmp = |cmp: Ordering| match (cmp, self.is_exact) {
+            (Ordering::Less | Ordering::Equal, false) => true,
+            (Ordering::Equal, true) => true,
+            (_, _) => false,
+        };
 
-    pub fn is_underestimate_of(&self, layout: Layout<'_>) -> bool {
-        self.min_size <= layout.size() && self.min_align <= layout.align().abi
+        cmp(self.min_size.cmp(&layout.size())) && cmp(self.min_align.cmp(&layout.align().abi))
     }
 
     #[must_use]
-    pub fn pad_to_align(self) -> Self {
-        Self { min_size: self.min_size.align_to(self.min_align), min_align: self.min_align }
+    pub fn pad_to_align(mut self) -> Self {
+        self.min_size = self.min_size.align_to(self.min_align);
+        self
     }
 
     #[must_use]
     pub fn concat<C: HasDataLayout>(&self, other: &Self, cx: &C) -> Option<Self> {
         Some(Self {
             min_size: self.min_size.checked_add(other.min_size, cx)?,
             min_align: std::cmp::max(self.min_align, other.min_align),
+            is_exact: self.is_exact && other.is_exact,
         })
     }
 
@@ -672,6 +693,7 @@ impl NaiveLayout {
         Self {
             min_size: std::cmp::max(self.min_size, other.min_size),
             min_align: std::cmp::max(self.min_align, other.min_align),
+            is_exact: self.is_exact && other.is_exact,
         }
     }
 }

compiler/rustc_ty_utils/src/layout.rs

@@ -90,9 +90,9 @@ fn layout_of<'tcx>(
     let cx = LayoutCx { tcx, param_env };
     let layout = layout_of_uncached(&cx, ty)?;
 
-    if !naive.is_underestimate_of(layout) {
+    if !naive.is_compatible_with(layout) {
         bug!(
-            "the estimated naive layout is bigger than the actual layout:\n{:#?}\n{:#?}",
+            "the naive layout isn't compatible with the actual layout:\n{:#?}\n{:#?}",
             naive,
             layout,
         );
@@ -119,15 +119,23 @@ fn naive_layout_of_uncached<'tcx>(
     let tcx = cx.tcx;
     let dl = cx.data_layout();
 
-    let scalar =
-        |value: Primitive| NaiveLayout { min_size: value.size(dl), min_align: value.align(dl).abi };
+    let scalar = |value: Primitive| NaiveLayout {
+        min_size: value.size(dl),
+        min_align: value.align(dl).abi,
+        is_exact: true,
+    };
 
     let univariant = |fields: &mut dyn Iterator<Item = Ty<'tcx>>,
                       repr: &ReprOptions|
      -> Result<NaiveLayout, &'tcx LayoutError<'tcx>> {
+        if repr.pack.is_some() && repr.align.is_some() {
+            cx.tcx.sess.delay_span_bug(DUMMY_SP, "struct cannot be packed and aligned");
+            return Err(error(cx, LayoutError::Unknown(ty)));
+        }
+
         // For simplicity, ignore inter-field padding; this may underestimate the size.
         // FIXME(reference_niches): Be smarter and implement something closer to the real layout logic.
-        let mut layout = NaiveLayout::EMPTY;
+        let mut layout = NaiveLayout::UNKNOWN;
         for field in fields {
             let field = cx.naive_layout_of(field)?;
             layout = layout
@@ -192,20 +200,22 @@ fn naive_layout_of_uncached<'tcx>(
                     .min_size
                     .checked_mul(count, cx)
                     .ok_or_else(|| error(cx, LayoutError::SizeOverflow(ty)))?,
-                min_align: element.min_align,
+                ..*element
             }
         }
-        ty::Slice(element) => {
-            NaiveLayout { min_size: Size::ZERO, min_align: cx.naive_layout_of(element)?.min_align }
-        }
+        ty::Slice(element) => NaiveLayout {
+            min_size: Size::ZERO,
+            // NOTE: this could be unconditionally exact if `NaiveLayout` guaranteed exact align.
+            ..*cx.naive_layout_of(element)?
+        },
         ty::Str => NaiveLayout::EMPTY,
 
         // Odd unit types.
         ty::FnDef(..) | ty::Dynamic(_, _, ty::Dyn) | ty::Foreign(..) => NaiveLayout::EMPTY,
 
         // FIXME(reference_niches): try to actually compute a reasonable layout estimate,
         // without duplicating too much code from `generator_layout`.
-        ty::Generator(..) => NaiveLayout::EMPTY,
+        ty::Generator(..) => NaiveLayout::UNKNOWN,
 
         ty::Closure(_, ref substs) => {
             univariant(&mut substs.as_closure().upvar_tys(), &ReprOptions::default())?
@@ -223,12 +233,21 @@ fn naive_layout_of_uncached<'tcx>(
         }
 
         ty::Adt(def, substs) => {
-            // For simplicity, assume that any discriminant field (if it exists)
-            // gets niched inside one of the variants; this will underestimate the size
-            // (and sometimes alignment) of enums.
-            // FIXME(reference_niches): Be smarter and actually take into accoount the discriminant.
             let repr = def.repr();
-            def.variants().iter().try_fold(NaiveLayout::EMPTY, |layout, v| {
+            let base = if def.is_struct() && !repr.simd() {
+                // FIXME(reference_niches): compute proper alignment for SIMD types.
+                NaiveLayout::EMPTY
+            } else {
+                // For simplicity, assume that any discriminant field (if it exists)
+                // gets niched inside one of the variants; this will underestimate the size
+                // (and sometimes alignment) of enums.
+                // FIXME(reference_niches): Be smarter and actually take into accoount the discriminant.
+                // Also consider adding a special case for null-optimized enums, so that we can have
+                // `Option<&T>: PointerLike` in generic contexts.
+                NaiveLayout::UNKNOWN
+            };
+
+            def.variants().iter().try_fold(base, |layout, v| {
                 let mut fields = v.fields.iter().map(|f| f.ty(tcx, substs));
                 let vlayout = univariant(&mut fields, &repr)?;
                 Ok(layout.union(&vlayout))
@@ -260,12 +279,10 @@ fn univariant_uninterned<'tcx>(
     kind: StructKind,
 ) -> Result<LayoutS, &'tcx LayoutError<'tcx>> {
     let dl = cx.data_layout();
-    let pack = repr.pack;
-    if pack.is_some() && repr.align.is_some() {
-        cx.tcx.sess.delay_span_bug(DUMMY_SP, "struct cannot be packed and aligned");
-        return Err(cx.tcx.arena.alloc(LayoutError::Unknown(ty)));
-    }
-
+    assert!(
+        !(repr.pack.is_some() && repr.align.is_some()),
+        "already rejected by `naive_layout_of`"
+    );
     cx.univariant(dl, fields, repr, kind).ok_or_else(|| error(cx, LayoutError::SizeOverflow(ty)))
 }
 
@@ -325,17 +342,7 @@ fn layout_of_uncached<'tcx>(
             if !ty.is_unsafe_ptr() {
                 // Calling `layout_of` here would cause a query cycle for recursive types;
                 // so use a conservative estimate that doesn't look past references.
-                let naive = match cx.naive_layout_of(pointee) {
-                    Ok(n) => n.layout,
-                    // This can happen when computing the `SizeSkeleton` of a generic type.
-                    Err(LayoutError::Unknown(_)) => {
-                        // TODO(reference_niches): this is *very* incorrect, but we can't
-                        // return an error here; this would break transmute checks.
-                        // We need some other solution.
-                        NaiveLayout::EMPTY
-                    }
-                    Err(err) => return Err(err),
-                };
+                let naive = cx.naive_layout_of(pointee)?.layout;
 
                 let niches = match *pointee.kind() {
                     ty::FnDef(def, ..)

tests/ui/lint/invalid_value.stderr

@@ -34,8 +34,7 @@ LL |         let _val: Wrap<&'static T> = mem::zeroed();
    |                                      this code causes undefined behavior when executed
    |                                      help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
    |
-   = note: `Wrap<&T>` must be non-null
-note: because references must be non-null (in this struct field)
+note: references must be non-null (in this struct field)
   --> $DIR/invalid_value.rs:17:18
    |
 LL | struct Wrap<T> { wrapped: T }
@@ -50,8 +49,7 @@ LL |         let _val: Wrap<&'static T> = mem::uninitialized();
    |                                      this code causes undefined behavior when executed
    |                                      help: use `MaybeUninit<T>` instead, and only call `assume_init` after initialization is done
    |
-   = note: `Wrap<&T>` must be non-null
-note: because references must be non-null (in this struct field)
+note: references must be non-null (in this struct field)
   --> $DIR/invalid_value.rs:17:18
    |
 LL | struct Wrap<T> { wrapped: T }

tests/ui/type-alias-impl-trait/issue-53092-2.stderr

@@ -9,7 +9,6 @@ note: ...which requires type-checking `CONST_BUG`...
    |
 LL | const CONST_BUG: Bug<u8, ()> = unsafe { std::mem::transmute(|_: u8| ()) };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   = note: ...which requires computing layout of `Bug<u8, ()>`...
    = note: ...which requires computing layout (naive) of `Bug<u8, ()>`...
    = note: ...which requires normalizing `Bug<u8, ()>`...
    = note: ...which again requires computing type of `Bug::{opaque#0}`, completing the cycle