Skip to content

Commit 6e1c115

Browse files
committed
Auto merge of #132915 - veluca93:unsafe-fields, r=jswrenn
Implement the unsafe-fields RFC. RFC: rust-lang/rfcs#3458 Tracking: - #132922 r? jswrenn
2 parents c49a687 + 9022bb2 commit 6e1c115

38 files changed

+793
-85
lines changed

Diff for: compiler/rustc_ast/src/ast.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3063,6 +3063,7 @@ pub struct FieldDef {
30633063
pub id: NodeId,
30643064
pub span: Span,
30653065
pub vis: Visibility,
3066+
pub safety: Safety,
30663067
pub ident: Option<Ident>,
30673068

30683069
pub ty: P<Ty>,

Diff for: compiler/rustc_ast/src/mut_visit.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1115,10 +1115,11 @@ fn walk_poly_trait_ref<T: MutVisitor>(vis: &mut T, p: &mut PolyTraitRef) {
11151115
}
11161116

11171117
pub fn walk_field_def<T: MutVisitor>(visitor: &mut T, fd: &mut FieldDef) {
1118-
let FieldDef { span, ident, vis, id, ty, attrs, is_placeholder: _ } = fd;
1118+
let FieldDef { span, ident, vis, id, ty, attrs, is_placeholder: _, safety } = fd;
11191119
visitor.visit_id(id);
11201120
visit_attrs(visitor, attrs);
11211121
visitor.visit_vis(vis);
1122+
visit_safety(visitor, safety);
11221123
visit_opt(ident, |ident| visitor.visit_ident(ident));
11231124
visitor.visit_ty(ty);
11241125
visitor.visit_span(span);

Diff for: compiler/rustc_ast/src/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,7 @@ pub fn walk_struct_def<'a, V: Visitor<'a>>(
961961
}
962962

963963
pub fn walk_field_def<'a, V: Visitor<'a>>(visitor: &mut V, field: &'a FieldDef) -> V::Result {
964-
let FieldDef { attrs, id: _, span: _, vis, ident, ty, is_placeholder: _ } = field;
964+
let FieldDef { attrs, id: _, span: _, vis, ident, ty, is_placeholder: _, safety: _ } = field;
965965
walk_list!(visitor, visit_attribute, attrs);
966966
try_visit!(visitor.visit_vis(vis));
967967
visit_opt!(visitor, visit_ident, ident);

Diff for: compiler/rustc_ast_lowering/src/item.rs

+1
Original file line numberDiff line numberDiff line change
@@ -724,6 +724,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
724724
},
725725
vis_span: self.lower_span(f.vis.span),
726726
ty,
727+
safety: self.lower_safety(f.safety, hir::Safety::Safe),
727728
}
728729
}
729730

Diff for: compiler/rustc_ast_passes/src/feature_gate.rs

