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

Implement the unsafe-fields RFC. #132915

Merged
merged 1 commit into from
Nov 23, 2024
Merged
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
1 change: 1 addition & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3063,6 +3063,7 @@ pub struct FieldDef {
pub id: NodeId,
pub span: Span,
pub vis: Visibility,
pub safety: Safety,
pub ident: Option<Ident>,

pub ty: P<Ty>,
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1115,10 +1115,11 @@ fn walk_poly_trait_ref<T: MutVisitor>(vis: &mut T, p: &mut PolyTraitRef) {
}

pub fn walk_field_def<T: MutVisitor>(visitor: &mut T, fd: &mut FieldDef) {
let FieldDef { span, ident, vis, id, ty, attrs, is_placeholder: _ } = fd;
let FieldDef { span, ident, vis, id, ty, attrs, is_placeholder: _, safety } = fd;
visitor.visit_id(id);
visit_attrs(visitor, attrs);
visitor.visit_vis(vis);
visit_safety(visitor, safety);
visit_opt(ident, |ident| visitor.visit_ident(ident));
visitor.visit_ty(ty);
visitor.visit_span(span);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ pub fn walk_struct_def<'a, V: Visitor<'a>>(
}

pub fn walk_field_def<'a, V: Visitor<'a>>(visitor: &mut V, field: &'a FieldDef) -> V::Result {
let FieldDef { attrs, id: _, span: _, vis, ident, ty, is_placeholder: _ } = field;
let FieldDef { attrs, id: _, span: _, vis, ident, ty, is_placeholder: _, safety: _ } = field;
walk_list!(visitor, visit_attribute, attrs);
try_visit!(visitor.visit_vis(vis));
visit_opt!(visitor, visit_ident, ident);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -724,6 +724,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
},
vis_span: self.lower_span(f.vis.span),
ty,
safety: self.lower_safety(f.safety, hir::Safety::Safe),
}
}

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_passes/src/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ pub fn check_crate(krate: &ast::Crate, sess: &Session, features: &Features) {
gate_all!(global_registration, "global registration is experimental");
gate_all!(return_type_notation, "return type notation is experimental");
gate_all!(pin_ergonomics, "pinned reference syntax is experimental");
gate_all!(unsafe_fields, "`unsafe` fields are experimental");

if !visitor.features.never_patterns() {
if let Some(spans) = spans.get(&sym::never_patterns) {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_expand/src/placeholders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use rustc_ast::mut_visit::*;
use rustc_ast::ptr::P;
use rustc_ast::token::Delimiter;
use rustc_ast::visit::AssocCtxt;
use rustc_ast::{self as ast};
use rustc_ast::{self as ast, Safety};
use rustc_data_structures::fx::FxHashMap;
use rustc_span::DUMMY_SP;
use rustc_span::symbol::Ident;
Expand Down Expand Up @@ -173,6 +173,7 @@ pub(crate) fn placeholder(
ty: ty(),
vis,
is_placeholder: true,
safety: Safety::Default,
}]),
AstFragmentKind::Variants => AstFragment::Variants(smallvec![ast::Variant {
attrs: Default::default(),
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_feature/src/unstable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,8 @@ declare_features! (
/// Allows creation of instances of a struct by moving fields that have
/// not changed from prior instances of the same struct (RFC #2528)
(unstable, type_changing_struct_update, "1.58.0", Some(86555)),
/// Allows declaring fields `unsafe`.
(incomplete, unsafe_fields, "CURRENT_RUSTC_VERSION", Some(132922)),
/// Allows const generic parameters to be defined with types that
/// are not `Sized`, e.g. `fn foo<const N: [u8]>() {`.
(incomplete, unsized_const_params, "1.82.0", Some(95174)),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3177,6 +3177,7 @@ pub struct FieldDef<'hir> {
pub hir_id: HirId,
pub def_id: LocalDefId,
pub ty: &'hir Ty<'hir>,
pub safety: Safety,
}

impl FieldDef<'_> {
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,13 @@ 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
104 changes: 72 additions & 32 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_data_structures::unord::{UnordMap, UnordSet};
use rustc_errors::MultiSpan;
use rustc_errors::codes::*;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::{Node, intravisit};
use rustc_hir::{Node, Safety, intravisit};
use rustc_infer::infer::{RegionVariableOrigin, TyCtxtInferExt};
use rustc_infer::traits::{Obligation, ObligationCauseCode};
use rustc_lint_defs::builtin::{
Expand Down Expand Up @@ -70,6 +70,7 @@ 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 All @@ -81,38 +82,45 @@ fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) {
check_packed(tcx, span, def);
}

fn allowed_union_or_unsafe_field<'tcx>(
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
span: Span,
) -> bool {
// We don't just accept all !needs_drop fields, due to semver concerns.
let allowed = match ty.kind() {
ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
ty::Tuple(tys) => {
// allow tuples of allowed types
tys.iter().all(|ty| allowed_union_or_unsafe_field(tcx, ty, typing_env, span))
}
ty::Array(elem, _len) => {
// Like `Copy`, we do *not* special-case length 0.
allowed_union_or_unsafe_field(tcx, *elem, typing_env, span)
}
_ => {
// Fallback case: allow `ManuallyDrop` and things that are `Copy`,
// also no need to report an error if the type is unresolved.
ty.ty_adt_def().is_some_and(|adt_def| adt_def.is_manually_drop())
|| ty.is_copy_modulo_regions(tcx, typing_env)
|| ty.references_error()
}
};
if allowed && ty.needs_drop(tcx, typing_env) {
// This should never happen. But we can get here e.g. in case of name resolution errors.
tcx.dcx()
.span_delayed_bug(span, "we should never accept maybe-dropping union or unsafe fields");
}
allowed
}

/// Check that the fields of the `union` do not need dropping.
fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> bool {
let item_type = tcx.type_of(item_def_id).instantiate_identity();
if let ty::Adt(def, args) = item_type.kind() {
assert!(def.is_union());

fn allowed_union_field<'tcx>(
ty: Ty<'tcx>,
tcx: TyCtxt<'tcx>,
typing_env: ty::TypingEnv<'tcx>,
) -> bool {
// We don't just accept all !needs_drop fields, due to semver concerns.
match ty.kind() {
ty::Ref(..) => true, // references never drop (even mutable refs, which are non-Copy and hence fail the later check)
ty::Tuple(tys) => {
// allow tuples of allowed types
tys.iter().all(|ty| allowed_union_field(ty, tcx, typing_env))
}
ty::Array(elem, _len) => {
// Like `Copy`, we do *not* special-case length 0.
allowed_union_field(*elem, tcx, typing_env)
}
_ => {
// Fallback case: allow `ManuallyDrop` and things that are `Copy`,
// also no need to report an error if the type is unresolved.
ty.ty_adt_def().is_some_and(|adt_def| adt_def.is_manually_drop())
|| ty.is_copy_modulo_regions(tcx, typing_env)
|| ty.references_error()
}
}
}

let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
for field in &def.non_enum_variant().fields {
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
Expand All @@ -121,7 +129,7 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> b
continue;
};

if !allowed_union_field(field_ty, tcx, typing_env) {
if !allowed_union_or_unsafe_field(tcx, field_ty, typing_env, span) {
let (field_span, ty_span) = match tcx.hir().get_if_local(field.did) {
// We are currently checking the type this field came from, so it must be local.
Some(Node::Field(field)) => (field.span, field.ty.span),
Expand All @@ -136,10 +144,6 @@ fn check_union_fields(tcx: TyCtxt<'_>, span: Span, item_def_id: LocalDefId) -> b
note: (),
});
return false;
} else if field_ty.needs_drop(tcx, typing_env) {
// This should never happen. But we can get here e.g. in case of name resolution errors.
tcx.dcx()
.span_delayed_bug(span, "we should never accept maybe-dropping union fields");
}
}
} else {
Expand All @@ -148,6 +152,41 @@ 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 item_type = tcx.type_of(item_def_id).instantiate_identity();
let ty::Adt(def, args) = item_type.kind() else {
span_bug!(span, "structs/enums must be ty::Adt, but got {:?}", item_type.kind());
};
let typing_env = ty::TypingEnv::non_body_analysis(tcx, item_def_id);
for field in def.all_fields() {
if field.safety != Safety::Unsafe {
continue;
}
let Ok(field_ty) = tcx.try_normalize_erasing_regions(typing_env, field.ty(tcx, args))
else {
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
continue;
};

if !allowed_union_or_unsafe_field(tcx, field_ty, 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 @@ -1464,6 +1503,7 @@ 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
1 change: 1 addition & 0 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1040,6 +1040,7 @@ fn lower_variant(
did: f.def_id.to_def_id(),
name: f.ident.name,
vis: tcx.visibility(f.def_id),
safety: f.safety,
})
.collect();
let recovered = match def {
Expand Down
23 changes: 23 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,17 @@ 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 @@ -755,6 +766,18 @@ 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
6 changes: 6 additions & 0 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rustc_data_structures::sync::{Lock, Lrc, OnceLock};
use rustc_data_structures::unhash::UnhashMap;
use rustc_expand::base::{SyntaxExtension, SyntaxExtensionKind};
use rustc_expand::proc_macro::{AttrProcMacro, BangProcMacro, DeriveProcMacro};
use rustc_hir::Safety;
use rustc_hir::def::Res;
use rustc_hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE};
use rustc_hir::definitions::{DefPath, DefPathData};
Expand Down Expand Up @@ -1101,6 +1102,7 @@ impl<'a> CrateMetadataRef<'a> {
did,
name: self.item_name(did.index),
vis: self.get_visibility(did.index),
safety: self.get_safety(did.index),
})
.collect(),
adt_kind,
Expand Down Expand Up @@ -1162,6 +1164,10 @@ impl<'a> CrateMetadataRef<'a> {
.map_id(|index| self.local_def_id(index))
}

fn get_safety(self, id: DefIndex) -> Safety {
self.root.tables.safety.get(self, id).unwrap_or_else(|| self.missing("safety", id))
}

fn get_trait_item_def_id(self, id: DefIndex) -> Option<DefId> {
self.root.tables.trait_item_def_id.get(self, id).map(|d| d.decode_from_cdata(self))
}
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1599,6 +1599,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
f.did.index
}));

for field in &variant.fields {
self.tables.safety.set_some(field.did.index, field.safety);
veluca93 marked this conversation as resolved.
Show resolved Hide resolved
}

if let Some((CtorKind::Fn, ctor_def_id)) = variant.ctor {
let fn_sig = tcx.fn_sig(ctor_def_id);
// FIXME only encode signature for ctor_def_id
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,6 +411,7 @@ define_tables! {
associated_item_or_field_def_ids: Table<DefIndex, LazyArray<DefIndex>>,
def_kind: Table<DefIndex, DefKind>,
visibility: Table<DefIndex, LazyValue<ty::Visibility<DefIndex>>>,
safety: Table<DefIndex, hir::Safety>,
def_span: Table<DefIndex, LazyValue<Span>>,
def_ident_span: Table<DefIndex, LazyValue<Span>>,
lookup_stability: Table<DefIndex, LazyValue<attr::Stability>>,
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_metadata/src/rmeta/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,13 @@ fixed_size_enum! {
}
}

fixed_size_enum! {
hir::Safety {
( Unsafe )
( Safe )
}
}

fixed_size_enum! {
ty::Asyncness {
( Yes )
Expand Down
Loading