OpenE2K
/
rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #75584 - RalfJung:union-no-deref, r=matthewjasper 

do not apply DerefMut on union field

This implements the part of [RFC 2514](https://github.com/rust-lang/rfcs/blob/master/text/2514-union-initialization-and-drop.md) about `DerefMut`. Unlike described in the RFC, we only apply this warning specifically when doing `DerefMut` of a `ManuallyDrop` field; that is really the case we are worried about here.

@matthewjasper suggested I patch `convert_place_derefs_to_mutable` and `convert_place_op_to_mutable` for this, but I could not find anything to do in `convert_place_op_to_mutable` and this is sufficient to make the test pass. However, maybe there are some other cases this misses? I have no familiarity with this code.

This is a breaking change *in theory*, if someone used `ManuallyDrop<T>` in a union field and relied on automatic `DerefMut`. But on stable this means `T: Copy`, so the `ManuallyDrop` is rather pointless.

Cc https://github.com/rust-lang/rust/issues/55149
This commit is contained in:
bors 2020-09-05 11:47:01 +00:00
parent 02fe30971e 66b340f500
commit 81a769f261
3 changed files with 109 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
compiler/rustc_typeck/src/check/place_op.rs
View File

@ -193,7 +193,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// Convert auto-derefs, indices, etc of an expression from `Deref` and `Index`
/// into `DerefMut` and `IndexMut` respectively.
///
/// This is a second pass of typechecking derefs/indices. We need this we do not
/// This is a second pass of typechecking derefs/indices. We need this because we do not
/// always know whether a place needs to be mutable or not in the first pass.
/// This happens whether there is an implicit mutable reborrow, e.g. when the type
/// is used as the receiver of a method call.
@ -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.
@ -236,6 +244,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let ty::Ref(region, _, mutbl) = *method.sig.output().kind() {
*deref = OverloadedDeref { region, mutbl };
}
// If this is a union field, also throw an error for `DerefMut` of `ManuallyDrop` (see RFC 2514).
// This helps avoid accidental drops.
if inside_union
&& source.ty_adt_def().map_or(false, |adt| adt.is_manually_drop())
{
let mut err = self.tcx.sess.struct_span_err(
expr.span,
"not automatically applying `DerefMut` on `ManuallyDrop` union field",
);
err.help(
"writing to this reference 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();
}
}
}
source = adjustment.target;

28
src/test/ui/union/union-deref.rs Normal file
View File

@ -0,0 +1,28 @@
// ignore-tidy-linelength
//! Test the part of RFC 2514 that is about not applying `DerefMut` coercions
//! of union fields.
#![feature(untagged_unions)]
use std::mem::ManuallyDrop;
union U1<T> { x:(), f: ManuallyDrop<(T,)> }
union U2<T> { x:(), f: (ManuallyDrop<(T,)>,) }
fn main() {
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 `ManuallyDrop` union field
unsafe { &mut (*u.f).0 }; // explicit deref, this compiles
unsafe { &mut u.f.0 }; //~ERROR not automatically applying `DerefMut` on `ManuallyDrop` union field
unsafe { (*u.f).0.push(0) }; // explicit deref, this compiles
unsafe { u.f.0.push(0) }; //~ERROR not automatically applying `DerefMut` on `ManuallyDrop` 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 `ManuallyDrop` union field
unsafe { &mut (*u.f.0).0 }; // explicit deref, this compiles
unsafe { &mut u.f.0.0 }; //~ERROR not automatically applying `DerefMut` on `ManuallyDrop` union field
unsafe { (*u.f.0).0.push(0) }; // explicit deref, this compiles
unsafe { u.f.0.0.push(0) }; //~ERROR not automatically applying `DerefMut` on `ManuallyDrop` union field
}

56
src/test/ui/union/union-deref.stderr Normal file
View File

@ -0,0 +1,56 @@
error: not automatically applying `DerefMut` on `ManuallyDrop` union field
--> $DIR/union-deref.rs:15:14
|
LL | unsafe { u.f.0 = Vec::new() };
| ^^^
|
= help: writing to this reference 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: not automatically applying `DerefMut` on `ManuallyDrop` union field
--> $DIR/union-deref.rs:17:19
|
LL | unsafe { &mut u.f.0 };
| ^^^
|
= help: writing to this reference 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: not automatically applying `DerefMut` on `ManuallyDrop` union field
--> $DIR/union-deref.rs:19:14
|
LL | unsafe { u.f.0.push(0) };
| ^^^
|
= help: writing to this reference 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: not automatically applying `DerefMut` on `ManuallyDrop` union field
--> $DIR/union-deref.rs:23:14
|
LL | unsafe { u.f.0.0 = Vec::new() };
| ^^^^^^^
|
= help: writing to this reference 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: not automatically applying `DerefMut` on `ManuallyDrop` union field
--> $DIR/union-deref.rs:25:19
|
LL | unsafe { &mut u.f.0.0 };
| ^^^^^^^
|
= help: writing to this reference 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: not automatically applying `DerefMut` on `ManuallyDrop` union field
--> $DIR/union-deref.rs:27:14
|
LL | unsafe { u.f.0.0.push(0) };
| ^^^^^^^
|
= help: writing to this reference 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 6 previous errors