Skip to content

Commit 5a9c4dd

Browse files
compiler: Make partially_check_layout fallible
We already propagate LayoutErrors in some cases, so double down on that.
1 parent d8b6aa7 commit 5a9c4dd

File tree

2 files changed

+30
-18
lines changed

2 files changed

+30
-18
lines changed

Diff for: compiler/rustc_ty_utils/src/layout.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,11 @@ fn layout_of<'tcx>(
8080
record_layout_for_printing(&cx, layout);
8181
}
8282

83-
invariant::partially_check_layout(&cx, &layout);
84-
85-
Ok(layout)
83+
if let Err(err) = invariant::partially_check_layout(&cx, &layout) {
84+
Err(map_error(&cx, ty, err))
85+
} else {
86+
Ok(layout)
87+
}
8688
}
8789

8890
fn error<'tcx>(cx: &LayoutCx<'tcx>, err: LayoutError<'tcx>) -> &'tcx LayoutError<'tcx> {

Diff for: compiler/rustc_ty_utils/src/layout/invariant.rs

+25-15
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
use std::assert_matches::assert_matches;
22

3+
use rustc_abi::LayoutCalculatorError;
34
use rustc_middle::bug;
45
use rustc_middle::ty::layout::{HasTyCtxt, LayoutCx, TyAndLayout};
56
use rustc_target::abi::*;
67

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

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

2330
if !cfg!(debug_assertions) {
2431
// Stop here, the rest is kind of expensive.
25-
return;
32+
return Ok(());
2633
}
2734

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

67-
fn check_layout_abi<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLayout<'tcx>) {
74+
fn check_layout_abi<'tcx>(
75+
cx: &LayoutCx<'tcx>,
76+
layout: &TyAndLayout<'tcx>,
77+
) -> Result<(), LayoutCalculatorError<TyAndLayout<'tcx>>> {
6878
// Verify the ABI mandated alignment and size.
6979
let align = layout.abi.inherent_align(cx).map(|align| align.abi);
7080
let size = layout.abi.inherent_size(cx);
@@ -74,21 +84,19 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
7484
Abi::Uninhabited | Abi::Aggregate { .. },
7585
"ABI unexpectedly missing alignment and/or size in {layout:#?}"
7686
);
77-
return;
87+
return Ok(());
7888
};
79-
assert_eq!(
80-
layout.layout.align().abi,
81-
align,
82-
"alignment mismatch between ABI and layout in {layout:#?}"
83-
);
89+
if layout.layout.align().abi != align {
90+
return Err(LayoutCalculatorError::ReprConflict);
91+
}
8492
assert_eq!(
8593
layout.layout.size(),
8694
size,
8795
"size mismatch between ABI and layout in {layout:#?}"
8896
);
8997

9098
// Verify per-ABI invariants
91-
match layout.layout.abi() {
99+
let _unit = match layout.layout.abi() {
92100
Abi::Scalar(_) => {
93101
// Check that this matches the underlying field.
94102
let inner = skip_newtypes(cx, layout);
@@ -104,7 +112,7 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
104112
}
105113
FieldsShape::Union(..) => {
106114
// FIXME: I guess we could also check something here? Like, look at all fields?
107-
return;
115+
return Ok(());
108116
}
109117
FieldsShape::Arbitrary { .. } => {
110118
// Should be an enum, the only field is the discriminant.
@@ -153,15 +161,15 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
153161
if matches!(inner.layout.variants(), Variants::Multiple { .. }) {
154162
// FIXME: ScalarPair for enums is enormously complicated and it is very hard
155163
// to check anything about them.
156-
return;
164+
return Ok(());
157165
}
158166
match inner.layout.fields() {
159167
FieldsShape::Arbitrary { .. } => {
160168
// Checked below.
161169
}
162170
FieldsShape::Union(..) => {
163171
// FIXME: I guess we could also check something here? Like, look at all fields?
164-
return;
172+
return Ok(());
165173
}
166174
_ => {
167175
panic!("`ScalarPair` layout with unexpected field shape in {inner:#?}");
@@ -236,10 +244,11 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
236244
// FIXME: Do some kind of check of the inner type, like for Scalar and ScalarPair.
237245
}
238246
Abi::Uninhabited | Abi::Aggregate { .. } => {} // Nothing to check.
239-
}
247+
};
248+
Ok(_unit)
240249
}
241250

242-
check_layout_abi(cx, layout);
251+
let _unit = check_layout_abi(cx, layout)?;
243252

244253
if let Variants::Multiple { variants, .. } = &layout.variants {
245254
for variant in variants.iter() {
@@ -293,4 +302,5 @@ pub(super) fn partially_check_layout<'tcx>(cx: &LayoutCx<'tcx>, layout: &TyAndLa
293302
}
294303
}
295304
}
305+
Ok(())
296306
}

0 commit comments

Comments
 (0)