From 0a08841bb075959874e2e29c538150c826a1401a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 3 Jul 2019 12:23:26 +0200 Subject: [PATCH 01/20] Remove uses of `allow(unions_with_drop_fields)` in the standard library --- src/libstd/panicking.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 638ce1679b8e9..296f2a59bb82e 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -12,7 +12,7 @@ use core::panic::{BoxMeUp, PanicInfo, Location}; use crate::any::Any; use crate::fmt; use crate::intrinsics; -use crate::mem; +use crate::mem::{self, ManuallyDrop}; use crate::ptr; use crate::raw; use crate::sync::atomic::{AtomicBool, Ordering}; @@ -227,10 +227,9 @@ pub use realstd::rt::update_panic_count; /// Invoke a closure, capturing the cause of an unwinding panic if one occurs. pub unsafe fn r#try R>(f: F) -> Result> { - #[allow(unions_with_drop_fields)] union Data { - f: F, - r: R, + f: ManuallyDrop, + r: ManuallyDrop, } // We do some sketchy operations with ownership here for the sake of @@ -261,7 +260,7 @@ pub unsafe fn r#try R>(f: F) -> Result> let mut any_data = 0; let mut any_vtable = 0; let mut data = Data { - f, + f: ManuallyDrop::new(f) }; let r = __rust_maybe_catch_panic(do_call::, @@ -271,7 +270,7 @@ pub unsafe fn r#try R>(f: F) -> Result> return if r == 0 { debug_assert!(update_panic_count(0) == 0); - Ok(data.r) + Ok(ManuallyDrop::into_inner(data.r)) } else { update_panic_count(-1); debug_assert!(update_panic_count(0) == 0); @@ -284,8 +283,8 @@ pub unsafe fn r#try R>(f: F) -> Result> fn do_call R, R>(data: *mut u8) { unsafe { let data = data as *mut Data; - let f = ptr::read(&mut (*data).f); - ptr::write(&mut (*data).r, f()); + let f = ptr::read(&mut *(*data).f); + ptr::write(&mut *(*data).r, f()); } } } From 84ca0a1cb47f71a43ee16da2f6bc173577b35cb9 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 3 Jul 2019 12:35:02 +0200 Subject: [PATCH 02/20] Remove most uses of `allow(unions_with_drop_fields)` in tests --- .../ui/associated-type-bounds/union-bounds.rs | 15 ++--- src/test/ui/drop/dynamic-drop.rs | 12 ++-- .../rfc-2093-infer-outlives/explicit-union.rs | 5 +- .../rfc-2093-infer-outlives/nested-union.rs | 5 +- src/test/ui/self/self-in-typedefs.rs | 11 +++- .../union/union-borrow-move-parent-sibling.rs | 61 +++++++++++++++---- .../union-borrow-move-parent-sibling.stderr | 34 ++++------- src/test/ui/union/union-derive-rpass.rs | 5 +- src/test/ui/union/union-drop-assign.rs | 11 ++-- src/test/ui/union/union-drop.rs | 11 +--- src/test/ui/union/union-generic-rpass.rs | 12 +--- src/test/ui/union/union-nodrop.rs | 11 ++-- src/test/ui/union/union-overwrite.rs | 14 +++-- .../union-with-drop-fields-lint-rpass.rs | 32 ---------- 14 files changed, 118 insertions(+), 121 deletions(-) delete mode 100644 src/test/ui/union/union-with-drop-fields-lint-rpass.rs diff --git a/src/test/ui/associated-type-bounds/union-bounds.rs b/src/test/ui/associated-type-bounds/union-bounds.rs index ce482fff401c8..97c5acf1f72ca 100644 --- a/src/test/ui/associated-type-bounds/union-bounds.rs +++ b/src/test/ui/associated-type-bounds/union-bounds.rs @@ -3,13 +3,13 @@ #![feature(associated_type_bounds)] #![feature(untagged_unions)] -#![allow(unions_with_drop_fields, unused_assignments)] +#![allow(unused_assignments)] -trait Tr1 { type As1; } -trait Tr2 { type As2; } -trait Tr3 { type As3; } -trait Tr4<'a> { type As4; } -trait Tr5 { type As5; } +trait Tr1: Copy { type As1: Copy; } +trait Tr2: Copy { type As2: Copy; } +trait Tr3: Copy { type As3: Copy; } +trait Tr4<'a>: Copy { type As4: Copy; } +trait Tr5: Copy { type As5: Copy; } impl Tr1 for &str { type As1 = bool; } impl Tr2 for bool { type As2 = u8; } @@ -71,7 +71,8 @@ where let _: &'a T = &x.f0; } -union UnSelf where Self: Tr1 { +#[derive(Copy, Clone)] +union UnSelf where Self: Tr1, T: Copy { f0: T, f1: ::As1, f2: <::As1 as Tr2>::As2, diff --git a/src/test/ui/drop/dynamic-drop.rs b/src/test/ui/drop/dynamic-drop.rs index 8516bc3d96424..29dcbfe9609a0 100644 --- a/src/test/ui/drop/dynamic-drop.rs +++ b/src/test/ui/drop/dynamic-drop.rs @@ -8,6 +8,7 @@ #![feature(slice_patterns)] use std::cell::{Cell, RefCell}; +use std::mem::ManuallyDrop; use std::ops::Generator; use std::panic; use std::pin::Pin; @@ -152,17 +153,16 @@ fn assignment1(a: &Allocator, c0: bool) { _v = _w; } -#[allow(unions_with_drop_fields)] union Boxy { - a: T, - b: T, + a: ManuallyDrop, + b: ManuallyDrop, } fn union1(a: &Allocator) { unsafe { - let mut u = Boxy { a: a.alloc() }; - u.b = a.alloc(); - drop(u.a); + let mut u = Boxy { a: ManuallyDrop::new(a.alloc()) }; + *u.b = a.alloc(); // drops first alloc + drop(ManuallyDrop::into_inner(u.a)); } } diff --git a/src/test/ui/rfc-2093-infer-outlives/explicit-union.rs b/src/test/ui/rfc-2093-infer-outlives/explicit-union.rs index a16fb7670a603..ea8a3c177e9d7 100644 --- a/src/test/ui/rfc-2093-infer-outlives/explicit-union.rs +++ b/src/test/ui/rfc-2093-infer-outlives/explicit-union.rs @@ -1,13 +1,12 @@ #![feature(rustc_attrs)] #![feature(untagged_unions)] -#![allow(unions_with_drop_fields)] #[rustc_outlives] -union Foo<'b, U> { //~ ERROR rustc_outlives +union Foo<'b, U: Copy> { //~ ERROR rustc_outlives bar: Bar<'b, U> } -union Bar<'a, T> where T: 'a { +union Bar<'a, T: Copy> where T: 'a { x: &'a (), y: T, } diff --git a/src/test/ui/rfc-2093-infer-outlives/nested-union.rs b/src/test/ui/rfc-2093-infer-outlives/nested-union.rs index 0ff667fbeba64..0da3cc2ba1b04 100644 --- a/src/test/ui/rfc-2093-infer-outlives/nested-union.rs +++ b/src/test/ui/rfc-2093-infer-outlives/nested-union.rs @@ -1,14 +1,13 @@ #![feature(rustc_attrs)] #![feature(untagged_unions)] -#![allow(unions_with_drop_fields)] #[rustc_outlives] -union Foo<'a, T> { //~ ERROR rustc_outlives +union Foo<'a, T: Copy> { //~ ERROR rustc_outlives field1: Bar<'a, T> } // Type U needs to outlive lifetime 'b -union Bar<'b, U> { +union Bar<'b, U: Copy> { field2: &'b U } diff --git a/src/test/ui/self/self-in-typedefs.rs b/src/test/ui/self/self-in-typedefs.rs index 73f23a9cc17cf..3b1eb9e1dfa16 100644 --- a/src/test/ui/self/self-in-typedefs.rs +++ b/src/test/ui/self/self-in-typedefs.rs @@ -3,7 +3,8 @@ #![feature(untagged_unions)] #![allow(dead_code)] -#![allow(unions_with_drop_fields)] + +use std::mem::ManuallyDrop; enum A<'a, T: 'a> where @@ -24,6 +25,14 @@ where union C<'a, T: 'a> where Self: Send, T: PartialEq +{ + foo: &'a Self, + bar: ManuallyDrop, +} + +union D<'a, T: 'a> +where + Self: Send, T: PartialEq + Copy { foo: &'a Self, bar: T, diff --git a/src/test/ui/union/union-borrow-move-parent-sibling.rs b/src/test/ui/union/union-borrow-move-parent-sibling.rs index 1b6052f10ba4a..edf08e6ca678f 100644 --- a/src/test/ui/union/union-borrow-move-parent-sibling.rs +++ b/src/test/ui/union/union-borrow-move-parent-sibling.rs @@ -1,51 +1,90 @@ #![feature(untagged_unions)] #![allow(unused)] -#[allow(unions_with_drop_fields)] +use std::ops::{Deref, DerefMut}; + +#[derive(Default)] +struct MockBox { + value: [T; 1], +} + +impl MockBox { + fn new(value: T) -> Self { MockBox { value: [value] } } +} + +impl Deref for MockBox { + type Target = T; + fn deref(&self) -> &T { &self.value[0] } +} + +impl DerefMut for MockBox { + fn deref_mut(&mut self) -> &mut T { &mut self.value[0] } +} + +#[derive(Default)] +struct MockVec { + value: [T; 0], +} + +impl MockVec { + fn new() -> Self { MockVec { value: [] } } +} + +impl Deref for MockVec { + type Target = [T]; + fn deref(&self) -> &[T] { &self.value } +} + +impl DerefMut for MockVec { + fn deref_mut(&mut self) -> &mut [T] { &mut self.value } +} + + union U { - x: ((Vec, Vec), Vec), - y: Box>, + x: ((MockVec, MockVec), MockVec), + y: MockBox>, } fn use_borrow(_: &T) {} unsafe fn parent_sibling_borrow() { - let mut u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; + let mut u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; let a = &mut u.x.0; let b = &u.y; //~ ERROR cannot borrow `u` (via `u.y`) use_borrow(a); } unsafe fn parent_sibling_move() { - let u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; + let u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; let a = u.x.0; let b = u.y; //~ ERROR use of moved value: `u` } unsafe fn grandparent_sibling_borrow() { - let mut u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; + let mut u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; let a = &mut (u.x.0).0; let b = &u.y; //~ ERROR cannot borrow `u` (via `u.y`) use_borrow(a); } unsafe fn grandparent_sibling_move() { - let u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; + let u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; let a = (u.x.0).0; let b = u.y; //~ ERROR use of moved value: `u` } unsafe fn deref_sibling_borrow() { - let mut u = U { y: Box::default() }; + let mut u = U { y: MockBox::default() }; let a = &mut *u.y; let b = &u.x; //~ ERROR cannot borrow `u` (via `u.x`) use_borrow(a); } unsafe fn deref_sibling_move() { - let u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; - let a = *u.y; - let b = u.x; //~ ERROR use of moved value: `u` + let u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; + // No way to test deref-move without Box in union + // let a = *u.y; + // let b = u.x; ERROR use of moved value: `u` } diff --git a/src/test/ui/union/union-borrow-move-parent-sibling.stderr b/src/test/ui/union/union-borrow-move-parent-sibling.stderr index 2f4c921ea08f9..1a7a02690d3ea 100644 --- a/src/test/ui/union/union-borrow-move-parent-sibling.stderr +++ b/src/test/ui/union/union-borrow-move-parent-sibling.stderr @@ -1,5 +1,5 @@ error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borrowed as mutable (via `u.x.0`) - --> $DIR/union-borrow-move-parent-sibling.rs:15:13 + --> $DIR/union-borrow-move-parent-sibling.rs:54:13 | LL | let a = &mut u.x.0; | ---------- mutable borrow occurs here (via `u.x.0`) @@ -11,9 +11,9 @@ LL | use_borrow(a); = note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0` error[E0382]: use of moved value: `u` - --> $DIR/union-borrow-move-parent-sibling.rs:22:13 + --> $DIR/union-borrow-move-parent-sibling.rs:61:13 | -LL | let u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; +LL | let u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; | - move occurs because `u` has type `U`, which does not implement the `Copy` trait LL | let a = u.x.0; | ----- value moved here @@ -21,7 +21,7 @@ LL | let b = u.y; | ^^^ value used here after move error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borrowed as mutable (via `u.x.0.0`) - --> $DIR/union-borrow-move-parent-sibling.rs:28:13 + --> $DIR/union-borrow-move-parent-sibling.rs:67:13 | LL | let a = &mut (u.x.0).0; | -------------- mutable borrow occurs here (via `u.x.0.0`) @@ -33,38 +33,28 @@ LL | use_borrow(a); = note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0.0` error[E0382]: use of moved value: `u` - --> $DIR/union-borrow-move-parent-sibling.rs:35:13 + --> $DIR/union-borrow-move-parent-sibling.rs:74:13 | -LL | let u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; +LL | let u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; | - move occurs because `u` has type `U`, which does not implement the `Copy` trait LL | let a = (u.x.0).0; | --------- value moved here LL | let b = u.y; | ^^^ value used here after move -error[E0502]: cannot borrow `u` (via `u.x`) as immutable because it is also borrowed as mutable (via `*u.y`) - --> $DIR/union-borrow-move-parent-sibling.rs:41:13 +error[E0502]: cannot borrow `u` (via `u.x`) as immutable because it is also borrowed as mutable (via `u.y`) + --> $DIR/union-borrow-move-parent-sibling.rs:80:13 | LL | let a = &mut *u.y; - | --------- mutable borrow occurs here (via `*u.y`) + | --- mutable borrow occurs here (via `u.y`) LL | let b = &u.x; - | ^^^^ immutable borrow of `u.x` -- which overlaps with `*u.y` -- occurs here + | ^^^^ immutable borrow of `u.x` -- which overlaps with `u.y` -- occurs here LL | use_borrow(a); | - mutable borrow later used here | - = note: `u.x` is a field of the union `U`, so it overlaps the field `*u.y` + = note: `u.x` is a field of the union `U`, so it overlaps the field `u.y` -error[E0382]: use of moved value: `u` - --> $DIR/union-borrow-move-parent-sibling.rs:48:13 - | -LL | let u = U { x: ((Vec::new(), Vec::new()), Vec::new()) }; - | - move occurs because `u` has type `U`, which does not implement the `Copy` trait -LL | let a = *u.y; - | ---- value moved here -LL | let b = u.x; - | ^^^ value used here after move - -error: aborting due to 6 previous errors +error: aborting due to 5 previous errors Some errors have detailed explanations: E0382, E0502. For more information about an error, try `rustc --explain E0382`. diff --git a/src/test/ui/union/union-derive-rpass.rs b/src/test/ui/union/union-derive-rpass.rs index d4b82cdb250b1..b2f7ae679fd68 100644 --- a/src/test/ui/union/union-derive-rpass.rs +++ b/src/test/ui/union/union-derive-rpass.rs @@ -1,7 +1,6 @@ // run-pass #![allow(dead_code)] #![allow(unused_variables)] -#![allow(unions_with_drop_fields)] // Some traits can be derived for unions. @@ -24,11 +23,11 @@ impl PartialEq for U { fn eq(&self, rhs: &Self) -> bool { true } } Copy, Eq )] -union W { +union W { a: T, } -impl PartialEq for W { fn eq(&self, rhs: &Self) -> bool { true } } +impl PartialEq for W { fn eq(&self, rhs: &Self) -> bool { true } } fn main() { let u = U { b: 0 }; diff --git a/src/test/ui/union/union-drop-assign.rs b/src/test/ui/union/union-drop-assign.rs index c4349c454641b..f1511b0a60180 100644 --- a/src/test/ui/union/union-drop-assign.rs +++ b/src/test/ui/union/union-drop-assign.rs @@ -1,15 +1,16 @@ // run-pass #![allow(unused_assignments)] -#![allow(unions_with_drop_fields)] // Drop works for union itself. #![feature(untagged_unions)] +use std::mem::ManuallyDrop; + struct S; union U { - a: S + a: ManuallyDrop } impl Drop for S { @@ -28,11 +29,11 @@ static mut CHECK: u8 = 0; fn main() { unsafe { - let mut u = U { a: S }; + let mut u = U { a: ManuallyDrop::new(S) }; assert_eq!(CHECK, 0); - u = U { a: S }; + u = U { a: ManuallyDrop::new(S) }; assert_eq!(CHECK, 1); // union itself is assigned, union is dropped, field is not dropped - u.a = S; + *u.a = S; assert_eq!(CHECK, 11); // union field is assigned, field is dropped } } diff --git a/src/test/ui/union/union-drop.rs b/src/test/ui/union/union-drop.rs index 2060950dda951..daa03ce6b6fd8 100644 --- a/src/test/ui/union/union-drop.rs +++ b/src/test/ui/union/union-drop.rs @@ -1,7 +1,6 @@ // run-pass #![allow(dead_code)] #![allow(unused_variables)] -#![allow(unions_with_drop_fields)] // Drop works for union itself. @@ -21,12 +20,6 @@ union Y { a: S, } -impl Drop for S { - fn drop(&mut self) { - unsafe { CHECK += 10; } - } -} - impl Drop for U { fn drop(&mut self) { unsafe { CHECK += 1; } @@ -51,10 +44,10 @@ fn main() { { let w = W { a: S }; } - assert_eq!(CHECK, 2); // 2, not 11, dtor of S is not called + assert_eq!(CHECK, 2); // 2, dtor of W is called { let y = Y { a: S }; } - assert_eq!(CHECK, 2); // 2, not 12, dtor of S is not called + assert_eq!(CHECK, 2); // 2, dtor of Y is called } } diff --git a/src/test/ui/union/union-generic-rpass.rs b/src/test/ui/union/union-generic-rpass.rs index 6f2caf8dc5b1e..5ca3bc0f722c3 100644 --- a/src/test/ui/union/union-generic-rpass.rs +++ b/src/test/ui/union/union-generic-rpass.rs @@ -1,30 +1,24 @@ // run-pass #![allow(dead_code)] -#![allow(unions_with_drop_fields)] #![feature(untagged_unions)] -union MaybeItem { +union MaybeItem where T::Item: Copy { elem: T::Item, none: (), } -union U { +union U where A: Copy, B: Copy { a: A, b: B, } -unsafe fn union_transmute(a: A) -> B { +unsafe fn union_transmute(a: A) -> B where A: Copy, B: Copy { U { a: a }.b } fn main() { unsafe { - let u = U::> { a: String::from("abcd") }; - - assert_eq!(u.b.len(), 4); - assert_eq!(u.b[0], b'a'); - let b = union_transmute::<(u8, u8), u16>((1, 1)); assert_eq!(b, (1 << 8) + 1); diff --git a/src/test/ui/union/union-nodrop.rs b/src/test/ui/union/union-nodrop.rs index 4cd64ddb5d67f..330f6f9593b13 100644 --- a/src/test/ui/union/union-nodrop.rs +++ b/src/test/ui/union/union-nodrop.rs @@ -1,12 +1,11 @@ // run-pass -#![feature(core_intrinsics)] #![feature(untagged_unions)] -#![allow(unions_with_drop_fields)] #![allow(dead_code)] -use std::intrinsics::needs_drop; +use std::mem::needs_drop; +use std::mem::ManuallyDrop; struct NeedDrop; @@ -16,10 +15,10 @@ impl Drop for NeedDrop { // Constant expressios allow `NoDrop` to go out of scope, // unlike a value of the interior type implementing `Drop`. -static X: () = (NoDrop { inner: NeedDrop }, ()).1; +static X: () = (NoDrop { inner: ManuallyDrop::new(NeedDrop) }, ()).1; // A union that scrubs the drop glue from its inner type -union NoDrop {inner: T} +union NoDrop { inner: ManuallyDrop } // Copy currently can't be implemented on drop-containing unions, // this may change later @@ -40,7 +39,7 @@ struct Baz { y: Box, } -union ActuallyDrop {inner: T} +union ActuallyDrop { inner: ManuallyDrop } impl Drop for ActuallyDrop { fn drop(&mut self) {} diff --git a/src/test/ui/union/union-overwrite.rs b/src/test/ui/union/union-overwrite.rs index 64c60604ba9f0..8234beb74a826 100644 --- a/src/test/ui/union/union-overwrite.rs +++ b/src/test/ui/union/union-overwrite.rs @@ -1,21 +1,27 @@ // run-pass -#![allow(unions_with_drop_fields)] - #![feature(untagged_unions)] #[repr(C)] +#[derive(Copy, Clone)] struct Pair(T, U); #[repr(C)] +#[derive(Copy, Clone)] struct Triple(T, T, T); #[repr(C)] -union U { +union U +where + A: Copy, B: Copy +{ a: Pair, b: B, } #[repr(C)] -union W { +union W +where + A: Copy, B: Copy +{ a: A, b: B, } diff --git a/src/test/ui/union/union-with-drop-fields-lint-rpass.rs b/src/test/ui/union/union-with-drop-fields-lint-rpass.rs deleted file mode 100644 index 4dbeb7c1e7e97..0000000000000 --- a/src/test/ui/union/union-with-drop-fields-lint-rpass.rs +++ /dev/null @@ -1,32 +0,0 @@ -// run-pass - -#![feature(untagged_unions)] -#![allow(dead_code)] -#![allow(unions_with_drop_fields)] - -union U { - a: u8, // OK -} - -union W { - a: String, // OK - b: String, // OK -} - -struct S(String); - -// `S` doesn't implement `Drop` trait, but still has non-trivial destructor -union Y { - a: S, // OK -} - -// We don't know if `T` is trivially-destructable or not until trans -union J { - a: T, // OK -} - -union H { - a: T, // OK -} - -fn main() {} From 2f0c821be9ba9cdf52a45c327b7d3f2831626225 Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Wed, 19 Dec 2018 20:58:20 +0100 Subject: [PATCH 03/20] Change untagged_unions to not allow union fields with drop Union fields may now never have a type with attached destructor. This for example allows unions to use arbitrary field types only by wrapping them in ManuallyDrop. The stable rule remains, that union fields must be Copy. We use the new rule for the `untagged_union` feature. See RFC 2514. Note for ui tests: We can't test move out through Box's deref-move since we can't have a Box in a union anymore. --- src/librustc/Cargo.toml | 2 +- src/librustc/ty/util.rs | 2 + src/librustc_typeck/check/mod.rs | 29 ++++++++ src/librustc_typeck/error_codes.rs | 4 ++ src/test/run-pass/union/union-manuallydrop.rs | 42 ++++++++++++ .../feature-gate-associated_type_bounds.rs | 6 +- .../explicit-union.stderr | 4 +- .../nested-union.stderr | 4 +- .../union-borrow-move-parent-sibling.stderr | 10 +-- src/test/ui/union/union-derive-clone.rs | 13 +++- src/test/ui/union/union-derive-clone.stderr | 10 +-- src/test/ui/union/union-unsafe.rs | 34 ++++++---- src/test/ui/union/union-unsafe.stderr | 66 +++++++++++++++---- .../union/union-with-drop-fields-lint.stderr | 26 -------- ...elds-lint.rs => union-with-drop-fields.rs} | 7 +- .../ui/union/union-with-drop-fields.stderr | 39 +++++++++++ 16 files changed, 222 insertions(+), 76 deletions(-) create mode 100644 src/test/run-pass/union/union-manuallydrop.rs delete mode 100644 src/test/ui/union/union-with-drop-fields-lint.stderr rename src/test/ui/union/{union-with-drop-fields-lint.rs => union-with-drop-fields.rs} (60%) create mode 100644 src/test/ui/union/union-with-drop-fields.stderr diff --git a/src/librustc/Cargo.toml b/src/librustc/Cargo.toml index 9b3609eca3e62..8f191961fdd04 100644 --- a/src/librustc/Cargo.toml +++ b/src/librustc/Cargo.toml @@ -35,5 +35,5 @@ parking_lot = "0.9" byteorder = { version = "1.3" } chalk-engine = { version = "0.9.0", default-features=false } rustc_fs_util = { path = "../librustc_fs_util" } -smallvec = { version = "0.6.7", features = ["union", "may_dangle"] } +smallvec = { version = "0.6.8", features = ["union", "may_dangle"] } measureme = "0.3" diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index d0e95a18c59fc..bd0d5846d9585 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -805,6 +805,8 @@ impl<'tcx> ty::TyS<'tcx> { /// /// (Note that this implies that if `ty` has a destructor attached, /// then `needs_drop` will definitely return `true` for `ty`.) + /// + /// Note that this method is used to check eligible types in unions. #[inline] pub fn needs_drop(&'tcx self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> bool { tcx.needs_drop_raw(param_env.and(self)).0 diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 7475b9cc3b327..2f122e5edb1cb 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1387,9 +1387,38 @@ fn check_union(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) { def.destructor(tcx); // force the destructor to be evaluated check_representable(tcx, span, def_id); check_transparent(tcx, span, def_id); + check_union_fields(tcx, span, def_id); check_packed(tcx, span, def_id); } +fn check_union_fields<'tcx>(tcx: TyCtxt<'tcx>, _sp: Span, item_def_id: DefId) -> bool { + // Without the feature we check Copy types only later + if !tcx.features().untagged_unions { + return true; + } + let t = tcx.type_of(item_def_id); + if let ty::Adt(def, substs) = t.sty { + if def.is_union() { + let fields = &def.non_enum_variant().fields; + for field in fields { + let field_ty = field.ty(tcx, substs); + // We are currently checking the type this field came from, so it must be local + let field_span = tcx.hir().span_if_local(field.did).unwrap(); + let param_env = tcx.param_env(field.did); + if field_ty.needs_drop(tcx, param_env) { + struct_span_err!(tcx.sess, field_span, E0740, + "unions may not contain fields that need dropping") + .span_note(field_span, + "`std::mem::ManuallyDrop` can be used to wrap the type") + .emit(); + return false; + } + } + } + } + return true; +} + /// Checks that an opaque type does not contain cycles and does not use `Self` or `T::Foo` /// projections that would result in "inheriting lifetimes". fn check_opaque<'tcx>( diff --git a/src/librustc_typeck/error_codes.rs b/src/librustc_typeck/error_codes.rs index ef08e8d4f0b7a..a92a2c9600391 100644 --- a/src/librustc_typeck/error_codes.rs +++ b/src/librustc_typeck/error_codes.rs @@ -4863,6 +4863,10 @@ assert_eq!(1, discriminant(&Enum::Struct{a: 7, b: 11})); ``` "##, +E0740: r##" +A `union` can not have fields with destructors. +"##, + E0733: r##" Recursion in an `async fn` requires boxing. For example, this will not compile: diff --git a/src/test/run-pass/union/union-manuallydrop.rs b/src/test/run-pass/union/union-manuallydrop.rs new file mode 100644 index 0000000000000..a43a505086569 --- /dev/null +++ b/src/test/run-pass/union/union-manuallydrop.rs @@ -0,0 +1,42 @@ +#![feature(untagged_unions)] +#![allow(dead_code)] +// run-pass + +use std::mem::needs_drop; +use std::mem::ManuallyDrop; + +struct NeedDrop; + +impl Drop for NeedDrop { + fn drop(&mut self) {} +} + +union UnionOk1 { + empty: (), + value: ManuallyDrop, +} + +union UnionOk2 { + value: ManuallyDrop, +} + +#[allow(dead_code)] +union UnionOk3 { + empty: (), + value: T, +} + +trait Foo { } + +trait ImpliesCopy : Copy { } + +#[allow(dead_code)] +union UnionOk4 { + value: T, +} + +fn main() { + // NeedDrop should not make needs_drop true + assert!(!needs_drop::>()); + assert!(!needs_drop::>()); +} diff --git a/src/test/ui/feature-gates/feature-gate-associated_type_bounds.rs b/src/test/ui/feature-gates/feature-gate-associated_type_bounds.rs index 3d75251e15616..0faa9090f4ebc 100644 --- a/src/test/ui/feature-gates/feature-gate-associated_type_bounds.rs +++ b/src/test/ui/feature-gates/feature-gate-associated_type_bounds.rs @@ -1,7 +1,7 @@ #![feature(untagged_unions)] -trait Tr1 { type As1; } -trait Tr2 { type As2; } +trait Tr1 { type As1: Copy; } +trait Tr2 { type As2: Copy; } struct S1; #[derive(Copy, Clone)] @@ -32,7 +32,7 @@ enum _En1> { union _Un1> { //~^ ERROR associated type bounds are unstable - outest: T, + outest: std::mem::ManuallyDrop, outer: T::As1, inner: ::As2, } diff --git a/src/test/ui/rfc-2093-infer-outlives/explicit-union.stderr b/src/test/ui/rfc-2093-infer-outlives/explicit-union.stderr index adf62651cacb1..8aa246e8bfeb3 100644 --- a/src/test/ui/rfc-2093-infer-outlives/explicit-union.stderr +++ b/src/test/ui/rfc-2093-infer-outlives/explicit-union.stderr @@ -1,7 +1,7 @@ error: rustc_outlives - --> $DIR/explicit-union.rs:6:1 + --> $DIR/explicit-union.rs:5:1 | -LL | / union Foo<'b, U> { +LL | / union Foo<'b, U: Copy> { LL | | bar: Bar<'b, U> LL | | } | |_^ diff --git a/src/test/ui/rfc-2093-infer-outlives/nested-union.stderr b/src/test/ui/rfc-2093-infer-outlives/nested-union.stderr index fc4b1a223294d..a42285a56d089 100644 --- a/src/test/ui/rfc-2093-infer-outlives/nested-union.stderr +++ b/src/test/ui/rfc-2093-infer-outlives/nested-union.stderr @@ -1,7 +1,7 @@ error: rustc_outlives - --> $DIR/nested-union.rs:6:1 + --> $DIR/nested-union.rs:5:1 | -LL | / union Foo<'a, T> { +LL | / union Foo<'a, T: Copy> { LL | | field1: Bar<'a, T> LL | | } | |_^ diff --git a/src/test/ui/union/union-borrow-move-parent-sibling.stderr b/src/test/ui/union/union-borrow-move-parent-sibling.stderr index 1a7a02690d3ea..8ba155bafb0b9 100644 --- a/src/test/ui/union/union-borrow-move-parent-sibling.stderr +++ b/src/test/ui/union/union-borrow-move-parent-sibling.stderr @@ -1,5 +1,5 @@ error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borrowed as mutable (via `u.x.0`) - --> $DIR/union-borrow-move-parent-sibling.rs:54:13 + --> $DIR/union-borrow-move-parent-sibling.rs:53:13 | LL | let a = &mut u.x.0; | ---------- mutable borrow occurs here (via `u.x.0`) @@ -11,7 +11,7 @@ LL | use_borrow(a); = note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0` error[E0382]: use of moved value: `u` - --> $DIR/union-borrow-move-parent-sibling.rs:61:13 + --> $DIR/union-borrow-move-parent-sibling.rs:60:13 | LL | let u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; | - move occurs because `u` has type `U`, which does not implement the `Copy` trait @@ -21,7 +21,7 @@ LL | let b = u.y; | ^^^ value used here after move error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borrowed as mutable (via `u.x.0.0`) - --> $DIR/union-borrow-move-parent-sibling.rs:67:13 + --> $DIR/union-borrow-move-parent-sibling.rs:66:13 | LL | let a = &mut (u.x.0).0; | -------------- mutable borrow occurs here (via `u.x.0.0`) @@ -33,7 +33,7 @@ LL | use_borrow(a); = note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0.0` error[E0382]: use of moved value: `u` - --> $DIR/union-borrow-move-parent-sibling.rs:74:13 + --> $DIR/union-borrow-move-parent-sibling.rs:73:13 | LL | let u = U { x: ((MockVec::new(), MockVec::new()), MockVec::new()) }; | - move occurs because `u` has type `U`, which does not implement the `Copy` trait @@ -43,7 +43,7 @@ LL | let b = u.y; | ^^^ value used here after move error[E0502]: cannot borrow `u` (via `u.x`) as immutable because it is also borrowed as mutable (via `u.y`) - --> $DIR/union-borrow-move-parent-sibling.rs:80:13 + --> $DIR/union-borrow-move-parent-sibling.rs:79:13 | LL | let a = &mut *u.y; | --- mutable borrow occurs here (via `u.y`) diff --git a/src/test/ui/union/union-derive-clone.rs b/src/test/ui/union/union-derive-clone.rs index 64c3caef44906..60e280f53f52c 100644 --- a/src/test/ui/union/union-derive-clone.rs +++ b/src/test/ui/union/union-derive-clone.rs @@ -1,5 +1,7 @@ #![feature(untagged_unions)] +use std::mem::ManuallyDrop; + #[derive(Clone)] //~ ERROR the trait bound `U1: std::marker::Copy` is not satisfied union U1 { a: u8, @@ -18,14 +20,19 @@ union U3 { } #[derive(Clone, Copy)] -union U4 { +union U4 { a: T, // OK } +#[derive(Clone, Copy)] +union U5 { + a: ManuallyDrop, // OK +} + #[derive(Clone)] struct CloneNoCopy; fn main() { - let u = U4 { a: CloneNoCopy }; - let w = u.clone(); //~ ERROR no method named `clone` found for type `U4` + let u = U5 { a: ManuallyDrop::new(CloneNoCopy) }; + let w = u.clone(); //~ ERROR no method named `clone` found for type `U5` } diff --git a/src/test/ui/union/union-derive-clone.stderr b/src/test/ui/union/union-derive-clone.stderr index 4f4c779b12bb3..a6dcd556a1fee 100644 --- a/src/test/ui/union/union-derive-clone.stderr +++ b/src/test/ui/union/union-derive-clone.stderr @@ -1,22 +1,22 @@ error[E0277]: the trait bound `U1: std::marker::Copy` is not satisfied - --> $DIR/union-derive-clone.rs:3:10 + --> $DIR/union-derive-clone.rs:5:10 | LL | #[derive(Clone)] | ^^^^^ the trait `std::marker::Copy` is not implemented for `U1` | = note: required by `std::clone::AssertParamIsCopy` -error[E0599]: no method named `clone` found for type `U4` in the current scope - --> $DIR/union-derive-clone.rs:30:15 +error[E0599]: no method named `clone` found for type `U5` in the current scope + --> $DIR/union-derive-clone.rs:37:15 | -LL | union U4 { +LL | union U5 { | ----------- method `clone` not found for this ... LL | let w = u.clone(); | ^^^^^ method not found in `U4` | = note: the method `clone` exists but the following trait bounds were not satisfied: - `U4 : std::clone::Clone` + `U5 : std::clone::Clone` = help: items from traits can only be used if the trait is implemented and in scope = note: the following trait defines an item `clone`, perhaps you need to implement it: candidate #1: `std::clone::Clone` diff --git a/src/test/ui/union/union-unsafe.rs b/src/test/ui/union/union-unsafe.rs index 6cfde35fe4c34..8535cbd019ce8 100644 --- a/src/test/ui/union/union-unsafe.rs +++ b/src/test/ui/union/union-unsafe.rs @@ -1,15 +1,16 @@ #![feature(untagged_unions)] +use std::mem::ManuallyDrop; union U1 { a: u8 } union U2 { - a: String + a: ManuallyDrop } union U3 { - a: T + a: ManuallyDrop } union U4 { @@ -17,13 +18,16 @@ union U4 { } fn generic_noncopy() { - let mut u3 = U3 { a: T::default() }; - u3.a = T::default(); //~ ERROR assignment to non-`Copy` union field is unsafe + let mut u3 = U3 { a: ManuallyDrop::new(T::default()) }; + u3.a = ManuallyDrop::new(T::default()); //~ ERROR assignment to non-`Copy` union field is unsafe + *u3.a = T::default(); //~ ERROR access to union field is unsafe } fn generic_copy() { - let mut u3 = U3 { a: T::default() }; - u3.a = T::default(); // OK + let mut u3 = U3 { a: ManuallyDrop::new(T::default()) }; + u3.a = ManuallyDrop::new(T::default()); // OK + *u3.a = T::default(); //~ ERROR access to union field is unsafe + let mut u4 = U4 { a: T::default() }; u4.a = T::default(); // OK } @@ -32,14 +36,20 @@ fn main() { let mut u1 = U1 { a: 10 }; // OK let a = u1.a; //~ ERROR access to union field is unsafe u1.a = 11; // OK + let U1 { a } = u1; //~ ERROR access to union field is unsafe if let U1 { a: 12 } = u1 {} //~ ERROR access to union field is unsafe // let U1 { .. } = u1; // OK - let mut u2 = U2 { a: String::from("old") }; // OK - u2.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field is unsafe - let mut u3 = U3 { a: 0 }; // OK - u3.a = 1; // OK - let mut u3 = U3 { a: String::from("old") }; // OK - u3.a = String::from("new"); //~ ERROR assignment to non-`Copy` union field is unsafe + let mut u2 = U2 { a: ManuallyDrop::new(String::from("old")) }; // OK + u2.a = ManuallyDrop::new(String::from("new")); //~ ERROR assignment to non-`Copy` union + *u2.a = String::from("new"); //~ ERROR access to union field is unsafe + + let mut u3 = U3 { a: ManuallyDrop::new(0) }; // OK + u3.a = ManuallyDrop::new(1); // OK + *u3.a = 1; //~ ERROR access to union field is unsafe + + let mut u3 = U3 { a: ManuallyDrop::new(String::from("old")) }; // OK + u3.a = ManuallyDrop::new(String::from("new")); //~ ERROR assignment to non-`Copy` union + *u3.a = String::from("new"); //~ ERROR access to union field is unsafe } diff --git a/src/test/ui/union/union-unsafe.stderr b/src/test/ui/union/union-unsafe.stderr index ab62508fcf666..e020dab63f8f4 100644 --- a/src/test/ui/union/union-unsafe.stderr +++ b/src/test/ui/union/union-unsafe.stderr @@ -1,13 +1,29 @@ error[E0133]: assignment to non-`Copy` union field is unsafe and requires unsafe function or block - --> $DIR/union-unsafe.rs:21:5 + --> $DIR/union-unsafe.rs:22:5 | -LL | u3.a = T::default(); - | ^^^^ assignment to non-`Copy` union field +LL | u3.a = ManuallyDrop::new(T::default()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to non-`Copy` union field | = note: the previous content of the field will be dropped, which causes undefined behavior if the field was not properly initialized error[E0133]: access to union field is unsafe and requires unsafe function or block - --> $DIR/union-unsafe.rs:33:13 + --> $DIR/union-unsafe.rs:23:6 + | +LL | *u3.a = T::default(); + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/union-unsafe.rs:29:6 + | +LL | *u3.a = T::default(); + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/union-unsafe.rs:37:13 | LL | let a = u1.a; | ^^^^ access to union field @@ -15,7 +31,7 @@ LL | let a = u1.a; = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior error[E0133]: access to union field is unsafe and requires unsafe function or block - --> $DIR/union-unsafe.rs:35:14 + --> $DIR/union-unsafe.rs:40:14 | LL | let U1 { a } = u1; | ^ access to union field @@ -23,7 +39,7 @@ LL | let U1 { a } = u1; = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior error[E0133]: access to union field is unsafe and requires unsafe function or block - --> $DIR/union-unsafe.rs:36:20 + --> $DIR/union-unsafe.rs:41:20 | LL | if let U1 { a: 12 } = u1 {} | ^^ access to union field @@ -31,21 +47,45 @@ LL | if let U1 { a: 12 } = u1 {} = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior error[E0133]: assignment to non-`Copy` union field is unsafe and requires unsafe function or block - --> $DIR/union-unsafe.rs:40:5 + --> $DIR/union-unsafe.rs:45:5 | -LL | u2.a = String::from("new"); - | ^^^^ assignment to non-`Copy` union field +LL | u2.a = ManuallyDrop::new(String::from("new")); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to non-`Copy` union field | = note: the previous content of the field will be dropped, which causes undefined behavior if the field was not properly initialized +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/union-unsafe.rs:46:6 + | +LL | *u2.a = String::from("new"); + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/union-unsafe.rs:50:6 + | +LL | *u3.a = 1; + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + error[E0133]: assignment to non-`Copy` union field is unsafe and requires unsafe function or block - --> $DIR/union-unsafe.rs:44:5 + --> $DIR/union-unsafe.rs:53:5 | -LL | u3.a = String::from("new"); - | ^^^^ assignment to non-`Copy` union field +LL | u3.a = ManuallyDrop::new(String::from("new")); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ assignment to non-`Copy` union field | = note: the previous content of the field will be dropped, which causes undefined behavior if the field was not properly initialized -error: aborting due to 6 previous errors +error[E0133]: access to union field is unsafe and requires unsafe function or block + --> $DIR/union-unsafe.rs:54:6 + | +LL | *u3.a = String::from("new"); + | ^^^^ access to union field + | + = note: the field may not be properly initialized: using uninitialized data will cause undefined behavior + +error: aborting due to 11 previous errors For more information about this error, try `rustc --explain E0133`. diff --git a/src/test/ui/union/union-with-drop-fields-lint.stderr b/src/test/ui/union/union-with-drop-fields-lint.stderr deleted file mode 100644 index 2f90f240d2e19..0000000000000 --- a/src/test/ui/union/union-with-drop-fields-lint.stderr +++ /dev/null @@ -1,26 +0,0 @@ -error: union contains a field with possibly non-trivial drop code, drop code of union fields is ignored when dropping the union - --> $DIR/union-with-drop-fields-lint.rs:10:5 - | -LL | a: String, - | ^^^^^^^^^ - | -note: lint level defined here - --> $DIR/union-with-drop-fields-lint.rs:3:9 - | -LL | #![deny(unions_with_drop_fields)] - | ^^^^^^^^^^^^^^^^^^^^^^^ - -error: union contains a field with possibly non-trivial drop code, drop code of union fields is ignored when dropping the union - --> $DIR/union-with-drop-fields-lint.rs:18:5 - | -LL | a: S, - | ^^^^ - -error: union contains a field with possibly non-trivial drop code, drop code of union fields is ignored when dropping the union - --> $DIR/union-with-drop-fields-lint.rs:23:5 - | -LL | a: T, - | ^^^^ - -error: aborting due to 3 previous errors - diff --git a/src/test/ui/union/union-with-drop-fields-lint.rs b/src/test/ui/union/union-with-drop-fields.rs similarity index 60% rename from src/test/ui/union/union-with-drop-fields-lint.rs rename to src/test/ui/union/union-with-drop-fields.rs index 8e502aa55f952..e3c63a6d5b5a2 100644 --- a/src/test/ui/union/union-with-drop-fields-lint.rs +++ b/src/test/ui/union/union-with-drop-fields.rs @@ -1,13 +1,12 @@ #![feature(untagged_unions)] #![allow(dead_code)] -#![deny(unions_with_drop_fields)] union U { a: u8, // OK } union W { - a: String, //~ ERROR union contains a field with possibly non-trivial drop code + a: String, //~ ERROR unions may not contain fields that need dropping b: String, // OK, only one field is reported } @@ -15,12 +14,12 @@ struct S(String); // `S` doesn't implement `Drop` trait, but still has non-trivial destructor union Y { - a: S, //~ ERROR union contains a field with possibly non-trivial drop code + a: S, //~ ERROR unions may not contain fields that need dropping } // We don't know if `T` is trivially-destructable or not until trans union J { - a: T, //~ ERROR union contains a field with possibly non-trivial drop code + a: T, //~ ERROR unions may not contain fields that need dropping } union H { diff --git a/src/test/ui/union/union-with-drop-fields.stderr b/src/test/ui/union/union-with-drop-fields.stderr new file mode 100644 index 0000000000000..0e77279be616a --- /dev/null +++ b/src/test/ui/union/union-with-drop-fields.stderr @@ -0,0 +1,39 @@ +error[E0740]: unions may not contain fields that need dropping + --> $DIR/union-with-drop-fields.rs:9:5 + | +LL | a: String, + | ^^^^^^^^^ + | +note: `std::mem::ManuallyDrop` can be used to wrap the type + --> $DIR/union-with-drop-fields.rs:9:5 + | +LL | a: String, + | ^^^^^^^^^ + +error[E0740]: unions may not contain fields that need dropping + --> $DIR/union-with-drop-fields.rs:17:5 + | +LL | a: S, + | ^^^^ + | +note: `std::mem::ManuallyDrop` can be used to wrap the type + --> $DIR/union-with-drop-fields.rs:17:5 + | +LL | a: S, + | ^^^^ + +error[E0740]: unions may not contain fields that need dropping + --> $DIR/union-with-drop-fields.rs:22:5 + | +LL | a: T, + | ^^^^ + | +note: `std::mem::ManuallyDrop` can be used to wrap the type + --> $DIR/union-with-drop-fields.rs:22:5 + | +LL | a: T, + | ^^^^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0740`. From fe13bbd0648c4f7ad8f7cdfe540ca13bc93ade60 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 3 Jul 2019 11:56:59 +0200 Subject: [PATCH 04/20] Remove unions_with_drop_fields lint Cases where it would trigger are now hard errors. --- .../src/lints/listing/warn-by-default.md | 24 --------------- src/librustc_lint/builtin.rs | 30 ------------------- src/librustc_lint/lib.rs | 3 -- 3 files changed, 57 deletions(-) diff --git a/src/doc/rustc/src/lints/listing/warn-by-default.md b/src/doc/rustc/src/lints/listing/warn-by-default.md index e486240fda896..813d7c4bafef8 100644 --- a/src/doc/rustc/src/lints/listing/warn-by-default.md +++ b/src/doc/rustc/src/lints/listing/warn-by-default.md @@ -596,30 +596,6 @@ warning: function cannot return without recursing | ``` -## unions-with-drop-fields - -This lint detects use of unions that contain fields with possibly non-trivial drop code. Some -example code that triggers this lint: - -```rust -#![feature(untagged_unions)] - -union U { - s: String, -} -``` - -This will produce: - -```text -warning: union contains a field with possibly non-trivial drop code, drop code of union fields is ignored when dropping the union - --> src/main.rs:4:5 - | -4 | s: String, - | ^^^^^^^^^ - | -``` - ## unknown-lints This lint detects unrecognized lint attribute. Some diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index d0a7eab071c31..f00bcd41b601e 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -979,35 +979,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnstableFeatures { } } -declare_lint! { - UNIONS_WITH_DROP_FIELDS, - Warn, - "use of unions that contain fields with possibly non-trivial drop code" -} - -declare_lint_pass!( - /// Lint for unions that contain fields with possibly non-trivial destructors. - UnionsWithDropFields => [UNIONS_WITH_DROP_FIELDS] -); - -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnionsWithDropFields { - fn check_item(&mut self, ctx: &LateContext<'_, '_>, item: &hir::Item) { - if let hir::ItemKind::Union(ref vdata, _) = item.kind { - for field in vdata.fields() { - let field_ty = ctx.tcx.type_of( - ctx.tcx.hir().local_def_id(field.hir_id)); - if field_ty.needs_drop(ctx.tcx, ctx.param_env) { - ctx.span_lint(UNIONS_WITH_DROP_FIELDS, - field.span, - "union contains a field with possibly non-trivial drop code, \ - drop code of union fields is ignored when dropping the union"); - return; - } - } - } - } -} - declare_lint! { pub UNREACHABLE_PUB, Allow, @@ -1287,7 +1258,6 @@ declare_lint_pass!( NO_MANGLE_GENERIC_ITEMS, MUTABLE_TRANSMUTES, UNSTABLE_FEATURES, - UNIONS_WITH_DROP_FIELDS, UNREACHABLE_PUB, TYPE_ALIAS_BOUNDS, TRIVIAL_BOUNDS diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 0e054013cd779..a898e1a3dcfa1 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -164,9 +164,6 @@ macro_rules! late_lint_mod_passes { // Depends on referenced function signatures in expressions MutableTransmutes: MutableTransmutes, - // Depends on types of fields, checks if they implement Drop - UnionsWithDropFields: UnionsWithDropFields, - TypeAliasBounds: TypeAliasBounds, TrivialConstraints: TrivialConstraints, From e247a40a1c5f37cf76ca41d5bbee09985a809529 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 3 Jul 2019 13:02:47 +0200 Subject: [PATCH 05/20] Fixes #41073, it is no longer an ICE --- src/test/ui/union/issue-41073.rs | 24 ++++++++++++++++++++++++ src/test/ui/union/issue-41073.stderr | 15 +++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 src/test/ui/union/issue-41073.rs create mode 100644 src/test/ui/union/issue-41073.stderr diff --git a/src/test/ui/union/issue-41073.rs b/src/test/ui/union/issue-41073.rs new file mode 100644 index 0000000000000..91e9a0d0b659c --- /dev/null +++ b/src/test/ui/union/issue-41073.rs @@ -0,0 +1,24 @@ +#![feature(untagged_unions)] + +union Test { + a: A, //~ ERROR unions may not contain fields that need dropping + b: B +} + +#[derive(Debug)] +struct A(i32); +impl Drop for A { + fn drop(&mut self) { println!("A"); } +} + +#[derive(Debug)] +struct B(f32); +impl Drop for B { + fn drop(&mut self) { println!("B"); } +} + +fn main() { + let mut test = Test { a: A(3) }; + println!("{:?}", unsafe { test.b }); + unsafe { test.b = B(0.5); } +} diff --git a/src/test/ui/union/issue-41073.stderr b/src/test/ui/union/issue-41073.stderr new file mode 100644 index 0000000000000..2e9598b227124 --- /dev/null +++ b/src/test/ui/union/issue-41073.stderr @@ -0,0 +1,15 @@ +error[E0740]: unions may not contain fields that need dropping + --> $DIR/issue-41073.rs:4:5 + | +LL | a: A, + | ^^^^ + | +note: `std::mem::ManuallyDrop` can be used to wrap the type + --> $DIR/issue-41073.rs:4:5 + | +LL | a: A, + | ^^^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0740`. From 05a644e8257b20c3372f27420d28bef5fd6d6d73 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 4 Jul 2019 15:51:55 +0200 Subject: [PATCH 06/20] Update src/librustc_typeck/check/mod.rs Co-Authored-By: Mazdak Farrokhzad --- src/librustc_typeck/check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 2f122e5edb1cb..294d1992b9d9d 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1402,7 +1402,7 @@ fn check_union_fields<'tcx>(tcx: TyCtxt<'tcx>, _sp: Span, item_def_id: DefId) -> let fields = &def.non_enum_variant().fields; for field in fields { let field_ty = field.ty(tcx, substs); - // We are currently checking the type this field came from, so it must be local + // We are currently checking the type this field came from, so it must be local. let field_span = tcx.hir().span_if_local(field.did).unwrap(); let param_env = tcx.param_env(field.did); if field_ty.needs_drop(tcx, param_env) { From 8c5ae86496d25ddd9df3486df46bcb671ce619ad Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 4 Jul 2019 15:52:37 +0200 Subject: [PATCH 07/20] Update src/librustc_typeck/check/mod.rs Co-Authored-By: Mazdak Farrokhzad --- src/librustc_typeck/check/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 294d1992b9d9d..a8b3a0483899c 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1391,7 +1391,9 @@ fn check_union(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) { check_packed(tcx, span, def_id); } -fn check_union_fields<'tcx>(tcx: TyCtxt<'tcx>, _sp: Span, item_def_id: DefId) -> bool { +/// When the `#![feature(untagged_unions)]` gate is active, +/// check that the fields of the `union` does not contain fields that need dropping. +fn check_union_fields(tcx: TyCtxt<'_>, _: Span, item_def_id: DefId) -> bool { // Without the feature we check Copy types only later if !tcx.features().untagged_unions { return true; From 0301eafd32a128b31cc7c551d5844f6fce965caa Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 4 Jul 2019 15:52:48 +0200 Subject: [PATCH 08/20] Update src/librustc_typeck/error_codes.rs Co-Authored-By: Mazdak Farrokhzad --- src/librustc_typeck/error_codes.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/error_codes.rs b/src/librustc_typeck/error_codes.rs index a92a2c9600391..78dedca8027f7 100644 --- a/src/librustc_typeck/error_codes.rs +++ b/src/librustc_typeck/error_codes.rs @@ -4864,7 +4864,7 @@ assert_eq!(1, discriminant(&Enum::Struct{a: 7, b: 11})); "##, E0740: r##" -A `union` can not have fields with destructors. +A `union` cannot have fields with destructors. "##, E0733: r##" From bf25a9c82fb9d78447da885eb70d7234d135ecd5 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 4 Jul 2019 15:54:16 +0200 Subject: [PATCH 09/20] Update src/test/run-pass/union/union-nodrop.rs Co-Authored-By: Mazdak Farrokhzad --- src/test/ui/union/union-nodrop.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/union/union-nodrop.rs b/src/test/ui/union/union-nodrop.rs index 330f6f9593b13..c99228d7d3439 100644 --- a/src/test/ui/union/union-nodrop.rs +++ b/src/test/ui/union/union-nodrop.rs @@ -13,7 +13,7 @@ impl Drop for NeedDrop { fn drop(&mut self) {} } -// Constant expressios allow `NoDrop` to go out of scope, +// Constant expressions allow `NoDrop` to go out of scope, // unlike a value of the interior type implementing `Drop`. static X: () = (NoDrop { inner: ManuallyDrop::new(NeedDrop) }, ()).1; From fc512d2cdf5ab654e55e25725b7243812f4630d1 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 4 Jul 2019 18:21:06 +0200 Subject: [PATCH 10/20] More descriptive variable name --- src/librustc_typeck/check/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index a8b3a0483899c..ea27efcd0fbcd 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1398,8 +1398,8 @@ fn check_union_fields(tcx: TyCtxt<'_>, _: Span, item_def_id: DefId) -> bool { if !tcx.features().untagged_unions { return true; } - let t = tcx.type_of(item_def_id); - if let ty::Adt(def, substs) = t.sty { + let item_type = tcx.type_of(item_def_id); + if let ty::Adt(def, substs) = item_type.sty { if def.is_union() { let fields = &def.non_enum_variant().fields; for field in fields { From 616cf52358a66f0d546aac67a58958e2b50c5f0b Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 4 Jul 2019 18:30:41 +0200 Subject: [PATCH 11/20] Extend union-nodrop.rs test --- src/test/ui/union/union-nodrop.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/ui/union/union-nodrop.rs b/src/test/ui/union/union-nodrop.rs index c99228d7d3439..59282bec59b84 100644 --- a/src/test/ui/union/union-nodrop.rs +++ b/src/test/ui/union/union-nodrop.rs @@ -13,10 +13,14 @@ impl Drop for NeedDrop { fn drop(&mut self) {} } -// Constant expressions allow `NoDrop` to go out of scope, +// Constant expressios allow `NoDrop` to go out of scope, // unlike a value of the interior type implementing `Drop`. static X: () = (NoDrop { inner: ManuallyDrop::new(NeedDrop) }, ()).1; +const Y: () = (NoDrop { inner: ManuallyDrop::new(NeedDrop) }, ()).1; + +const fn _f() { (NoDrop { inner: ManuallyDrop::new(NeedDrop) }, ()).1 } + // A union that scrubs the drop glue from its inner type union NoDrop { inner: ManuallyDrop } From 50ec10e6059f923204c573b4004612e99aa6cdad Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Sep 2019 10:59:29 +0200 Subject: [PATCH 12/20] rpass tests are now part of `ui` tests --- src/test/ui/union/union-derive-clone.stderr | 2 +- .../union/union-manuallydrop-rpass.rs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/test/{run-pass/union/union-manuallydrop.rs => ui/union/union-manuallydrop-rpass.rs} (100%) diff --git a/src/test/ui/union/union-derive-clone.stderr b/src/test/ui/union/union-derive-clone.stderr index a6dcd556a1fee..6893f9176f2db 100644 --- a/src/test/ui/union/union-derive-clone.stderr +++ b/src/test/ui/union/union-derive-clone.stderr @@ -13,7 +13,7 @@ LL | union U5 { | ----------- method `clone` not found for this ... LL | let w = u.clone(); - | ^^^^^ method not found in `U4` + | ^^^^^ method not found in `U5` | = note: the method `clone` exists but the following trait bounds were not satisfied: `U5 : std::clone::Clone` diff --git a/src/test/run-pass/union/union-manuallydrop.rs b/src/test/ui/union/union-manuallydrop-rpass.rs similarity index 100% rename from src/test/run-pass/union/union-manuallydrop.rs rename to src/test/ui/union/union-manuallydrop-rpass.rs From 9c1ad0ff2fe64c02a91c1daf0ce6670b1eaf75f6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Sep 2019 12:22:06 +0200 Subject: [PATCH 13/20] Preserve originally intended test semantics --- src/test/ui/union/union-generic-rpass.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/ui/union/union-generic-rpass.rs b/src/test/ui/union/union-generic-rpass.rs index 5ca3bc0f722c3..eb169c516d2a8 100644 --- a/src/test/ui/union/union-generic-rpass.rs +++ b/src/test/ui/union/union-generic-rpass.rs @@ -3,8 +3,10 @@ #![feature(untagged_unions)] -union MaybeItem where T::Item: Copy { - elem: T::Item, +use std::mem::ManuallyDrop; + +union MaybeItem { + elem: ManuallyDrop, none: (), } @@ -25,7 +27,7 @@ fn main() { let v: Vec = vec![1, 2, 3]; let mut i = v.iter(); i.next(); - let mi = MaybeItem::> { elem: i.next().unwrap() }; - assert_eq!(*mi.elem, 2); + let mi = MaybeItem::> { elem: ManuallyDrop::new(i.next().unwrap()) }; + assert_eq!(**mi.elem, 2); } } From 2fc257ca8144ad6a41b053a01f977afce7a23c95 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Sep 2019 12:40:57 +0200 Subject: [PATCH 14/20] Prefer `ManuallyDrop::{take,new}` over `ptr::{read,write}` --- src/libstd/lib.rs | 1 + src/libstd/panicking.rs | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 5ff32d7adafc2..8e3e02586a625 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -275,6 +275,7 @@ #![feature(link_args)] #![feature(linkage)] #![feature(log_syntax)] +#![feature(manually_drop_take)] #![feature(maybe_uninit_ref)] #![feature(maybe_uninit_slice)] #![feature(mem_take)] diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 296f2a59bb82e..2dde81bb0ecd4 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -13,7 +13,6 @@ use crate::any::Any; use crate::fmt; use crate::intrinsics; use crate::mem::{self, ManuallyDrop}; -use crate::ptr; use crate::raw; use crate::sync::atomic::{AtomicBool, Ordering}; use crate::sys::stdio::panic_output; @@ -283,8 +282,9 @@ pub unsafe fn r#try R>(f: F) -> Result> fn do_call R, R>(data: *mut u8) { unsafe { let data = data as *mut Data; - let f = ptr::read(&mut *(*data).f); - ptr::write(&mut *(*data).r, f()); + let data = &mut (*data); + let f = ManuallyDrop::take(&mut data.f); + data.r = ManuallyDrop::new(f()); } } } From fb23a5cf3bda8d4d5ee89be4c1777d28a1061f9b Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Sep 2019 12:46:41 +0200 Subject: [PATCH 15/20] Clarify a vague comment --- src/librustc_typeck/check/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index ea27efcd0fbcd..d2a4b824439d3 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1394,7 +1394,8 @@ fn check_union(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) { /// When the `#![feature(untagged_unions)]` gate is active, /// check that the fields of the `union` does not contain fields that need dropping. fn check_union_fields(tcx: TyCtxt<'_>, _: Span, item_def_id: DefId) -> bool { - // Without the feature we check Copy types only later + // Without the feature we check that all fields are `Copy` in our stability checking + // infrastructure. if !tcx.features().untagged_unions { return true; } From 7e1a65dce1d13d02de8e0ffafad15538556cb7b9 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Sep 2019 13:53:46 +0200 Subject: [PATCH 16/20] Ensure we do not treat all unions as not having any drop glue. --- src/test/ui/union/union-custom-drop.rs | 19 +++++++++++++++ src/test/ui/union/union-custom-drop.stderr | 28 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 src/test/ui/union/union-custom-drop.rs create mode 100644 src/test/ui/union/union-custom-drop.stderr diff --git a/src/test/ui/union/union-custom-drop.rs b/src/test/ui/union/union-custom-drop.rs new file mode 100644 index 0000000000000..8f816cc1b737c --- /dev/null +++ b/src/test/ui/union/union-custom-drop.rs @@ -0,0 +1,19 @@ +// test for a union with a field that's a union with a manual impl Drop +// Ensures we do not treat all unions as not having any drop glue. + +#![feature(untagged_unions)] + +union Foo { + bar: Bar, //~ ERROR unions may not contain fields that need dropping +} + +union Bar { + a: i32, + b: u32, +} + +impl Drop for Bar { + fn drop(&mut self) {} +} + +fn main() {} diff --git a/src/test/ui/union/union-custom-drop.stderr b/src/test/ui/union/union-custom-drop.stderr new file mode 100644 index 0000000000000..8e4e5dd40fd3f --- /dev/null +++ b/src/test/ui/union/union-custom-drop.stderr @@ -0,0 +1,28 @@ +error[E0601]: `main` function not found in crate `union_custom_drop` + --> $DIR/union-custom-drop.rs:4:1 + | +LL | / #![feature(untagged_unions)] +LL | | +LL | | union Foo { +LL | | bar: Bar, +... | +LL | | } +LL | | } + | |_^ consider adding a `main` function to `$DIR/union-custom-drop.rs` + +error[E0740]: unions may not contain fields that need dropping + --> $DIR/union-custom-drop.rs:7:5 + | +LL | bar: Bar, + | ^^^^^^^^ + | +note: `std::mem::ManuallyDrop` can be used to wrap the type + --> $DIR/union-custom-drop.rs:7:5 + | +LL | bar: Bar, + | ^^^^^^^^ + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0601, E0740. +For more information about an error, try `rustc --explain E0601`. From f5669ebb45353bfb828650472af783331b7b90fe Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 20 Sep 2019 15:23:10 +0200 Subject: [PATCH 17/20] Update ui stderr --- src/test/ui/union/union-custom-drop.stderr | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/test/ui/union/union-custom-drop.stderr b/src/test/ui/union/union-custom-drop.stderr index 8e4e5dd40fd3f..ee2333f905fb4 100644 --- a/src/test/ui/union/union-custom-drop.stderr +++ b/src/test/ui/union/union-custom-drop.stderr @@ -1,15 +1,3 @@ -error[E0601]: `main` function not found in crate `union_custom_drop` - --> $DIR/union-custom-drop.rs:4:1 - | -LL | / #![feature(untagged_unions)] -LL | | -LL | | union Foo { -LL | | bar: Bar, -... | -LL | | } -LL | | } - | |_^ consider adding a `main` function to `$DIR/union-custom-drop.rs` - error[E0740]: unions may not contain fields that need dropping --> $DIR/union-custom-drop.rs:7:5 | @@ -22,7 +10,6 @@ note: `std::mem::ManuallyDrop` can be used to wrap the type LL | bar: Bar, | ^^^^^^^^ -error: aborting due to 2 previous errors +error: aborting due to previous error -Some errors have detailed explanations: E0601, E0740. -For more information about an error, try `rustc --explain E0601`. +For more information about this error, try `rustc --explain E0740`. From bb5a652361bb416e38fa4f41784dd5d08b232f59 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 17 Oct 2019 13:28:14 +0200 Subject: [PATCH 18/20] Rebase fallout --- src/librustc_typeck/check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index d2a4b824439d3..3e0527393cd12 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1400,7 +1400,7 @@ fn check_union_fields(tcx: TyCtxt<'_>, _: Span, item_def_id: DefId) -> bool { return true; } let item_type = tcx.type_of(item_def_id); - if let ty::Adt(def, substs) = item_type.sty { + if let ty::Adt(def, substs) = item_type.kind { if def.is_union() { let fields = &def.non_enum_variant().fields; for field in fields { From 0653694fdc46a2bca119b9790d1dfd62e1b4901e Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 21 Oct 2019 09:08:05 +0200 Subject: [PATCH 19/20] Don't silently do nothing on mis_use of `check_union_fields` --- src/librustc_typeck/check/mod.rs | 33 ++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 3e0527393cd12..d5182d69c3e70 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1393,7 +1393,7 @@ fn check_union(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) { /// When the `#![feature(untagged_unions)]` gate is active, /// check that the fields of the `union` does not contain fields that need dropping. -fn check_union_fields(tcx: TyCtxt<'_>, _: Span, item_def_id: DefId) -> bool { +fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: DefId) -> bool { // Without the feature we check that all fields are `Copy` in our stability checking // infrastructure. if !tcx.features().untagged_unions { @@ -1401,23 +1401,24 @@ fn check_union_fields(tcx: TyCtxt<'_>, _: Span, item_def_id: DefId) -> bool { } let item_type = tcx.type_of(item_def_id); if let ty::Adt(def, substs) = item_type.kind { - if def.is_union() { - let fields = &def.non_enum_variant().fields; - for field in fields { - let field_ty = field.ty(tcx, substs); - // We are currently checking the type this field came from, so it must be local. - let field_span = tcx.hir().span_if_local(field.did).unwrap(); - let param_env = tcx.param_env(field.did); - if field_ty.needs_drop(tcx, param_env) { - struct_span_err!(tcx.sess, field_span, E0740, - "unions may not contain fields that need dropping") - .span_note(field_span, - "`std::mem::ManuallyDrop` can be used to wrap the type") - .emit(); - return false; - } + assert!(def.is_union()); + let fields = &def.non_enum_variant().fields; + for field in fields { + let field_ty = field.ty(tcx, substs); + // We are currently checking the type this field came from, so it must be local. + let field_span = tcx.hir().span_if_local(field.did).unwrap(); + let param_env = tcx.param_env(field.did); + if field_ty.needs_drop(tcx, param_env) { + struct_span_err!(tcx.sess, field_span, E0740, + "unions may not contain fields that need dropping") + .span_note(field_span, + "`std::mem::ManuallyDrop` can be used to wrap the type") + .emit(); + return false; } } + } else { + span_bug!(span, "unions must be ty::Adt, but got {:?}", item_type.kind); } return true; } From 875bdd5dbe663a6dafd785b86c8964a90653eeb7 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 21 Oct 2019 10:12:09 +0200 Subject: [PATCH 20/20] Report even duplilcate errors in case the feature gat is not active --- src/librustc_typeck/check/mod.rs | 5 ---- .../feature-gate-untagged_unions.rs | 4 +-- .../feature-gate-untagged_unions.stderr | 29 +++++++++++++++++-- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index d5182d69c3e70..c5b809ad380f7 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1394,11 +1394,6 @@ fn check_union(tcx: TyCtxt<'_>, id: hir::HirId, span: Span) { /// When the `#![feature(untagged_unions)]` gate is active, /// check that the fields of the `union` does not contain fields that need dropping. fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: DefId) -> bool { - // Without the feature we check that all fields are `Copy` in our stability checking - // infrastructure. - if !tcx.features().untagged_unions { - return true; - } let item_type = tcx.type_of(item_def_id); if let ty::Adt(def, substs) = item_type.kind { assert!(def.is_union()); diff --git a/src/test/ui/feature-gates/feature-gate-untagged_unions.rs b/src/test/ui/feature-gates/feature-gate-untagged_unions.rs index 3bac3d853e907..9ee0e6f681dcc 100644 --- a/src/test/ui/feature-gates/feature-gate-untagged_unions.rs +++ b/src/test/ui/feature-gates/feature-gate-untagged_unions.rs @@ -7,11 +7,11 @@ union U2 { // OK } union U3 { //~ ERROR unions with non-`Copy` fields are unstable - a: String, + a: String, //~ ERROR unions may not contain fields that need dropping } union U4 { //~ ERROR unions with non-`Copy` fields are unstable - a: T, + a: T, //~ ERROR unions may not contain fields that need dropping } union U5 { //~ ERROR unions with `Drop` implementations are unstable diff --git a/src/test/ui/feature-gates/feature-gate-untagged_unions.stderr b/src/test/ui/feature-gates/feature-gate-untagged_unions.stderr index f59a34e2c81f6..1885518a4585c 100644 --- a/src/test/ui/feature-gates/feature-gate-untagged_unions.stderr +++ b/src/test/ui/feature-gates/feature-gate-untagged_unions.stderr @@ -31,6 +31,31 @@ LL | | } = note: for more information, see https://github.com/rust-lang/rust/issues/32836 = help: add `#![feature(untagged_unions)]` to the crate attributes to enable -error: aborting due to 3 previous errors +error[E0740]: unions may not contain fields that need dropping + --> $DIR/feature-gate-untagged_unions.rs:10:5 + | +LL | a: String, + | ^^^^^^^^^ + | +note: `std::mem::ManuallyDrop` can be used to wrap the type + --> $DIR/feature-gate-untagged_unions.rs:10:5 + | +LL | a: String, + | ^^^^^^^^^ + +error[E0740]: unions may not contain fields that need dropping + --> $DIR/feature-gate-untagged_unions.rs:14:5 + | +LL | a: T, + | ^^^^ + | +note: `std::mem::ManuallyDrop` can be used to wrap the type + --> $DIR/feature-gate-untagged_unions.rs:14:5 + | +LL | a: T, + | ^^^^ + +error: aborting due to 5 previous errors -For more information about this error, try `rustc --explain E0658`. +Some errors have detailed explanations: E0658, E0740. +For more information about an error, try `rustc --explain E0658`.