Skip to content

Commit 2d28b63

Browse files
committed
Auto merge of #124482 - spastorino:unsafe-extern-blocks, r=oli-obk
Unsafe extern blocks This implements RFC 3484. Tracking issue #123743 and RFC rust-lang/rfcs#3484 This is better reviewed commit by commit.
2 parents 2b6a342 + 525828d commit 2d28b63

File tree

93 files changed

+714
-152
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

93 files changed

+714
-152
lines changed

compiler/rustc_ast/src/ast.rs

+6
Original file line numberDiff line numberDiff line change
@@ -2501,6 +2501,8 @@ pub enum IsAuto {
25012501
pub enum Safety {
25022502
/// `unsafe` an item is explicitly marked as `unsafe`.
25032503
Unsafe(Span),
2504+
/// `safe` an item is explicitly marked as `safe`.
2505+
Safe(Span),
25042506
/// Default means no value was provided, it will take a default value given the context in
25052507
/// which is used.
25062508
Default,
@@ -3162,6 +3164,7 @@ pub struct DelegationMac {
31623164
#[derive(Clone, Encodable, Decodable, Debug)]
31633165
pub struct StaticItem {
31643166
pub ty: P<Ty>,
3167+
pub safety: Safety,
31653168
pub mutability: Mutability,
31663169
pub expr: Option<P<Expr>>,
31673170
}
@@ -3171,6 +3174,7 @@ pub struct StaticItem {
31713174
#[derive(Clone, Encodable, Decodable, Debug)]
31723175
pub struct StaticForeignItem {
31733176
pub ty: P<Ty>,
3177+
pub safety: Safety,
31743178
pub mutability: Mutability,
31753179
pub expr: Option<P<Expr>>,
31763180
}
@@ -3179,6 +3183,7 @@ impl From<StaticItem> for StaticForeignItem {
31793183
fn from(static_item: StaticItem) -> StaticForeignItem {
31803184
StaticForeignItem {
31813185
ty: static_item.ty,
3186+
safety: static_item.safety,
31823187
mutability: static_item.mutability,
31833188
expr: static_item.expr,
31843189
}
@@ -3189,6 +3194,7 @@ impl From<StaticForeignItem> for StaticItem {
31893194
fn from(static_item: StaticForeignItem) -> StaticItem {
31903195
StaticItem {
31913196
ty: static_item.ty,
3197+
safety: static_item.safety,
31923198
mutability: static_item.mutability,
31933199
expr: static_item.expr,
31943200
}

compiler/rustc_ast/src/mut_visit.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,7 @@ fn visit_defaultness<T: MutVisitor>(defaultness: &mut Defaultness, vis: &mut T)
862862
fn visit_safety<T: MutVisitor>(safety: &mut Safety, vis: &mut T) {
863863
match safety {
864864
Safety::Unsafe(span) => vis.visit_span(span),
865+
Safety::Safe(span) => vis.visit_span(span),
865866
Safety::Default => {}
866867
}
867868
}
@@ -1079,7 +1080,7 @@ impl NoopVisitItemKind for ItemKind {
10791080
match self {
10801081
ItemKind::ExternCrate(_orig_name) => {}
10811082
ItemKind::Use(use_tree) => vis.visit_use_tree(use_tree),
1082-
ItemKind::Static(box StaticItem { ty, mutability: _, expr }) => {
1083+
ItemKind::Static(box StaticItem { ty, safety: _, mutability: _, expr }) => {
10831084
vis.visit_ty(ty);
10841085
visit_opt(expr, |expr| vis.visit_expr(expr));
10851086
}
@@ -1289,7 +1290,12 @@ pub fn noop_flat_map_item<K: NoopVisitItemKind>(
12891290
impl NoopVisitItemKind for ForeignItemKind {
12901291
fn noop_visit(&mut self, visitor: &mut impl MutVisitor) {
12911292
match self {
1292-
ForeignItemKind::Static(box StaticForeignItem { ty, mutability: _, expr }) => {
1293+
ForeignItemKind::Static(box StaticForeignItem {
1294+
ty,
1295+
mutability: _,
1296+
expr,
1297+
safety: _,
1298+
}) => {
12931299
visitor.visit_ty(ty);
12941300
visit_opt(expr, |expr| visitor.visit_expr(expr));
12951301
}

compiler/rustc_ast/src/token.rs

+2
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ pub fn ident_can_begin_expr(name: Symbol, span: Span, is_raw: IdentIsRaw) -> boo
210210
kw::Unsafe,
211211
kw::While,
212212
kw::Yield,
213+
kw::Safe,
213214
kw::Static,
214215
]
215216
.contains(&name)
@@ -577,6 +578,7 @@ impl Token {
577578
kw::Impl,
578579
kw::Unsafe,
579580
kw::Const,
581+
kw::Safe,
580582
kw::Static,
581583
kw::Union,
582584
kw::Macro,

compiler/rustc_ast/src/visit.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ impl WalkItemKind for ItemKind {
334334
match self {
335335
ItemKind::ExternCrate(_) => {}
336336
ItemKind::Use(use_tree) => try_visit!(visitor.visit_use_tree(use_tree, item.id, false)),
337-
ItemKind::Static(box StaticItem { ty, mutability: _, expr }) => {
337+
ItemKind::Static(box StaticItem { ty, safety: _, mutability: _, expr }) => {
338338
try_visit!(visitor.visit_ty(ty));
339339
visit_opt!(visitor, visit_expr, expr);
340340
}
@@ -658,7 +658,12 @@ impl WalkItemKind for ForeignItemKind {
658658
) -> V::Result {
659659
let &Item { id, span, ident, ref vis, .. } = item;
660660
match self {
661-
ForeignItemKind::Static(box StaticForeignItem { ty, mutability: _, expr }) => {
661+
ForeignItemKind::Static(box StaticForeignItem {
662+
ty,
663+
mutability: _,
664+
expr,
665+
safety: _,
666+
}) => {
662667
try_visit!(visitor.visit_ty(ty));
663668
visit_opt!(visitor, visit_expr, expr);
664669
}

compiler/rustc_ast_lowering/src/item.rs

+18-9
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
181181

182182
self.lower_use_tree(use_tree, &prefix, id, vis_span, ident, attrs)
183183
}
184-
ItemKind::Static(box ast::StaticItem { ty: t, mutability: m, expr: e }) => {
184+
ItemKind::Static(box ast::StaticItem { ty: t, safety: _, mutability: m, expr: e }) => {
185185
let (ty, body_id) =
186186
self.lower_const_item(t, span, e.as_deref(), ImplTraitPosition::StaticTy);
187187
hir::ItemKind::Static(ty, *m, body_id)
@@ -388,7 +388,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
388388
ImplPolarity::Negative(s) => ImplPolarity::Negative(self.lower_span(*s)),
389389
};
390390
hir::ItemKind::Impl(self.arena.alloc(hir::Impl {
391-
safety: self.lower_safety(*safety),
391+
safety: self.lower_safety(*safety, hir::Safety::Safe),
392392
polarity,
393393
defaultness,
394394
defaultness_span,
@@ -418,7 +418,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
418418
let items = this.arena.alloc_from_iter(
419419
items.iter().map(|item| this.lower_trait_item_ref(item)),
420420
);
421-
let safety = this.lower_safety(*safety);
421+
let safety = this.lower_safety(*safety, hir::Safety::Safe);
422422
(safety, items, bounds)
423423
},
424424
);
@@ -660,13 +660,21 @@ impl<'hir> LoweringContext<'_, 'hir> {
660660
this.lower_fn_params_to_names(fdec),
661661
)
662662
});
663+
let safety = self.lower_safety(sig.header.safety, hir::Safety::Unsafe);
663664

664-
hir::ForeignItemKind::Fn(fn_dec, fn_args, generics)
665+
hir::ForeignItemKind::Fn(fn_dec, fn_args, generics, safety)
665666
}
666-
ForeignItemKind::Static(box StaticForeignItem { ty, mutability, expr: _ }) => {
667+
ForeignItemKind::Static(box StaticForeignItem {
668+
ty,
669+
mutability,
670+
expr: _,
671+
safety,
672+
}) => {
667673
let ty = self
668674
.lower_ty(ty, ImplTraitContext::Disallowed(ImplTraitPosition::StaticTy));
669-
hir::ForeignItemKind::Static(ty, *mutability)
675+
let safety = self.lower_safety(*safety, hir::Safety::Unsafe);
676+
677+
hir::ForeignItemKind::Static(ty, *mutability, safety)
670678
}
671679
ForeignItemKind::TyAlias(..) => hir::ForeignItemKind::Type,
672680
ForeignItemKind::MacCall(_) => panic!("macro shouldn't exist here"),
@@ -1360,7 +1368,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
13601368
hir::IsAsync::NotAsync
13611369
};
13621370
hir::FnHeader {
1363-
safety: self.lower_safety(h.safety),
1371+
safety: self.lower_safety(h.safety, hir::Safety::Safe),
13641372
asyncness: asyncness,
13651373
constness: self.lower_constness(h.constness),
13661374
abi: self.lower_extern(h.ext),
@@ -1410,10 +1418,11 @@ impl<'hir> LoweringContext<'_, 'hir> {
14101418
}
14111419
}
14121420

1413-
pub(super) fn lower_safety(&mut self, s: Safety) -> hir::Safety {
1421+
pub(super) fn lower_safety(&mut self, s: Safety, default: hir::Safety) -> hir::Safety {
14141422
match s {
14151423
Safety::Unsafe(_) => hir::Safety::Unsafe,
1416-
Safety::Default => hir::Safety::Safe,
1424+
Safety::Default => default,
1425+
Safety::Safe(_) => hir::Safety::Safe,
14171426
}
14181427
}
14191428

compiler/rustc_ast_lowering/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1321,7 +1321,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
13211321
let generic_params = self.lower_lifetime_binder(t.id, &f.generic_params);
13221322
hir::TyKind::BareFn(self.arena.alloc(hir::BareFnTy {
13231323
generic_params,
1324-
safety: self.lower_safety(f.safety),
1324+
safety: self.lower_safety(f.safety, hir::Safety::Safe),
13251325
abi: self.lower_extern(f.ext),
13261326
decl: self.lower_fn_decl(&f.decl, t.id, t.span, FnDeclKind::Pointer, None),
13271327
param_names: self.lower_fn_params_to_names(&f.decl),

compiler/rustc_ast_passes/messages.ftl

+5
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ ast_passes_extern_fn_qualifiers = functions in `extern` blocks cannot have quali
6767
.label = in this `extern` block
6868
.suggestion = remove this qualifier
6969
70+
ast_passes_extern_invalid_safety = items in unadorned `extern` blocks cannot have safety qualifiers
71+
.suggestion = add unsafe to this `extern` block
72+
7073
ast_passes_extern_item_ascii = items in `extern` blocks cannot use non-ascii identifiers
7174
.label = in this `extern` block
7275
.note = this limitation may be lifted in the future; see issue #83942 <https://github.com/rust-lang/rust/issues/83942> for more information
@@ -174,6 +177,8 @@ ast_passes_match_arm_with_no_body =
174177
`match` arm with no body
175178
.suggestion = add a body after the pattern
176179
180+
ast_passes_missing_unsafe_on_extern = extern blocks must be unsafe
181+
177182
ast_passes_module_nonascii = trying to load file for module `{$name}` with non-ascii identifier name
178183
.help = consider using the `#[path]` attribute to specify filesystem path
179184

compiler/rustc_ast_passes/src/ast_validation.rs

+61-20
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ use rustc_data_structures::fx::FxIndexMap;
1515
use rustc_feature::Features;
1616
use rustc_parse::validate_attr;
1717
use rustc_session::lint::builtin::{
18-
DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, PATTERNS_IN_FNS_WITHOUT_BODY,
18+
DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, MISSING_UNSAFE_ON_EXTERN,
19+
PATTERNS_IN_FNS_WITHOUT_BODY,
1920
};
2021
use rustc_session::lint::{BuiltinLintDiag, LintBuffer};
2122
use rustc_session::Session;
@@ -86,6 +87,9 @@ struct AstValidator<'a> {
8687
/// or `Foo::Bar<impl Trait>`
8788
is_impl_trait_banned: bool,
8889

90+
/// Used to ban explicit safety on foreign items when the extern block is not marked as unsafe.
91+
extern_mod_safety: Option<Safety>,
92+
8993
lint_buffer: &'a mut LintBuffer,
9094
}
9195

@@ -116,6 +120,12 @@ impl<'a> AstValidator<'a> {
116120
self.outer_trait_or_trait_impl = old;
117121
}
118122

123+
fn with_in_extern_mod(&mut self, extern_mod_safety: Safety, f: impl FnOnce(&mut Self)) {
124+
let old = mem::replace(&mut self.extern_mod_safety, Some(extern_mod_safety));
125+
f(self);
126+
self.extern_mod_safety = old;
127+
}
128+
119129
fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) {
120130
let old = mem::replace(&mut self.is_impl_trait_banned, true);
121131
f(self);
@@ -429,6 +439,18 @@ impl<'a> AstValidator<'a> {
429439
}
430440
}
431441

442+
fn check_foreign_item_safety(&self, item_span: Span, safety: Safety) {
443+
if matches!(safety, Safety::Unsafe(_) | Safety::Safe(_))
444+
&& (self.extern_mod_safety == Some(Safety::Default)
445+
|| !self.features.unsafe_extern_blocks)
446+
{
447+
self.dcx().emit_err(errors::InvalidSafetyOnExtern {
448+
item_span,
449+
block: self.current_extern_span(),
450+
});
451+
}
452+
}
453+
432454
fn check_defaultness(&self, span: Span, defaultness: Defaultness) {
433455
if let Defaultness::Default(def_span) = defaultness {
434456
let span = self.session.source_map().guess_head_span(span);
@@ -518,18 +540,14 @@ impl<'a> AstValidator<'a> {
518540
fn check_foreign_fn_headerless(
519541
&self,
520542
// Deconstruct to ensure exhaustiveness
521-
FnHeader { safety, coroutine_kind, constness, ext }: FnHeader,
543+
FnHeader { safety: _, coroutine_kind, constness, ext }: FnHeader,
522544
) {
523545
let report_err = |span| {
524546
self.dcx().emit_err(errors::FnQualifierInExtern {
525547
span: span,
526548
block: self.current_extern_span(),
527549
});
528550
};
529-
match safety {
530-
Safety::Unsafe(span) => report_err(span),
531-
Safety::Default => (),
532-
}
533551
match coroutine_kind {
534552
Some(knd) => report_err(knd.span()),
535553
None => (),
@@ -1017,19 +1035,39 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10171035
return; // Avoid visiting again.
10181036
}
10191037
ItemKind::ForeignMod(ForeignMod { abi, safety, .. }) => {
1020-
let old_item = mem::replace(&mut self.extern_mod, Some(item));
1021-
self.visibility_not_permitted(
1022-
&item.vis,
1023-
errors::VisibilityNotPermittedNote::IndividualForeignItems,
1024-
);
1025-
if let &Safety::Unsafe(span) = safety {
1026-
self.dcx().emit_err(errors::UnsafeItem { span, kind: "extern block" });
1027-
}
1028-
if abi.is_none() {
1029-
self.maybe_lint_missing_abi(item.span, item.id);
1030-
}
1031-
visit::walk_item(self, item);
1032-
self.extern_mod = old_item;
1038+
self.with_in_extern_mod(*safety, |this| {
1039+
let old_item = mem::replace(&mut this.extern_mod, Some(item));
1040+
this.visibility_not_permitted(
1041+
&item.vis,
1042+
errors::VisibilityNotPermittedNote::IndividualForeignItems,
1043+
);
1044+
1045+
if this.features.unsafe_extern_blocks {
1046+
if &Safety::Default == safety {
1047+
if item.span.at_least_rust_2024() {
1048+
this.dcx()
1049+
.emit_err(errors::MissingUnsafeOnExtern { span: item.span });
1050+
} else {
1051+
this.lint_buffer.buffer_lint(
1052+
MISSING_UNSAFE_ON_EXTERN,
1053+
item.id,
1054+
item.span,
1055+
BuiltinLintDiag::MissingUnsafeOnExtern {
1056+
suggestion: item.span.shrink_to_lo(),
1057+
},
1058+
);
1059+
}
1060+
}
1061+
} else if let &Safety::Unsafe(span) = safety {
1062+
this.dcx().emit_err(errors::UnsafeItem { span, kind: "extern block" });
1063+
}
1064+
1065+
if abi.is_none() {
1066+
this.maybe_lint_missing_abi(item.span, item.id);
1067+
}
1068+
visit::walk_item(this, item);
1069+
this.extern_mod = old_item;
1070+
});
10331071
return; // Avoid visiting again.
10341072
}
10351073
ItemKind::Enum(def, _) => {
@@ -1161,6 +1199,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
11611199
fn visit_foreign_item(&mut self, fi: &'a ForeignItem) {
11621200
match &fi.kind {
11631201
ForeignItemKind::Fn(box Fn { defaultness, sig, body, .. }) => {
1202+
self.check_foreign_item_safety(fi.span, sig.header.safety);
11641203
self.check_defaultness(fi.span, *defaultness);
11651204
self.check_foreign_fn_bodyless(fi.ident, body.as_deref());
11661205
self.check_foreign_fn_headerless(sig.header);
@@ -1180,7 +1219,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
11801219
self.check_foreign_ty_genericless(generics, where_clauses);
11811220
self.check_foreign_item_ascii_only(fi.ident);
11821221
}
1183-
ForeignItemKind::Static(box StaticForeignItem { ty: _, mutability: _, expr }) => {
1222+
ForeignItemKind::Static(box StaticForeignItem { expr, safety, .. }) => {
1223+
self.check_foreign_item_safety(fi.span, *safety);
11841224
self.check_foreign_kind_bodyless(fi.ident, "static", expr.as_ref().map(|b| b.span));
11851225
self.check_foreign_item_ascii_only(fi.ident);
11861226
}
@@ -1736,6 +1776,7 @@ pub fn check_crate(
17361776
outer_impl_trait: None,
17371777
disallow_tilde_const: Some(DisallowTildeConstContext::Item),
17381778
is_impl_trait_banned: false,
1779+
extern_mod_safety: None,
17391780
lint_buffer: lints,
17401781
};
17411782
visit::walk_crate(&mut validator, krate);

compiler/rustc_ast_passes/src/errors.rs

+16
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,15 @@ pub enum ExternBlockSuggestion {
216216
},
217217
}
218218

219+
#[derive(Diagnostic)]
220+
#[diag(ast_passes_extern_invalid_safety)]
221+
pub struct InvalidSafetyOnExtern {
222+
#[primary_span]
223+
pub item_span: Span,
224+
#[suggestion(code = "", applicability = "maybe-incorrect")]
225+
pub block: Span,
226+
}
227+
219228
#[derive(Diagnostic)]
220229
#[diag(ast_passes_bound_in_context)]
221230
pub struct BoundInContext<'a> {
@@ -485,6 +494,13 @@ pub struct UnsafeItem {
485494
pub kind: &'static str,
486495
}
487496

497+
#[derive(Diagnostic)]
498+
#[diag(ast_passes_missing_unsafe_on_extern)]
499+
pub struct MissingUnsafeOnExtern {
500+
#[primary_span]
501+
pub span: Span,
502+
}
503+
488504
#[derive(Diagnostic)]
489505
#[diag(ast_passes_fieldless_union)]
490506
pub struct FieldlessUnion {

0 commit comments

Comments
 (0)