Skip to content

Commit

Permalink
Draft implementation of the unsafe-fields RFC.
Browse files Browse the repository at this point in the history
Co-Authored-By: Jacob Pratt <jacob@jhpratt.dev>
  • Loading branch information
veluca93 and jhpratt committed Nov 11, 2024
1 parent d4822c2 commit d25841e
Show file tree
Hide file tree
Showing 32 changed files with 533 additions and 79 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3036,6 +3036,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 @@ -1044,10 +1044,11 @@ pub fn walk_flat_map_field_def<T: MutVisitor>(
visitor: &mut T,
mut fd: FieldDef,
) -> SmallVec<[FieldDef; 1]> {
let FieldDef { span, ident, vis, id, ty, attrs, is_placeholder: _ } = &mut fd;
let FieldDef { span, ident, vis, id, ty, attrs, is_placeholder: _, safety } = &mut 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 @@ -929,7 +929,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 @@ -548,6 +548,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 @@ -623,6 +623,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(1234567)), // FIXME: proper tracking issue.
/// 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
105 changes: 77 additions & 28 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 @@ -71,6 +71,7 @@ fn check_struct(tcx: TyCtxt<'_>, def_id: LocalDefId) {

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

fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) {
Expand All @@ -82,38 +83,38 @@ fn check_union(tcx: TyCtxt<'_>, def_id: LocalDefId) {
check_packed(tcx, span, def);
}

fn allowed_union_or_unsafe_field<'tcx>(
ty: Ty<'tcx>,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'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_or_unsafe_field(ty, tcx, param_env))
}
ty::Array(elem, _len) => {
// Like `Copy`, we do *not* special-case length 0.
allowed_union_or_unsafe_field(*elem, tcx, param_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, param_env)
|| ty.references_error()
}
}
}

/// 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>,
param_env: ty::ParamEnv<'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, param_env))
}
ty::Array(elem, _len) => {
// Like `Copy`, we do *not* special-case length 0.
allowed_union_field(*elem, tcx, param_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, param_env)
|| ty.references_error()
}
}
}

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

if !allowed_union_field(field_ty, tcx, param_env) {
if !allowed_union_or_unsafe_field(field_ty, tcx, param_env) {
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 @@ -149,6 +150,53 @@ 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<'_>, 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() {
let param_env = tcx.param_env(item_def_id);
for variant in def.variants() {
for field in &variant.fields {
if field.safety != Safety::Unsafe {
continue;
}
let Ok(field_ty) =
tcx.try_normalize_erasing_regions(param_env, field.ty(tcx, args))
else {
tcx.dcx().span_delayed_bug(span, "could not normalize field type");
continue;
};

if !allowed_union_or_unsafe_field(field_ty, tcx, param_env) {
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),
_ => unreachable!("mir field has to correspond to hir field"),
};
tcx.dcx().emit_err(errors::InvalidUnsafeField {
field_span,
sugg: errors::InvalidUnsafeFieldSuggestion {
lo: ty_span.shrink_to_lo(),
hi: ty_span.shrink_to_hi(),
},
note: (),
});
return false;
} else if field_ty.needs_drop(tcx, param_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 {
span_bug!(span, "structs/enums must be ty::Adt, but got {:?}", item_type.kind());
}
true
}

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

detect_discriminant_duplicate(tcx, def);
check_transparent(tcx, def);
check_unsafe_fields(tcx, tcx.def_span(def_id), 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 @@ -1031,6 +1031,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
5 changes: 5 additions & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1595,6 +1595,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
f.did.index
}));

for field in &variant.fields {
// FIXME: this probably blows up rmeta size.
self.tables.safety.set_some(field.did.index, field.safety);
}

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

0 comments on commit d25841e

Please sign in to comment.