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

Make assignments to Copy union fields safe #42083

Merged
merged 2 commits into from
May 26, 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
24 changes: 24 additions & 0 deletions src/librustc/middle/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ fn type_is_unsafe_function(ty: Ty) -> bool {
struct EffectCheckVisitor<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
tables: &'a ty::TypeckTables<'tcx>,
body_id: hir::BodyId,

/// Whether we're in an unsafe context.
unsafe_context: UnsafeContext,
Expand Down Expand Up @@ -99,10 +100,13 @@ impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> {

fn visit_nested_body(&mut self, body: hir::BodyId) {
let old_tables = self.tables;
let old_body_id = self.body_id;
self.tables = self.tcx.body_tables(body);
self.body_id = body;
let body = self.tcx.hir.body(body);
self.visit_body(body);
self.tables = old_tables;
self.body_id = old_body_id;
}

fn visit_fn(&mut self, fn_kind: FnKind<'tcx>, fn_decl: &'tcx hir::FnDecl,
Expand Down Expand Up @@ -218,6 +222,25 @@ impl<'a, 'tcx> Visitor<'tcx> for EffectCheckVisitor<'a, 'tcx> {
}
}
}
hir::ExprAssign(ref lhs, ref rhs) => {
if let hir::ExprField(ref base_expr, field) = lhs.node {
if let ty::TyAdt(adt, ..) = self.tables.expr_ty_adjusted(base_expr).sty {
if adt.is_union() {
let field_ty = self.tables.expr_ty_adjusted(lhs);
let owner_def_id = self.tcx.hir.body_owner_def_id(self.body_id);
let param_env = self.tcx.param_env(owner_def_id);
if field_ty.moves_by_default(self.tcx, param_env, field.span) {
self.require_unsafe(field.span,
"assignment to non-`Copy` union field");
}
// Do not walk the field expr again.
intravisit::walk_expr(self, base_expr);
intravisit::walk_expr(self, rhs);
return
}
}
}
}
_ => {}
}

Expand All @@ -243,6 +266,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
let mut visitor = EffectCheckVisitor {
tcx: tcx,
tables: &ty::TypeckTables::empty(),
body_id: hir::BodyId { node_id: ast::CRATE_NODE_ID },
unsafe_context: UnsafeContext::new(SafeContext),
};

Expand Down
45 changes: 38 additions & 7 deletions src/test/compile-fail/union/union-unsafe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,46 @@

#![feature(untagged_unions)]

union U {
union U1 {
a: u8
}

union U2 {
a: String
}

union U3<T> {
a: T
}

union U4<T: Copy> {
a: T
}

fn generic_noncopy<T: Default>() {
let mut u3 = U3 { a: T::default() };
u3.a = T::default(); //~ ERROR assignment to non-`Copy` union field requires unsafe
}

fn generic_copy<T: Copy + Default>() {
let mut u3 = U3 { a: T::default() };
u3.a = T::default(); // OK
let mut u4 = U4 { a: T::default() };
u4.a = T::default(); // OK
}

fn main() {
let mut u = U { a: 10 }; // OK
let a = u.a; //~ ERROR access to union field requires unsafe function or block
u.a = 11; //~ ERROR access to union field requires unsafe function or block
let U { a } = u; //~ ERROR matching on union field requires unsafe function or block
if let U { a: 12 } = u {} //~ ERROR matching on union field requires unsafe function or block
// let U { .. } = u; // OK
let mut u1 = U1 { a: 10 }; // OK
let a = u1.a; //~ ERROR access to union field requires unsafe
u1.a = 11; // OK
let U1 { a } = u1; //~ ERROR matching on union field requires unsafe
if let U1 { a: 12 } = u1 {} //~ ERROR matching on union field requires 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 requires 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 requires unsafe
}