Skip to content
Open
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
31 changes: 24 additions & 7 deletions compiler/rustc_hir_typeck/src/place_op.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use rustc_errors::Applicability;
use rustc_hir::LangItem;
use rustc_hir_analysis::autoderef::Autoderef;
use rustc_infer::infer::InferOk;
use rustc_infer::traits::{Obligation, ObligationCauseCode};
Expand All @@ -8,7 +9,8 @@ use rustc_middle::ty::adjustment::{
PointerCoercion,
};
use rustc_middle::ty::{self, Ty};
use rustc_span::{Span, sym};
use rustc_span::{DUMMY_SP, Span, sym};
use rustc_trait_selection::infer::InferCtxtExt;
use tracing::debug;
use {rustc_ast as ast, rustc_hir as hir};

Expand Down Expand Up @@ -285,9 +287,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Clear previous flag; after a pointer indirection it does not apply any more.
inside_union = false;
}
if source.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.
Expand Down Expand Up @@ -315,14 +314,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
};
*deref = OverloadedDeref { mutbl, span: deref.span };
self.enforce_context_effects(None, expr.span, method.def_id, method.args);
// If this is a union field, also throw an error for `DerefMut` of `ManuallyDrop` (see RFC 2514).
// If this is a union field, also throw an error for `DerefMut` of non `BikeshedGuaranteedNoDrop` (see RFC 2514).
// This helps avoid accidental drops.
if inside_union
&& source.ty_adt_def().is_some_and(|adt| adt.is_manually_drop())
&& !self
.infcx
.type_implements_trait(
self.tcx.require_lang_item(
LangItem::BikeshedGuaranteedNoDrop,
DUMMY_SP,
),
[adjustment.target],
Copy link
Member

Choose a reason for hiding this comment

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

Previously this checked source, now it checks adjustment.target... I don't know anything about this code so I can't say what the consequences of that are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously it was an error if source was a ManuallyDrop, irrespective of the type it contains; now it's an error if the expression dereferences to a target that doesn't implement BikeshedGuaranteedNoDrop - so ManuallyDrop<MaybeUninit<T>> is fine, because the target of dereferencing the ManuallyDrop is guaranteed not to have drop glue; but ManuallyDrop<Vec<T>> errors because Vec<T> obviously doesn't have that guarantee.

self.param_env,
)
.must_apply_considering_regions()
{
self.dcx().struct_span_err(
expr.span,
"not automatically applying `DerefMut` on `ManuallyDrop` union field",
"not automatically applying `DerefMut` through union field to target that might have `Drop` glue",
)
.with_help(
"writing to this reference calls the destructor for the old value",
Expand All @@ -335,6 +344,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
self.typeck_results.borrow_mut().adjustments_mut().insert(expr.hir_id, adjustments);
}
// Now that any autoderef of this expression component is complete, if it resolved to a
// `union` then remember we're inside a `union` as we iterate over subsequent expression
// components. If this test were performed prior to autoderef resolution, it would only
// detect expression components of the (owned) `union` type and not references thereto.
// See <https://github.com/rust-lang/rust/issues/141621>.
if source.is_union() {
inside_union = true;
}

match expr.kind {
hir::ExprKind::Index(base_expr, ..) => {
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_middle/src/ty/adt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ bitflags::bitflags! {
const IS_PIN = 1 << 11;
/// Indicates whether the type is `#[pin_project]`.
const IS_PIN_PROJECT = 1 << 12;
/// Indicates whether the type is `MaybeUninit`.
const IS_MAYBE_UNINIT = 1 << 13;
}
}
rustc_data_structures::external_bitflags_debug! { AdtFlags }
Expand Down Expand Up @@ -224,6 +226,10 @@ impl<'tcx> rustc_type_ir::inherent::AdtDef<TyCtxt<'tcx>> for AdtDef<'tcx> {
self.is_manually_drop()
}

fn is_maybe_uninit(self) -> bool {
self.is_maybe_uninit()
}

fn all_field_tys(
self,
tcx: TyCtxt<'tcx>,
Expand Down Expand Up @@ -315,6 +321,9 @@ impl AdtDefData {
if tcx.is_lang_item(did, LangItem::ManuallyDrop) {
flags |= AdtFlags::IS_MANUALLY_DROP;
}
if tcx.is_lang_item(did, LangItem::MaybeUninit) {
flags |= AdtFlags::IS_MAYBE_UNINIT;
}
if tcx.is_lang_item(did, LangItem::UnsafeCell) {
flags |= AdtFlags::IS_UNSAFE_CELL;
}
Expand Down Expand Up @@ -439,6 +448,12 @@ impl<'tcx> AdtDef<'tcx> {
self.flags().contains(AdtFlags::IS_MANUALLY_DROP)
}

/// Returns `true` if this is `ManuallyDrop<T>`.
#[inline]
pub fn is_maybe_uninit(self) -> bool {
self.flags().contains(AdtFlags::IS_MAYBE_UNINIT)
}

/// Returns `true` if this is `Pin<T>`.
#[inline]
pub fn is_pin(self) -> bool {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_next_trait_solver/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,8 +701,8 @@ where
match ty.kind() {
// `&mut T` and `&T` always implement `BikeshedGuaranteedNoDrop`.
ty::Ref(..) => {}
// `ManuallyDrop<T>` always implements `BikeshedGuaranteedNoDrop`.
ty::Adt(def, _) if def.is_manually_drop() => {}
// `ManuallyDrop<T>` and `MaybeUninit<T>` always implement `BikeshedGuaranteedNoDrop`.
ty::Adt(def, _) if def.is_manually_drop() || def.is_maybe_uninit() => {}
// Arrays and tuples implement `BikeshedGuaranteedNoDrop` only if
// their constituent types implement `BikeshedGuaranteedNoDrop`.
ty::Tuple(tys) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1257,8 +1257,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
match *self_ty.skip_binder().kind() {
// `&mut T` and `&T` always implement `BikeshedGuaranteedNoDrop`.
ty::Ref(..) => {}
// `ManuallyDrop<T>` always implements `BikeshedGuaranteedNoDrop`.
ty::Adt(def, _) if def.is_manually_drop() => {}
// `ManuallyDrop<T>` and `MaybeUninit<T>` always implement `BikeshedGuaranteedNoDrop`.
ty::Adt(def, _) if def.is_manually_drop() || def.is_maybe_uninit() => {}
// Arrays and tuples implement `BikeshedGuaranteedNoDrop` only if
// their constituent types implement `BikeshedGuaranteedNoDrop`.
ty::Tuple(tys) => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_type_ir/src/inherent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ pub trait AdtDef<I: Interner>: Copy + Debug + Hash + Eq {
fn is_phantom_data(self) -> bool;

fn is_manually_drop(self) -> bool;
fn is_maybe_uninit(self) -> bool;

// FIXME: perhaps use `all_fields` and expose `FieldDef`.
fn all_field_tys(self, interner: I) -> ty::EarlyBinder<I, impl IntoIterator<Item = I::Ty>>;
Expand Down
1 change: 1 addition & 0 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,7 @@ impl<T: PointeeSized> Copy for &T {}
/// Implemented for:
/// * `&T`, `&mut T` for all `T`,
/// * `ManuallyDrop<T>` for all `T`,
/// * `MaybeUninit<T>` for all `T`,
/// * tuples and arrays whose elements implement `BikeshedGuaranteedNoDrop`,
/// * or otherwise, all types that are `Copy`.
///
Expand Down
27 changes: 21 additions & 6 deletions tests/ui/union/union-deref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,32 @@ 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 { u.f.0 = Vec::new() }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
unsafe { &mut (*u.f).0 }; // explicit deref, this compiles
unsafe { &mut u.f.0 }; //~ERROR not automatically applying `DerefMut` on `ManuallyDrop` union field
unsafe { &mut u.f.0 }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
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
unsafe { u.f.0.push(0) }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue

unsafe { (*(&mut u).f).0 = Vec::new() }; // explicit deref, this compiles
unsafe { (&mut u).f.0 = Vec::new() }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
unsafe { &mut (*(&mut u).f).0 }; // explicit deref, this compiles
unsafe { &mut (&mut u).f.0 }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
unsafe { (*(&mut u).f).0.push(0) }; // explicit deref, this compiles
unsafe { (&mut u).f.0.push(0) }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue


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 { u.f.0.0 = Vec::new() }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
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 { &mut u.f.0.0 }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
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
unsafe { u.f.0.0.push(0) }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue

unsafe { (*(&mut u).f.0).0 = Vec::new() }; // explicit deref, this compiles
unsafe { (&mut u).f.0.0 = Vec::new() }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
unsafe { &mut (*(&mut u).f.0).0 }; // explicit deref, this compiles
unsafe { &mut (&mut u).f.0.0 }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
unsafe { (*(&mut u).f.0).0.push(0) }; // explicit deref, this compiles
unsafe { (&mut u).f.0.0.push(0) }; //~ERROR not automatically applying `DerefMut` through union field to target that might have `Drop` glue
}
74 changes: 64 additions & 10 deletions tests/ui/union/union-deref.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: not automatically applying `DerefMut` on `ManuallyDrop` union field
error: not automatically applying `DerefMut` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:13:14
|
LL | unsafe { u.f.0 = Vec::new() };
Expand All @@ -7,7 +7,7 @@ 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
error: not automatically applying `DerefMut` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:15:19
|
LL | unsafe { &mut u.f.0 };
Expand All @@ -16,7 +16,7 @@ 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
error: not automatically applying `DerefMut` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:17:14
|
LL | unsafe { u.f.0.push(0) };
Expand All @@ -25,32 +25,86 @@ 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:21:14
error: not automatically applying `DerefMut` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:20:14
|
LL | unsafe { (&mut 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` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:22:19
|
LL | unsafe { &mut (&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` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:24:14
|
LL | unsafe { (&mut 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` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:29: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:23:19
error: not automatically applying `DerefMut` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:31: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:25:14
error: not automatically applying `DerefMut` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:33: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
error: not automatically applying `DerefMut` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:36:14
|
LL | unsafe { (&mut 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` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:38:19
|
LL | unsafe { &mut (&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` through union field to target that might have `Drop` glue
--> $DIR/union-deref.rs:40:14
|
LL | unsafe { (&mut 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 12 previous errors

Loading