Skip to content

Commit

Permalink
Rollup merge of rust-lang#137808 - jswrenn:droppy-unsafe-fields, r=nn…
Browse files Browse the repository at this point in the history
…ethercote

Do not require that unsafe fields lack drop glue

Instead, we adopt the position that introducing an `unsafe` field itself carries a safety invariant: that if you assign an invariant to that field weaker than what the field's destructor requires, you must ensure that field is in a droppable state in your destructor.

See:
- rust-lang/rfcs#3458 (comment)
- https://rust-lang.zulipchat.com/#narrow/channel/213817-t-lang/topic/unsafe.20fields.20RFC/near/502113897

Tracking Issue: rust-lang#132922
  • Loading branch information
jieyouxu authored Mar 5, 2025
2 parents a7d1fa7 + 91034ad commit 1c4da09
Show file tree
Hide file tree
Showing 6 changed files with 5 additions and 80 deletions.
7 changes: 0 additions & 7 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,6 @@ hir_analysis_invalid_union_field =
hir_analysis_invalid_union_field_sugg =
wrap the field type in `ManuallyDrop<...>`
hir_analysis_invalid_unsafe_field =
field must implement `Copy` or be wrapped in `ManuallyDrop<...>` to be unsafe
.note = unsafe fields must not have drop side-effects, which is currently enforced via either `Copy` or `ManuallyDrop<...>`
hir_analysis_invalid_unsafe_field_sugg =
wrap the field type in `ManuallyDrop<...>`
hir_analysis_late_bound_const_in_apit = `impl Trait` can only mention const parameters from an fn or impl
.label = const parameter declared here
Expand Down
32 changes: 0 additions & 32 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ fn check_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) {

check_transparent(tcx, def);
check_packed(tcx, span, def);
check_unsafe_fields(tcx, def_id);
}

fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) {
Expand Down Expand Up @@ -144,36 +143,6 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> b
true
}

/// Check that the unsafe fields do not need dropping.
fn check_unsafe_fields(tcx: TyCtxt<'_>, item_def_id: LocalDefId) {
let span = tcx.def_span(item_def_id);
let def = tcx.adt_def(item_def_id);

let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
let args = ty::GenericArgs::identity_for_item(tcx, item_def_id);

for field in def.all_fields() {
if !field.safety.is_unsafe() {
continue;
}

if !allowed_union_or_unsafe_field(tcx, field.ty(tcx, args), typing_env, span) {
let hir::Node::Field(field) = tcx.hir_node_by_def_id(field.did.expect_local()) else {
unreachable!("field has to correspond to hir field")
};
let ty_span = field.ty.span;
tcx.dcx().emit_err(errors::InvalidUnsafeField {
field_span: field.span,
sugg: errors::InvalidUnsafeFieldSuggestion {
lo: ty_span.shrink_to_lo(),
hi: ty_span.shrink_to_hi(),
},
note: (),
});
}
}
}

/// Check that a `static` is inhabited.
fn check_static_inhabited(tcx: TyCtxt<'_>, def_id: LocalDefId) {
// Make sure statics are inhabited.
Expand Down Expand Up @@ -1512,7 +1481,6 @@ fn check_enum(tcx: TyCtxt<'_>, def_id: LocalDefId) {

detect_discriminant_duplicate(tcx, def);
check_transparent(tcx, def);
check_unsafe_fields(tcx, def_id);
}

/// Part of enum check. Given the discriminants of an enum, errors if two or more discriminants are equal
Expand Down
23 changes: 0 additions & 23 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -712,17 +712,6 @@ pub(crate) struct InvalidUnionField {
pub note: (),
}

#[derive(Diagnostic)]
#[diag(hir_analysis_invalid_unsafe_field, code = E0740)]
pub(crate) struct InvalidUnsafeField {
#[primary_span]
pub field_span: Span,
#[subdiagnostic]
pub sugg: InvalidUnsafeFieldSuggestion,
#[note]
pub note: (),
}

#[derive(Diagnostic)]
#[diag(hir_analysis_return_type_notation_on_non_rpitit)]
pub(crate) struct ReturnTypeNotationOnNonRpitit<'tcx> {
Expand All @@ -744,18 +733,6 @@ pub(crate) struct InvalidUnionFieldSuggestion {
pub hi: Span,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(
hir_analysis_invalid_unsafe_field_sugg,
applicability = "machine-applicable"
)]
pub(crate) struct InvalidUnsafeFieldSuggestion {
#[suggestion_part(code = "std::mem::ManuallyDrop<")]
pub lo: Span,
#[suggestion_part(code = ">")]
pub hi: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_return_type_notation_equality_bound)]
pub(crate) struct ReturnTypeNotationEqualityBound {
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,8 +453,8 @@ impl Copy for ! {}
#[stable(feature = "rust1", since = "1.0.0")]
impl<T: ?Sized> Copy for &T {}

/// Marker trait for the types that are allowed in union fields, unsafe fields,
/// and unsafe binder types.
/// Marker trait for the types that are allowed in union fields and unsafe
/// binder types.
///
/// Implemented for:
/// * `&T`, `&mut T` for all `T`,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/unsafe-fields/unsafe-fields.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ fn f(a: A) {
}

struct WithInvalidUnsafeField {
unsafe unsafe_noncopy_field: Vec<u32>, //~ ERROR
unsafe unsafe_noncopy_field: Vec<u32>,
}

struct WithManuallyDropUnsafeField {
Expand Down
17 changes: 2 additions & 15 deletions tests/ui/unsafe-fields/unsafe-fields.stderr
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
error[E0740]: field must implement `Copy` or be wrapped in `ManuallyDrop<...>` to be unsafe
--> $DIR/unsafe-fields.rs:20:5
|
LL | unsafe unsafe_noncopy_field: Vec<u32>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: unsafe fields must not have drop side-effects, which is currently enforced via either `Copy` or `ManuallyDrop<...>`
help: wrap the field type in `ManuallyDrop<...>`
|
LL | unsafe unsafe_noncopy_field: std::mem::ManuallyDrop<Vec<u32>>,
| +++++++++++++++++++++++ +

error[E0133]: use of unsafe field is unsafe and requires unsafe block
--> $DIR/unsafe-fields.rs:15:30
|
Expand Down Expand Up @@ -69,7 +57,6 @@ LL | &raw const self.unsafe_field
|
= note: unsafe fields may carry library invariants

error: aborting due to 8 previous errors
error: aborting due to 7 previous errors

Some errors have detailed explanations: E0133, E0740.
For more information about an error, try `rustc --explain E0133`.
For more information about this error, try `rustc --explain E0133`.

0 comments on commit 1c4da09

Please sign in to comment.