Skip to content

Commit

Permalink
Rollup merge of #105623 - compiler-errors:generator-type-size-fix, r=…
Browse files Browse the repository at this point in the history
…Nilstrieb

Fix `-Z print-type-sizes` for generators with discriminant field ordered first

Fixes #105589
Fixes #105591
  • Loading branch information
matthiaskrgr authored Dec 15, 2022
2 parents c00eac3 + bdc3c4b commit 5d24760
Show file tree
Hide file tree
Showing 17 changed files with 89 additions and 129 deletions.
30 changes: 23 additions & 7 deletions compiler/rustc_ty_utils/src/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ fn variant_info_for_generator<'tcx>(
def_id: DefId,
substs: ty::SubstsRef<'tcx>,
) -> (Vec<VariantInfo>, Option<Size>) {
let Variants::Multiple { tag, ref tag_encoding, .. } = layout.variants else {
let Variants::Multiple { tag, ref tag_encoding, tag_field, .. } = layout.variants else {
return (vec![], None);
};

Expand Down Expand Up @@ -975,12 +975,28 @@ fn variant_info_for_generator<'tcx>(
if variant_size == Size::ZERO {
variant_size = upvars_size;
}
// We need to add the discriminant size back into min_size, since it is subtracted
// later during printing.
variant_size += match tag_encoding {
TagEncoding::Direct => tag.size(cx),
_ => Size::ZERO,
};

// This `if` deserves some explanation.
//
// The layout code has a choice of where to place the discriminant of this generator.
// If the discriminant of the generator is placed early in the layout (before the
// variant's own fields), then it'll implicitly be counted towards the size of the
// variant, since we use the maximum offset to calculate size.
// (side-note: I know this is a bit problematic given upvars placement, etc).
//
// This is important, since the layout printing code always subtracts this discriminant
// size from the variant size if the struct is "enum"-like, so failing to account for it
// will either lead to numerical underflow, or an underreported variant size...
//
// However, if the discriminant is placed past the end of the variant, then we need
// to factor in the size of the discriminant manually. This really should be refactored
// better, but this "works" for now.
if layout.fields.offset(tag_field) >= variant_size {
variant_size += match tag_encoding {
TagEncoding::Direct => tag.size(cx),
_ => Size::ZERO,
};
}

VariantInfo {
name: Some(Symbol::intern(&ty::GeneratorSubsts::variant_name(variant_idx))),
Expand Down
12 changes: 2 additions & 10 deletions src/test/ui/print_type_sizes/async.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,11 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type lib
// edition:2021
// build-pass
// ignore-pass

#![feature(start)]

async fn wait() {}

async fn test(arg: [u8; 8192]) {
pub async fn test(arg: [u8; 8192]) {
wait().await;
drop(arg);
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
let _ = test([0; 8192]);
0
}
8 changes: 4 additions & 4 deletions src/test/ui/print_type_sizes/async.stdout
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
print-type-size type: `[async fn body@$DIR/async.rs:10:32: 13:2]`: 16386 bytes, alignment: 1 bytes
print-type-size type: `[async fn body@$DIR/async.rs:8:36: 11:2]`: 16386 bytes, alignment: 1 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Suspend0`: 16385 bytes
print-type-size field `.arg`: 8192 bytes, offset: 0 bytes, alignment: 1 bytes
Expand All @@ -16,14 +16,14 @@ print-type-size type: `std::mem::MaybeUninit<[u8; 8192]>`: 8192 bytes, alignment
print-type-size variant `MaybeUninit`: 8192 bytes
print-type-size field `.uninit`: 0 bytes
print-type-size field `.value`: 8192 bytes
print-type-size type: `[async fn body@$DIR/async.rs:8:17: 8:19]`: 1 bytes, alignment: 1 bytes
print-type-size type: `[async fn body@$DIR/async.rs:6:17: 6:19]`: 1 bytes, alignment: 1 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Unresumed`: 0 bytes
print-type-size variant `Returned`: 0 bytes
print-type-size variant `Panicked`: 0 bytes
print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/async.rs:8:17: 8:19]>`: 1 bytes, alignment: 1 bytes
print-type-size type: `std::mem::ManuallyDrop<[async fn body@$DIR/async.rs:6:17: 6:19]>`: 1 bytes, alignment: 1 bytes
print-type-size field `.value`: 1 bytes
print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/async.rs:8:17: 8:19]>`: 1 bytes, alignment: 1 bytes
print-type-size type: `std::mem::MaybeUninit<[async fn body@$DIR/async.rs:6:17: 6:19]>`: 1 bytes, alignment: 1 bytes
print-type-size variant `MaybeUninit`: 1 bytes
print-type-size field `.uninit`: 0 bytes
print-type-size field `.value`: 1 bytes
Expand Down
8 changes: 3 additions & 5 deletions src/test/ui/print_type_sizes/generator.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass
// ignore-pass

#![feature(start, generators, generator_trait)]
#![feature(generators, generator_trait)]

use std::ops::Generator;

Expand All @@ -13,8 +13,6 @@ fn generator<const C: usize>(array: [u8; C]) -> impl Generator<Yield = (), Retur
}
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
pub fn foo() {
let _ = generator([0; 8192]);
0
}
23 changes: 23 additions & 0 deletions src/test/ui/print_type_sizes/generator_discr_placement.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// compile-flags: -Z print-type-sizes --crate-type lib
// build-pass
// ignore-pass

// Tests a generator that has its discriminant as the *final* field.

// Avoid emitting panic handlers, like the rest of these tests...
#![feature(generators)]

pub fn foo() {
let a = || {
{
let w: i32 = 4;
yield;
drop(w);
}
{
let z: i32 = 7;
yield;
drop(z);
}
};
}
11 changes: 11 additions & 0 deletions src/test/ui/print_type_sizes/generator_discr_placement.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
print-type-size type: `[generator@$DIR/generator_discr_placement.rs:11:13: 11:15]`: 8 bytes, alignment: 4 bytes
print-type-size discriminant: 1 bytes
print-type-size variant `Suspend0`: 7 bytes
print-type-size padding: 3 bytes
print-type-size field `.w`: 4 bytes, alignment: 4 bytes
print-type-size variant `Suspend1`: 7 bytes
print-type-size padding: 3 bytes
print-type-size field `.z`: 4 bytes, alignment: 4 bytes
print-type-size variant `Unresumed`: 0 bytes
print-type-size variant `Returned`: 0 bytes
print-type-size variant `Panicked`: 0 bytes
24 changes: 2 additions & 22 deletions src/test/ui/print_type_sizes/generics.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass
// ignore-pass
// ^-- needed because `--pass check` does not emit the output needed.
Expand All @@ -8,24 +8,6 @@
// monomorphized, in the MIR of the original function in which they
// occur, to have their size reported.

#![feature(start)]

// In an ad-hoc attempt to avoid the injection of unwinding code
// (which clutters the output of `-Z print-type-sizes` with types from
// `unwind::libunwind`):
//
// * I am not using Default to build values because that seems to
// cause the injection of unwinding code. (Instead I just make `fn new`
// methods.)
//
// * Pair derive Copy to ensure that we don't inject
// unwinding code into generic uses of Pair when T itself is also
// Copy.
//
// (I suspect this reflect some naivety within the rust compiler
// itself; it should be checking for drop glue, i.e., a destructor
// somewhere in the monomorphized types. It should not matter whether
// the type is Copy.)
#[derive(Copy, Clone)]
pub struct Pair<T> {
_car: T,
Expand Down Expand Up @@ -61,11 +43,9 @@ pub fn f1<T:Copy>(x: T) {
Pair::new(FiftyBytes::new(), FiftyBytes::new());
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
pub fn start() {
let _b: Pair<u8> = Pair::new(0, 0);
let _s: Pair<SevenBytes> = Pair::new(SevenBytes::new(), SevenBytes::new());
let ref _z: ZeroSized = ZeroSized;
f1::<SevenBytes>(SevenBytes::new());
0
}
12 changes: 1 addition & 11 deletions src/test/ui/print_type_sizes/multiple_types.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,13 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass

// This file illustrates that when multiple structural types occur in
// a function, every one of them is included in the output.

#![feature(start)]

pub struct SevenBytes([u8; 7]);
pub struct FiftyBytes([u8; 50]);

pub enum Enum {
Small(SevenBytes),
Large(FiftyBytes),
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
let _e: Enum;
let _f: FiftyBytes;
let _s: SevenBytes;
0
}
10 changes: 3 additions & 7 deletions src/test/ui/print_type_sizes/niche-filling.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass
// ignore-pass
// ^-- needed because `--pass check` does not emit the output needed.
Expand All @@ -14,7 +14,6 @@
// aligned (while on most it is 8-byte aligned) and so the resulting
// padding and overall computed sizes can be quite different.

#![feature(start)]
#![feature(rustc_attrs)]
#![allow(dead_code)]

Expand Down Expand Up @@ -56,7 +55,7 @@ pub struct NestedNonZero {

impl Default for NestedNonZero {
fn default() -> Self {
NestedNonZero { pre: 0, val: NonZeroU32::new(1).unwrap(), post: 0 }
NestedNonZero { pre: 0, val: unsafe { NonZeroU32::new_unchecked(1) }, post: 0 }
}
}

Expand All @@ -76,8 +75,7 @@ pub union Union2<A: Copy, B: Copy> {
b: B,
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
pub fn test() {
let _x: MyOption<NonZeroU32> = Default::default();
let _y: EmbeddedDiscr = Default::default();
let _z: MyOption<IndirectNonZero> = Default::default();
Expand All @@ -96,6 +94,4 @@ fn start(_: isize, _: *const *const u8) -> isize {
// ...even when theoretically possible.
let _j: MyOption<Union1<NonZeroU32>> = Default::default();
let _k: MyOption<Union2<NonZeroU32, NonZeroU32>> = Default::default();

0
}
8 changes: 2 additions & 6 deletions src/test/ui/print_type_sizes/no_duplicates.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass
// ignore-pass
// ^-- needed because `--pass check` does not emit the output needed.
Expand All @@ -8,16 +8,12 @@
// (even if multiple functions), it is only printed once in the
// print-type-sizes output.

#![feature(start)]

pub struct SevenBytes([u8; 7]);

pub fn f1() {
let _s: SevenBytes = SevenBytes([0; 7]);
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
pub fn test() {
let _s: SevenBytes = SevenBytes([0; 7]);
0
}
20 changes: 5 additions & 15 deletions src/test/ui/print_type_sizes/packed.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass
// ignore-pass
// ^-- needed because `--pass check` does not emit the output needed.
Expand All @@ -13,11 +13,10 @@
// padding and overall computed sizes can be quite different.

#![allow(dead_code)]
#![feature(start)]

#[derive(Default)]
#[repr(packed)]
struct Packed1 {
pub struct Packed1 {
a: u8,
b: u8,
g: i32,
Expand All @@ -28,7 +27,7 @@ struct Packed1 {

#[derive(Default)]
#[repr(packed(2))]
struct Packed2 {
pub struct Packed2 {
a: u8,
b: u8,
g: i32,
Expand All @@ -40,7 +39,7 @@ struct Packed2 {
#[derive(Default)]
#[repr(packed(2))]
#[repr(C)]
struct Packed2C {
pub struct Packed2C {
a: u8,
b: u8,
g: i32,
Expand All @@ -50,20 +49,11 @@ struct Packed2C {
}

#[derive(Default)]
struct Padded {
pub struct Padded {
a: u8,
b: u8,
g: i32,
c: u8,
h: i16,
d: u8,
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
let _c: Packed1 = Default::default();
let _d: Packed2 = Default::default();
let _e: Packed2C = Default::default();
let _f: Padded = Default::default();
0
}
8 changes: 1 addition & 7 deletions src/test/ui/print_type_sizes/padding.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass

// This file illustrates how padding is handled: alignment
Expand All @@ -9,7 +9,6 @@
// aligned (while on most it is 8-byte aligned) and so the resulting
// padding and overall computed sizes can be quite different.

#![feature(start)]
#![allow(dead_code)]

struct S {
Expand All @@ -27,8 +26,3 @@ enum E2 {
A(i8, i32),
B(S),
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
0
}
12 changes: 3 additions & 9 deletions src/test/ui/print_type_sizes/repr-align.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// compile-flags: -Z print-type-sizes
// compile-flags: -Z print-type-sizes --crate-type=lib
// build-pass
// ignore-pass
// ^-- needed because `--pass check` does not emit the output needed.
Expand All @@ -11,7 +11,7 @@
// It avoids using u64/i64 because on some targets that is only 4-byte
// aligned (while on most it is 8-byte aligned) and so the resulting
// padding and overall computed sizes can be quite different.
#![feature(start)]

#![allow(dead_code)]

#[repr(align(16))]
Expand All @@ -24,15 +24,9 @@ enum E {
}

#[derive(Default)]
struct S {
pub struct S {
a: i32,
b: i32,
c: A,
d: i8,
}

#[start]
fn start(_: isize, _: *const *const u8) -> isize {
let _s: S = Default::default();
0
}
Loading

0 comments on commit 5d24760

Please sign in to comment.