+1
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session, features: &Features) {
557557
gate_all!(global_registration, "global registration is experimental");
558558
gate_all!(return_type_notation, "return type notation is experimental");
559559
gate_all!(pin_ergonomics, "pinned reference syntax is experimental");
560+
gate_all!(unsafe_fields, "`unsafe` fields are experimental");
560561

561562
if !visitor.features.never_patterns() {
562563
if let Some(spans) = spans.get(&sym::never_patterns) {

Diff for: compiler/rustc_expand/src/placeholders.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use rustc_ast::mut_visit::*;
22
use rustc_ast::ptr::P;
33
use rustc_ast::token::Delimiter;
44
use rustc_ast::visit::AssocCtxt;
5-
use rustc_ast::{self as ast};
5+
use rustc_ast::{self as ast, Safety};
66
use rustc_data_structures::fx::FxHashMap;
77
use rustc_span::DUMMY_SP;
88
use rustc_span::symbol::Ident;
@@ -173,6 +173,7 @@ pub(crate) fn placeholder(
173173
ty: ty(),
174174
vis,
175175
is_placeholder: true,
176+
safety: Safety::Default,
176177
}]),
177178
AstFragmentKind::Variants => AstFragment::Variants(smallvec![ast::Variant {
178179
attrs: Default::default(),

Diff for: compiler/rustc_feature/src/unstable.rs

+2
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,8 @@ declare_features! (
629629
/// Allows creation of instances of a struct by moving fields that have
630630
/// not changed from prior instances of the same struct (RFC #2528)
631631
(unstable, type_changing_struct_update, "1.58.0", Some(86555)),
632+
/// Allows declaring fields `unsafe`.
633+
(incomplete, unsafe_fields, "CURRENT_RUSTC_VERSION", Some(132922)),
632634
/// Allows const generic parameters to be defined with types that
633635
/// are not `Sized`, e.g. `fn foo<const N: [u8]>() {`.
634636
(incomplete, unsized_const_params, "1.82.0", Some(95174)),

Diff for: compiler/rustc_hir/src/hir.rs

+1
Original file line numberDiff line numberDiff line change
@@ -3177,6 +3177,7 @@ pub struct FieldDef<'hir> {
31773177
pub hir_id: HirId,
31783178
pub def_id: LocalDefId,
31793179
pub ty: &'hir Ty<'hir>,
3180+
pub safety: Safety,
31803181
}
31813182

31823183
impl FieldDef<'_> {

Diff for: compiler/rustc_hir_analysis/messages.ftl

+7
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,13 @@ hir_analysis_invalid_union_field =
253253
hir_analysis_invalid_union_field_sugg =
254254
wrap the field type in `ManuallyDrop<...>`
255255
256+
hir_analysis_invalid_unsafe_field =
257+
field must implement `Copy` or be wrapped in `ManuallyDrop<...>` to be unsafe
258+
.note = unsafe fields must not have drop side-effects, which is currently enforced via either `Copy` or `ManuallyDrop<...>`
259+
260+
hir_analysis_invalid_unsafe_field_sugg =
261+
wrap the field type in `ManuallyDrop<...>`
262+
256263
hir_analysis_late_bound_const_in_apit = `impl Trait` can only mention const parameters from an fn or impl
257264
.label = const parameter declared here
258265

Diff for: compiler/rustc_hir_analysis/src/check/check.rs

+72-32
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet};
66
use rustc_errors::MultiSpan;
77
use rustc_errors::codes::*;
88
use rustc_hir::def::{CtorKind, DefKind};
9-
use rustc_hir::{Node, intravisit};
9+
use rustc_hir::{Node, Safety, intravisit};
1010
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
1111
use rustc_infer::traits::{Obligation, ObligationCauseCode};
1212
use rustc_lint_defs::builtin::{
@@ -70,6 +70,7 @@ fn check_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) {
7070

7171
check_transparent(tcx, def);
7272
check_packed(tcx, span, def);
73+
check_unsafe_fields(tcx, def_id);
7374
}
7475

7576
fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) {
@@ -81,38 +82,45 @@ fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) {
8182
check_packed(tcx, span, def);
8283
}
8384

85+
fn allowed_union_or_unsafe_field<'tcx>(
86+
tcx: TyCtxt<'tcx>,
87+
ty: Ty<'tcx>,
88+
typing_env: ty::TypingEnv<'tcx>,
89+
span: Span,
90+
) -> bool {
91+
// We don't just accept all !needs_drop fields, due to semver concerns.
92+
let allowed = match ty.kind() {
93+
ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
94+
ty::Tuple(tys) => {
95+
// allow tuples of allowed types
96+
tys.iter().all(|ty| allowed_union_or_unsafe_field(tcx, ty, typing_env, span))
97+
}
98+
ty::Array(elem, _len) => {
99+
// Like `Copy`, we do *not* special-case length 0.
100+
allowed_union_or_unsafe_field(tcx, *elem, typing_env, span)
101+
}
102+
_ => {
103+
// Fallback case: allow `ManuallyDrop` and things that are `Copy`,
104+
// also no need to report an error if the type is unresolved.
105+
ty.ty_adt_def().is_some_and(|adt_def| adt_def.is_manually_drop())
106+
|| ty.is_copy_modulo_regions(tcx, typing_env)
107+
|| ty.references_error()
108+
}
109+
};
110+
if allowed && ty.needs_drop(tcx, typing_env) {
111+
// This should never happen. But we can get here e.g. in case of name resolution errors.
112+
tcx.dcx()
113+
.span_delayed_bug(span, "we should never accept maybe-dropping union or unsafe fields");
114+
}
115+
allowed
116+
}
117+
84118
/// Check that the fields of the `union` do not need dropping.
85119
fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> bool {
86120
let item_type = tcx.type_of(item_def_id).instantiate_identity();
87121
if let ty::Adt(def, args) = item_type.kind() {
88122
assert!(def.is_union());
89123

90-
fn allowed_union_field<'tcx>(
91-
ty: Ty<'tcx>,
92-
tcx: TyCtxt<'tcx>,
93-
typing_env: ty::TypingEnv<'tcx>,
94-
) -> bool {
95-
// We don't just accept all !needs_drop fields, due to semver concerns.
96-
match ty.kind() {
97-
ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
98-
ty::Tuple(tys) => {
99-
// allow tuples of allowed types
100-
tys.iter().all(|ty| allowed_union_field(ty, tcx, typing_env))
101-
}
102-
ty::Array(elem, _len) => {
103-
// Like `Copy`, we do *not* special-case length 0.
104-
allowed_union_field(*elem, tcx, typing_env)
105-
}
106-
_ => {
107-
// Fallback case: allow `ManuallyDrop` and things that are `Copy`,
108-
// also no need to report an error if the type is unresolved.
109-
ty.ty_adt_def().is_some_and(|adt_def| adt_def.is_manually_drop())
110-
|| ty.is_copy_modulo_regions(tcx, typing_env)
111-
|| ty.references_error()
112-
}
113-
}
114-
}
115-
116124
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
117125
for field in &def.non_enum_variant().fields {
118126
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
@@ -121,7 +129,7 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> b
121129
continue;
122130
};
123131

124-
if !allowed_union_field(field_ty, tcx, typing_env) {
132+
if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
125133
let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
126134
// We are currently checking the type this field came from, so it must be local.
127135
Some(Node::Field(field)) => (field.span, field.ty.span),
@@ -136,10 +144,6 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> b
136144
note: (),
137145
});
138146
return false;
139-
} else if field_ty.needs_drop(tcx, typing_env) {
140-
// This should never happen. But we can get here e.g. in case of name resolution errors.
141-
tcx.dcx()
142-
.span_delayed_bug(span, "we should never accept maybe-dropping union fields");
143147
}
144148
}
145149
} else {
@@ -148,6 +152,41 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> b
148152
true
149153
}
150154

