Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change untagged_unions to not allow union fields with drop #62330

Merged
merged 20 commits into from
Oct 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions src/doc/rustc/src/lints/listing/warn-by-default.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
2 changes: 2 additions & 0 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 0 additions & 30 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
28 changes: 28 additions & 0 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1387,9 +1387,37 @@ 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);
}

/// 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 {
let item_type = tcx.type_of(item_def_id);
if let ty::Adt(def, substs) = item_type.kind {
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;
}

/// 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>(
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_typeck/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4863,6 +4863,10 @@ assert_eq!(1, discriminant(&Enum::Struct{a: 7, b: 11}));
```
"##,

E0740: r##"
A `union` cannot have fields with destructors.
"##,

E0733: r##"
Recursion in an `async fn` requires boxing. For example, this will not compile:

Expand Down
1 change: 1 addition & 0 deletions src/libstd/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
17 changes: 8 additions & 9 deletions src/libstd/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,7 @@ use core::panic::{BoxMeUp, PanicInfo, Location};
use crate::any::Any;
use crate::fmt;
use crate::intrinsics;
use crate::mem;
use crate::ptr;
use crate::mem::{self, ManuallyDrop};
use crate::raw;
use crate::sync::atomic::{AtomicBool, Ordering};
use crate::sys::stdio::panic_output;
Expand Down Expand Up @@ -227,10 +226,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: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>> {
#[allow(unions_with_drop_fields)]
union Data<F, R> {
f: F,
r: R,
f: ManuallyDrop<F>,
r: ManuallyDrop<R>,
}

// We do some sketchy operations with ownership here for the sake of
Expand Down Expand Up @@ -261,7 +259,7 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
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::<F, R>,
Expand All @@ -271,7 +269,7 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>

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);
Expand All @@ -284,8 +282,9 @@ pub unsafe fn r#try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<dyn Any + Send>>
fn do_call<F: FnOnce() -> R, R>(data: *mut u8) {
unsafe {
let data = data as *mut Data<F, R>;
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());
}
}
}
Expand Down
15 changes: 8 additions & 7 deletions src/test/ui/associated-type-bounds/union-bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand Down Expand Up @@ -71,7 +71,8 @@ where
let _: &'a T = &x.f0;
}

union UnSelf<T> where Self: Tr1<As1: Tr2> {
#[derive(Copy, Clone)]
union UnSelf<T> where Self: Tr1<As1: Tr2>, T: Copy {
f0: T,
f1: <Self as Tr1>::As1,
f2: <<Self as Tr1>::As1 as Tr2>::As2,
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/drop/dynamic-drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -152,17 +153,16 @@ fn assignment1(a: &Allocator, c0: bool) {
_v = _w;
}

#[allow(unions_with_drop_fields)]
union Boxy<T> {
a: T,
b: T,
a: ManuallyDrop<T>,
b: ManuallyDrop<T>,
}

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));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -32,7 +32,7 @@ enum _En1<T: Tr1<As1: Tr2>> {

union _Un1<T: Tr1<As1: Tr2>> {
//~^ ERROR associated type bounds are unstable
outest: T,
outest: std::mem::ManuallyDrop<T>,
outer: T::As1,
inner: <T::As1 as Tr2>::As2,
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/feature-gates/feature-gate-untagged_unions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ union U2<T: Copy> { // 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<T> { //~ 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
Expand Down
29 changes: 27 additions & 2 deletions src/test/ui/feature-gates/feature-gate-untagged_unions.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
5 changes: 2 additions & 3 deletions src/test/ui/rfc-2093-infer-outlives/explicit-union.rs
Original file line number Diff line number Diff line change
@@ -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,
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/rfc-2093-infer-outlives/explicit-union.stderr
Original file line number Diff line number Diff line change
@@ -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 | | }
| |_^
Expand Down
5 changes: 2 additions & 3 deletions src/test/ui/rfc-2093-infer-outlives/nested-union.rs
Original file line number Diff line number Diff line change
@@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/rfc-2093-infer-outlives/nested-union.stderr
Original file line number Diff line number Diff line change
@@ -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 | | }
| |_^
Expand Down
Loading