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

do not apply DerefMut on union field #75584

Merged
merged 4 commits into from
Sep 5, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
also detect DerefMut in nested union fields
  • Loading branch information
RalfJung committed Aug 31, 2020
commit 44defaea3a2dd2e7e40336d3609df12b83db424a
29 changes: 17 additions & 12 deletions compiler/rustc_typeck/src/check/place_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
debug!("convert_place_derefs_to_mutable: exprs={:?}", exprs);

// Fix up autoderefs and derefs.
let mut inside_union = false;
for (i, &expr) in exprs.iter().rev().enumerate() {
debug!("convert_place_derefs_to_mutable: i={} expr={:?}", i, expr);

let mut source = self.node_ty(expr.hir_id);
if matches!(expr.kind, hir::ExprKind::Unary(hir::UnOp::UnDeref, _)) {
// Clear previous flag; after a pointer indirection it does not apply any more.
inside_union = false;
}
if source.ty_adt_def().map_or(false, |adt| adt.is_union()) {
inside_union = true;
}
// Fix up the autoderefs. Autorefs can only occur immediately preceding
// overloaded place ops, and will be fixed by them in order to get
// the correct region.
let mut source = self.node_ty(expr.hir_id);
// Do not mutate adjustments in place, but rather take them,
// and replace them after mutating them, to avoid having the
// typeck results borrowed during (`deref_mut`) method resolution.
Expand All @@ -238,17 +246,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
// If this is a union field, also throw an error.
// Union fields should not get mutable auto-deref'd (see RFC 2514).
if let hir::ExprKind::Field(ref outer_expr, _) = expr.kind {
let ty = self.node_ty(outer_expr.hir_id);
if ty.ty_adt_def().map_or(false, |adt| adt.is_union()) {
let mut err = self.tcx.sess.struct_span_err(
expr.span,
"not automatically applying `DerefMut` on union field",
);
err.help("writing to this field calls the destructor for the old value");
err.help("add an explicit `*` if that is desired, or call `ptr::write` to not run the destructor");
err.emit();
}
if inside_union {
let mut err = self.tcx.sess.struct_span_err(
expr.span,
"not automatically applying `DerefMut` on union field",
);
err.help("writing to this field calls the destructor for the old value");
err.help("add an explicit `*` if that is desired, or call `ptr::write` to not run the destructor");
err.emit();
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions src/test/ui/union/union-deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,16 @@

use std::mem::ManuallyDrop;

union U<T> { x:(), f: ManuallyDrop<(T,)> }
union U1<T> { x:(), f: ManuallyDrop<(T,)> }

union U2<T> { x:(), f: (ManuallyDrop<(T,)>,) }

fn main() {
let mut u : U<Vec<i32>> = U { x: () };
let mut u : U1<Vec<i32>> = U1 { x: () };
unsafe { (*u.f).0 = Vec::new() }; // explicit deref, this compiles
unsafe { u.f.0 = Vec::new() }; //~ERROR not automatically applying `DerefMut` on union field

let mut u : U2<Vec<i32>> = U2 { x: () };
unsafe { (*u.f.0).0 = Vec::new() }; // explicit deref, this compiles
unsafe { u.f.0.0 = Vec::new() }; //~ERROR not automatically applying `DerefMut` on union field
}
13 changes: 11 additions & 2 deletions src/test/ui/union/union-deref.stderr
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
error: not automatically applying `DerefMut` on union field
--> $DIR/union-deref.rs:12:14
--> $DIR/union-deref.rs:14:14
|
LL | unsafe { u.f.0 = Vec::new() };
| ^^^
|
= help: writing to this field calls the destructor for the old value
= help: add an explicit `*` if that is desired, or call `ptr::write` to not run the destructor

error: aborting due to previous error
error: not automatically applying `DerefMut` on union field
--> $DIR/union-deref.rs:18:14
|
LL | unsafe { u.f.0.0 = Vec::new() };
| ^^^^^^^
Copy link
Member Author

Choose a reason for hiding this comment

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

The span for this is slightly wrong, I'd expect it to only cover u.f.0. I don't know how this is happening, since the span is right above.

Copy link
Member

Choose a reason for hiding this comment

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

I would guess this has to do with the special handling of multiple tuple field accesses in a row. Perhaps 0.0 is a float token.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't know there was such special handling.^^ So this would be a problem elsewhere, in the code that sets up the span for these places?

|
= help: writing to this field calls the destructor for the old value
= help: add an explicit `*` if that is desired, or call `ptr::write` to not run the destructor

error: aborting due to 2 previous errors