155+
/// Check that the unsafe fields do not need dropping.
156+
fn check_unsafe_fields(tcx: TyCtxt<'_>, item_def_id: LocalDefId) {
157+
let span = tcx.def_span(item_def_id);
158+
let item_type = tcx.type_of(item_def_id).instantiate_identity();
159+
let ty::Adt(def, args) = item_type.kind() else {
160+
span_bug!(span, "structs/enums must be ty::Adt, but got {:?}", item_type.kind());
161+
};
162+
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
163+
for field in def.all_fields() {
164+
if field.safety != Safety::Unsafe {
165+
continue;
166+
}
167+
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
168+
else {
169+
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
170+
continue;
171+
};
172+
173+
if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
174+
let hir::Node::Field(field) = tcx.hir_node_by_def_id(field.did.expect_local()) else {
175+
unreachable!("field has to correspond to hir field")
176+
};
177+
let ty_span = field.ty.span;
178+
tcx.dcx().emit_err(errors::InvalidUnsafeField {
179+
field_span: field.span,
180+
sugg: errors::InvalidUnsafeFieldSuggestion {
181+
lo: ty_span.shrink_to_lo(),
182+
hi: ty_span.shrink_to_hi(),
183+
},
184+
note: (),
185+
});
186+
}
187+
}
188+
}
189+
151190
/// Check that a `static` is inhabited.
152191
fn check_static_inhabited(tcx: TyCtxt<'_>, def_id: LocalDefId) {
153192
// Make sure statics are inhabited.
@@ -1464,6 +1503,7 @@ fn check_enum(tcx: TyCtxt<'_>, def_id: LocalDefId) {
14641503

14651504
detect_discriminant_duplicate(tcx, def);
14661505
check_transparent(tcx, def);
1506+
check_unsafe_fields(tcx, def_id);
14671507
}
14681508

14691509
/// Part of enum check. Given the discriminants of an enum, errors if two or more discriminants are equal

Diff for: compiler/rustc_hir_analysis/src/collect.rs

+1
Original file line numberDiff line numberDiff line change
@@ -1040,6 +1040,7 @@ fn lower_variant(
10401040
did: f.def_id.to_def_id(),
10411041
name: f.ident.name,
10421042
vis: tcx.visibility(f.def_id),
1043+
safety: f.safety,
10431044
})
10441045
.collect();
10451046
let recovered = match def {

Diff for: compiler/rustc_hir_analysis/src/errors.rs

+23
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,17 @@ pub(crate) struct InvalidUnionField {
734734
pub note: (),
735735
}
736736

737+
#[derive(Diagnostic)]
738+
#[diag(hir_analysis_invalid_unsafe_field, code = E0740)]
739+
pub(crate) struct InvalidUnsafeField {
740+
#[primary_span]
741+
pub field_span: Span,
742+
#[subdiagnostic]
743+
pub sugg: InvalidUnsafeFieldSuggestion,
744+
#[note]
745+
pub note: (),
746+
}
747+
737748
#[derive(Diagnostic)]
738749
#[diag(hir_analysis_return_type_notation_on_non_rpitit)]
739750
pub(crate) struct ReturnTypeNotationOnNonRpitit<'tcx> {
@@ -755,6 +766,18 @@ pub(crate) struct InvalidUnionFieldSuggestion {
755766
pub hi: Span,
756767
}
757768

769+
#[derive(Subdiagnostic)]
770+
#[multipart_suggestion(
771+
hir_analysis_invalid_unsafe_field_sugg,
772+
applicability = "machine-applicable"
773+
)]
774+
pub(crate) struct InvalidUnsafeFieldSuggestion {
775+
#[suggestion_part(code = "std::mem::ManuallyDrop<")]
776+
pub lo: Span,
777+
#[suggestion_part(code = ">")]
778+
pub hi: Span,
779+
}
780+
758781
#[derive(Diagnostic)]
759782
#[diag(hir_analysis_return_type_notation_equality_bound)]
760783
pub(crate) struct ReturnTypeNotationEqualityBound {

Diff for: compiler/rustc_metadata/src/rmeta/decoder.rs

+6
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use rustc_data_structures::sync::{Lock, Lrc, OnceLock};
1515
use rustc_data_structures::unhash::UnhashMap;
1616
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
1717
use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, DeriveProcMacro};
18+
use rustc_hir::Safety;
1819
use rustc_hir::def::Res;
1920
use rustc_hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE};
2021
use rustc_hir::definitions::{DefPath, DefPathData};
@@ -1101,6 +1102,7 @@ impl<'a> CrateMetadataRef<'a> {
11011102
did,
11021103
name: self.item_name(did.index),
11031104
vis: self.get_visibility(did.index),
1105+
safety: self.get_safety(did.index),
11041106
})
11051107
.collect(),
11061108
adt_kind,
@@ -1162,6 +1164,10 @@ impl<'a> CrateMetadataRef<'a> {
11621164
.map_id(|index| self.local_def_id(index))
11631165
}
11641166

