Skip to content

Commit

Permalink
compiler: Make partially_check_layout fallible
Browse files Browse the repository at this point in the history
We already propagate LayoutErrors in some cases,
so double down on that.
  • Loading branch information
workingjubilee committed Oct 18, 2024
1 parent d8b6aa7 commit 5a9c4dd
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 18 deletions.
8 changes: 5 additions & 3 deletions compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,11 @@ fn layout_of<'tcx>(
record_layout_for_printing(&cx, layout);
}

invariant::partially_check_layout(&cx, &layout);

Ok(layout)
if let Err(err) = invariant::partially_check_layout(&cx, &layout) {
Err(map_error(&cx, ty, err))
} else {
Ok(layout)
}
}

fn error<'tcx>(cx: &LayoutCx<'tcx>, err: LayoutError<'tcx>) -> &'tcx LayoutError<'tcx> {
Expand Down
40 changes: 25 additions & 15 deletions compiler/rustc_ty_utils/src/layout/invariant.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
use std::assert_matches::assert_matches;

use rustc_abi::LayoutCalculatorError;
use rustc_middle::bug;
use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, TyAndLayout};
use rustc_target::abi::*;

/// Enforce some basic invariants on layouts.
pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayout<'tcx>) {
///
/// Note most of the code is skipped if debug assertions are not enabled,
/// and we bail left and right, thus the name of "partially check".
pub(super) fn partially_check_layout<'tcx>(
cx: &LayoutCx<'tcx>,
layout: &TyAndLayout<'tcx>,
) -> Result<(), LayoutCalculatorError<TyAndLayout<'tcx>>> {
let tcx = cx.tcx();

// Type-level uninhabitedness should always imply ABI uninhabitedness.
Expand All @@ -22,7 +29,7 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa

if !cfg!(debug_assertions) {
// Stop here, the rest is kind of expensive.
return;
return Ok(());
}

/// Yields non-ZST fields of the type
Expand Down Expand Up @@ -64,7 +71,10 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
*layout
}

fn check_layout_abi<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayout<'tcx>) {
fn check_layout_abi<'tcx>(
cx: &LayoutCx<'tcx>,
layout: &TyAndLayout<'tcx>,
) -> Result<(), LayoutCalculatorError<TyAndLayout<'tcx>>> {
// Verify the ABI mandated alignment and size.
let align = layout.abi.inherent_align(cx).map(|align| align.abi);
let size = layout.abi.inherent_size(cx);
Expand All @@ -74,21 +84,19 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
Abi::Uninhabited | Abi::Aggregate { .. },
"ABI unexpectedly missing alignment and/or size in {layout:#?}"
);
return;
return Ok(());
};
assert_eq!(
layout.layout.align().abi,
align,
"alignment mismatch between ABI and layout in {layout:#?}"
);
if layout.layout.align().abi != align {
return Err(LayoutCalculatorError::ReprConflict);
}
assert_eq!(
layout.layout.size(),
size,
"size mismatch between ABI and layout in {layout:#?}"
);

// Verify per-ABI invariants
match layout.layout.abi() {
let _unit = match layout.layout.abi() {
Abi::Scalar(_) => {
// Check that this matches the underlying field.
let inner = skip_newtypes(cx, layout);
Expand All @@ -104,7 +112,7 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
}
FieldsShape::Union(..) => {
// FIXME: I guess we could also check something here? Like, look at all fields?
return;
return Ok(());
}
FieldsShape::Arbitrary { .. } => {
// Should be an enum, the only field is the discriminant.
Expand Down Expand Up @@ -153,15 +161,15 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
if matches!(inner.layout.variants(), Variants::Multiple { .. }) {
// FIXME: ScalarPair for enums is enormously complicated and it is very hard
// to check anything about them.
return;
return Ok(());
}
match inner.layout.fields() {
FieldsShape::Arbitrary { .. } => {
// Checked below.
}
FieldsShape::Union(..) => {
// FIXME: I guess we could also check something here? Like, look at all fields?
return;
return Ok(());
}
_ => {
panic!("`ScalarPair` layout with unexpected field shape in {inner:#?}");
Expand Down Expand Up @@ -236,10 +244,11 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
// FIXME: Do some kind of check of the inner type, like for Scalar and ScalarPair.
}
Abi::Uninhabited | Abi::Aggregate { .. } => {} // Nothing to check.
}
};
Ok(_unit)
}

check_layout_abi(cx, layout);
let _unit = check_layout_abi(cx, layout)?;

if let Variants::Multiple { variants, .. } = &layout.variants {
for variant in variants.iter() {
Expand Down Expand Up @@ -293,4 +302,5 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
}
}
}
Ok(())
}

0 comments on commit 5a9c4dd

Please sign in to comment.