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

Remove destructor-related restrictions from unions #38934

Merged
merged 1 commit into from
Jan 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions src/librustc/ty/contents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
})
});

if def.is_union() {
// unions don't have destructors regardless of the child types
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test checking that needs_drop still returns true on unions that implement Drop themselves?
I suspect the condition should be def.is_union() && !def.has_dtor() instead of just def.is_union().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The destructor is added below IIRC.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, that bit gets re-added below.

res = res - TC::NeedsDrop;
}

if def.has_dtor() {
res = res | TC::OwnsDtor;
}
Expand Down
63 changes: 63 additions & 0 deletions src/test/run-pass/union/union-nodrop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(core_intrinsics)]
#![feature(untagged_unions)]

#![allow(unions_with_drop_fields)]
#![allow(dead_code)]

use std::intrinsics::needs_drop;

// drop types in destructors should not require
// drop_types_in_const
static X: Option<NoDrop<Box<u8>>> = None;

// A union that scrubs the drop glue from its inner type
union NoDrop<T> {inner: T}

// Copy currently can't be implemented on drop-containing unions,
// this may change later
// https://github.com/rust-lang/rust/pull/38934#issuecomment-271219289

// // We should be able to implement Copy for NoDrop
// impl<T> Copy for NoDrop<T> {}
// impl<T> Clone for NoDrop<T> {fn clone(&self) -> Self { *self }}

// // We should be able to implement Copy for things using NoDrop
// #[derive(Copy, Clone)]
struct Foo {
x: NoDrop<Box<u8>>
}

struct Baz {
x: NoDrop<Box<u8>>,
y: Box<u8>,
}

union ActuallyDrop<T> {inner: T}

impl<T> Drop for ActuallyDrop<T> {
fn drop(&mut self) {}
}

fn main() {
unsafe {
// NoDrop should not make needs_drop true
assert!(!needs_drop::<Foo>());
assert!(!needs_drop::<NoDrop<u8>>());
assert!(!needs_drop::<NoDrop<Box<u8>>>());
// presence of other drop types should still work
assert!(needs_drop::<Baz>());
// drop impl on union itself should work
assert!(needs_drop::<ActuallyDrop<u8>>());
assert!(needs_drop::<ActuallyDrop<Box<u8>>>());
}
}