1167+
fn get_safety(self, id: DefIndex) -> Safety {
1168+
self.root.tables.safety.get(self, id).unwrap_or_else(|| self.missing("safety", id))
1169+
}
1170+
11651171
fn get_trait_item_def_id(self, id: DefIndex) -> Option<DefId> {
11661172
self.root.tables.trait_item_def_id.get(self, id).map(|d| d.decode_from_cdata(self))
11671173
}

Diff for: compiler/rustc_metadata/src/rmeta/encoder.rs

+4
Original file line numberDiff line numberDiff line change
@@ -1599,6 +1599,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
15991599
f.did.index
16001600
}));
16011601

1602+
for field in &variant.fields {
1603+
self.tables.safety.set_some(field.did.index, field.safety);
1604+
}
1605+
16021606
if let Some((CtorKind::Fn, ctor_def_id)) = variant.ctor {
16031607
let fn_sig = tcx.fn_sig(ctor_def_id);
16041608
// FIXME only encode signature for ctor_def_id

Diff for: compiler/rustc_metadata/src/rmeta/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,7 @@ define_tables! {
411411
associated_item_or_field_def_ids: Table<DefIndex, LazyArray<DefIndex>>,
412412
def_kind: Table<DefIndex, DefKind>,
413413
visibility: Table<DefIndex, LazyValue<ty::Visibility<DefIndex>>>,
414+
safety: Table<DefIndex, hir::Safety>,
414415
def_span: Table<DefIndex, LazyValue<Span>>,
415416
def_ident_span: Table<DefIndex, LazyValue<Span>>,
416417
lookup_stability: Table<DefIndex, LazyValue<attr::Stability>>,

Diff for: compiler/rustc_metadata/src/rmeta/table.rs

+7
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,13 @@ fixed_size_enum! {
198198
}
199199
}
200200

201+
fixed_size_enum! {
202+
hir::Safety {
203+
( Unsafe )
204+
( Safe )
205+
}
206+
}
207+
201208
fixed_size_enum! {
202209
ty::Asyncness {
203210
( Yes )

0 commit comments

Comments
 (0)