From 48d1be4f46225350bb59f87a46bf2f6cba041013 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Mon, 6 May 2019 17:06:59 -0400 Subject: [PATCH 1/4] Test interaction of unions with non-zero/niche-filling optimization Notably this nails down part of the behavior that MaybeUninit assumes, e.g. that a Option> does not take advantage of non-zero optimization, and thus is a safe construct. --- src/test/run-pass/union/union-nonzero.rs | 30 +++++++++++++++++++ src/test/ui/print_type_sizes/niche-filling.rs | 17 +++++++++++ .../ui/print_type_sizes/niche-filling.stdout | 26 ++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 src/test/run-pass/union/union-nonzero.rs diff --git a/src/test/run-pass/union/union-nonzero.rs b/src/test/run-pass/union/union-nonzero.rs new file mode 100644 index 0000000000000..614aa9ec0ce17 --- /dev/null +++ b/src/test/run-pass/union/union-nonzero.rs @@ -0,0 +1,30 @@ +// run-pass +#![allow(dead_code)] + +use std::mem::{size_of, transmute}; + +union U1 { + a: A, +} + +union U2 { + a: A, + b: B, +} + +fn main() { + // Unions do not participate in niche-filling/non-zero optimization... + assert!(size_of::>>() > size_of::>()); + assert!(size_of::>>() > size_of::>()); + + // ...even when theoretically possible: + assert!(size_of::>>() > size_of::>()); + assert!(size_of::>>() > size_of::>()); + + // The unused bits of the () variant can have any value. + let zeroed: U2<&u8, ()> = unsafe { transmute(std::ptr::null::()) }; + + if let None = Some(zeroed) { + panic!() + } +} diff --git a/src/test/ui/print_type_sizes/niche-filling.rs b/src/test/ui/print_type_sizes/niche-filling.rs index bed1e32a60158..0127261b2b7d0 100644 --- a/src/test/ui/print_type_sizes/niche-filling.rs +++ b/src/test/ui/print_type_sizes/niche-filling.rs @@ -57,6 +57,15 @@ pub enum Enum4 { Four(D) } +pub union Union1 { + a: A, +} + +pub union Union2 { + a: A, + b: B, +} + #[start] fn start(_: isize, _: *const *const u8) -> isize { let _x: MyOption = Default::default(); @@ -69,5 +78,13 @@ fn start(_: isize, _: *const *const u8) -> isize { let _e: Enum4<(), char, (), ()> = Enum4::One(()); let _f: Enum4<(), (), bool, ()> = Enum4::One(()); let _g: Enum4<(), (), (), MyOption> = Enum4::One(()); + + // Unions do not currently participate in niche filling. + let _h: MyOption> = Default::default(); + + // ...even when theoretically possible. + let _i: MyOption> = Default::default(); + let _j: MyOption> = Default::default(); + 0 } diff --git a/src/test/ui/print_type_sizes/niche-filling.stdout b/src/test/ui/print_type_sizes/niche-filling.stdout index 9cdb2ae4f57e0..301edc0d086b1 100644 --- a/src/test/ui/print_type_sizes/niche-filling.stdout +++ b/src/test/ui/print_type_sizes/niche-filling.stdout @@ -14,6 +14,21 @@ print-type-size field `.post`: 2 bytes print-type-size field `.pre`: 1 bytes print-type-size variant `None`: 0 bytes print-type-size end padding: 1 bytes +print-type-size type: `MyOption>`: 8 bytes, alignment: 4 bytes +print-type-size discriminant: 4 bytes +print-type-size variant `Some`: 4 bytes +print-type-size field `.0`: 4 bytes +print-type-size variant `None`: 0 bytes +print-type-size type: `MyOption>`: 8 bytes, alignment: 4 bytes +print-type-size discriminant: 4 bytes +print-type-size variant `Some`: 4 bytes +print-type-size field `.0`: 4 bytes +print-type-size variant `None`: 0 bytes +print-type-size type: `MyOption>`: 8 bytes, alignment: 4 bytes +print-type-size discriminant: 4 bytes +print-type-size variant `Some`: 4 bytes +print-type-size field `.0`: 4 bytes +print-type-size variant `None`: 0 bytes print-type-size type: `NestedNonZero`: 8 bytes, alignment: 4 bytes print-type-size field `.val`: 4 bytes print-type-size field `.post`: 2 bytes @@ -36,6 +51,17 @@ print-type-size type: `MyOption`: 4 bytes, alignment: 4 by print-type-size variant `Some`: 4 bytes print-type-size field `.0`: 4 bytes print-type-size variant `None`: 0 bytes +print-type-size type: `Union1`: 4 bytes, alignment: 4 bytes +print-type-size variant `Union1`: 4 bytes +print-type-size field `.a`: 4 bytes +print-type-size type: `Union2`: 4 bytes, alignment: 4 bytes +print-type-size variant `Union2`: 4 bytes +print-type-size field `.a`: 4 bytes +print-type-size field `.b`: 4 bytes, offset: 0 bytes, alignment: 4 bytes +print-type-size type: `Union2`: 4 bytes, alignment: 4 bytes +print-type-size variant `Union2`: 4 bytes +print-type-size field `.a`: 4 bytes +print-type-size field `.b`: 4 bytes, offset: 0 bytes, alignment: 4 bytes print-type-size type: `std::num::NonZeroU32`: 4 bytes, alignment: 4 bytes print-type-size field `.0`: 4 bytes print-type-size type: `Enum4<(), (), (), MyOption>`: 2 bytes, alignment: 1 bytes From 76c52290214c49a9ba0cf506ac7e636543e087ff Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 7 May 2019 12:16:33 -0400 Subject: [PATCH 2/4] Document purpose of union-nonzero test --- src/test/run-pass/union/union-nonzero.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/test/run-pass/union/union-nonzero.rs b/src/test/run-pass/union/union-nonzero.rs index 614aa9ec0ce17..07a58aa7746d1 100644 --- a/src/test/run-pass/union/union-nonzero.rs +++ b/src/test/run-pass/union/union-nonzero.rs @@ -1,6 +1,18 @@ // run-pass #![allow(dead_code)] +// Tests that unions aren't subject to unsafe non-zero/niche-filling optimizations. +// +// For example, if a union `U` can contain both a `&T` and a `*const T`, there's definitely no +// bit-value that an `Option` could reuse as `None`; this test makes sure that isn't done. +// +// Secondly, this tests the status quo to not apply such optimizations to types containing unions +// even if they're theoretically possible. (discussion: https://github.com/rust-lang/rust/issues/36394) +// +// Notably this nails down part of the behavior that `MaybeUninit` assumes: that a +// `Option>` does not take advantage of non-zero optimization, and thus is a safe +// construct. + use std::mem::{size_of, transmute}; union U1 { From aa1db2476a7b6e3155bf40cc48f6bc897575c622 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Tue, 7 May 2019 12:17:28 -0400 Subject: [PATCH 3/4] Add non-non-zero test to union-nonzero test --- src/test/run-pass/union/union-nonzero.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/test/run-pass/union/union-nonzero.rs b/src/test/run-pass/union/union-nonzero.rs index 07a58aa7746d1..2108cc3a4ea78 100644 --- a/src/test/run-pass/union/union-nonzero.rs +++ b/src/test/run-pass/union/union-nonzero.rs @@ -24,10 +24,18 @@ union U2 { b: B, } +// Option uses a value other than 0 and 1 as None +#[derive(Clone,Copy)] +enum E { + A = 0, + B = 1, +} + fn main() { // Unions do not participate in niche-filling/non-zero optimization... assert!(size_of::>>() > size_of::>()); assert!(size_of::>>() > size_of::>()); + assert!(size_of::>>() > size_of::>()); // ...even when theoretically possible: assert!(size_of::>>() > size_of::>()); From a91ad60158647c1f6a89b9c01915279ce9314a65 Mon Sep 17 00:00:00 2001 From: Peter Todd Date: Sat, 18 May 2019 22:27:33 -0400 Subject: [PATCH 4/4] =?UTF-8?q?Make=20clear=20that=20status=20quo=20?= =?UTF-8?q?=E2=89=A0=20guarantee?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/test/run-pass/union/union-nonzero.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/run-pass/union/union-nonzero.rs b/src/test/run-pass/union/union-nonzero.rs index 2108cc3a4ea78..bd84b46bf3d23 100644 --- a/src/test/run-pass/union/union-nonzero.rs +++ b/src/test/run-pass/union/union-nonzero.rs @@ -6,8 +6,9 @@ // For example, if a union `U` can contain both a `&T` and a `*const T`, there's definitely no // bit-value that an `Option` could reuse as `None`; this test makes sure that isn't done. // -// Secondly, this tests the status quo to not apply such optimizations to types containing unions -// even if they're theoretically possible. (discussion: https://github.com/rust-lang/rust/issues/36394) +// Secondly, this tests the status quo (not a guarantee; subject to change!) to not apply such +// optimizations to types containing unions even if they're theoretically possible. (discussion: +// https://github.com/rust-lang/rust/issues/36394) // // Notably this nails down part of the behavior that `MaybeUninit` assumes: that a // `Option>` does not take advantage of non-zero optimization, and thus is a safe