Fail when using safe/unsafe items inside unadorned extern blocks

spastorino · spastorino · commit b4cbdb7246c6 · 2024-06-04T14:19:43.000-03:00
diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl
@@ -67,6 +67,9 @@ ast_passes_extern_fn_qualifiers = functions in `extern` blocks cannot have quali
     .label = in this `extern` block
     .suggestion = remove this qualifier
 
+ast_passes_extern_invalid_safety = items in unadorned `extern` blocks cannot have safety qualifiers
+    .suggestion = add unsafe to this `extern` block
+
 ast_passes_extern_item_ascii = items in `extern` blocks cannot use non-ascii identifiers
     .label = in this `extern` block
     .note = this limitation may be lifted in the future; see issue #83942 <https://github.com/rust-lang/rust/issues/83942> for more information
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -87,6 +87,9 @@ struct AstValidator<'a> {
     /// or `Foo::Bar<impl Trait>`
     is_impl_trait_banned: bool,
 
+    /// Used to ban explicit safety on foreign items when the extern block is not marked as unsafe.
+    extern_mod_safety: Option<Safety>,
+
     lint_buffer: &'a mut LintBuffer,
 }
 
@@ -117,6 +120,12 @@ impl<'a> AstValidator<'a> {
         self.outer_trait_or_trait_impl = old;
     }
 
+    fn with_in_extern_mod(&mut self, extern_mod_safety: Safety, f: impl FnOnce(&mut Self)) {
+        let old = mem::replace(&mut self.extern_mod_safety, Some(extern_mod_safety));
+        f(self);
+        self.extern_mod_safety = old;
+    }
+
     fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) {
         let old = mem::replace(&mut self.is_impl_trait_banned, true);
         f(self);
@@ -430,6 +439,20 @@ impl<'a> AstValidator<'a> {
         }
     }
 
+    fn check_foreign_item_safety(&self, item_span: Span, safety: Safety) {
+        match safety {
+            Safety::Unsafe(_) | Safety::Safe(_)
+                if self.extern_mod_safety == Some(Safety::Default) =>
+            {
+                self.dcx().emit_err(errors::InvalidSafetyOnExtern {
+                    item_span,
+                    block: self.current_extern_span(),
+                });
+            }
+            _ => {}
+        }
+    }
+
     fn check_defaultness(&self, span: Span, defaultness: Defaultness) {
         if let Defaultness::Default(def_span) = defaultness {
             let span = self.session.source_map().guess_head_span(span);
@@ -1014,26 +1037,28 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 return; // Avoid visiting again.
             }
             ItemKind::ForeignMod(ForeignMod { abi, safety, .. }) => {
-                let old_item = mem::replace(&mut self.extern_mod, Some(item));
-                self.visibility_not_permitted(
-                    &item.vis,
-                    errors::VisibilityNotPermittedNote::IndividualForeignItems,
-                );
-
-                if &Safety::Default == safety {
-                    self.lint_buffer.buffer_lint(
-                        MISSING_UNSAFE_ON_EXTERN,
-                        item.id,
-                        item.span,
-                        BuiltinLintDiag::MissingUnsafeOnExtern,
+                self.with_in_extern_mod(*safety, |this| {
+                    let old_item = mem::replace(&mut this.extern_mod, Some(item));
+                    this.visibility_not_permitted(
+                        &item.vis,
+                        errors::VisibilityNotPermittedNote::IndividualForeignItems,
                     );
-                }
 
-                if abi.is_none() {
-                    self.maybe_lint_missing_abi(item.span, item.id);
-                }
-                visit::walk_item(self, item);
-                self.extern_mod = old_item;
+                    if &Safety::Default == safety {
+                        this.lint_buffer.buffer_lint(
+                            MISSING_UNSAFE_ON_EXTERN,
+                            item.id,
+                            item.span,
+                            BuiltinLintDiag::MissingUnsafeOnExtern,
+                        );
+                    }
+
+                    if abi.is_none() {
+                        this.maybe_lint_missing_abi(item.span, item.id);
+                    }
+                    visit::walk_item(this, item);
+                    this.extern_mod = old_item;
+                });
                 return; // Avoid visiting again.
             }
             ItemKind::Enum(def, _) => {
@@ -1165,6 +1190,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     fn visit_foreign_item(&mut self, fi: &'a ForeignItem) {
         match &fi.kind {
             ForeignItemKind::Fn(box Fn { defaultness, sig, body, .. }) => {
+                self.check_foreign_item_safety(fi.span, sig.header.safety);
                 self.check_defaultness(fi.span, *defaultness);
                 self.check_foreign_fn_bodyless(fi.ident, body.as_deref());
                 self.check_foreign_fn_headerless(sig.header);
@@ -1184,7 +1210,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 self.check_foreign_ty_genericless(generics, where_clauses);
                 self.check_foreign_item_ascii_only(fi.ident);
             }
-            ForeignItemKind::Static(box StaticForeignItem { expr, .. }) => {
+            ForeignItemKind::Static(box StaticForeignItem { expr, safety, .. }) => {
+                self.check_foreign_item_safety(fi.span, *safety);
                 self.check_foreign_kind_bodyless(fi.ident, "static", expr.as_ref().map(|b| b.span));
                 self.check_foreign_item_ascii_only(fi.ident);
             }
@@ -1740,6 +1767,7 @@ pub fn check_crate(
         outer_impl_trait: None,
         disallow_tilde_const: Some(DisallowTildeConstContext::Item),
         is_impl_trait_banned: false,
+        extern_mod_safety: None,
         lint_buffer: lints,
     };
     visit::walk_crate(&mut validator, krate);
diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs
@@ -216,6 +216,15 @@ pub enum ExternBlockSuggestion {
     },
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_extern_invalid_safety)]
+pub struct InvalidSafetyOnExtern {
+    #[primary_span]
+    pub item_span: Span,
+    #[suggestion(code = "", applicability = "maybe-incorrect")]
+    pub block: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_bound_in_context)]
 pub struct BoundInContext<'a> {
diff --git a/tests/ui/parser/fn-header-semantic-fail.rs b/tests/ui/parser/fn-header-semantic-fail.rs
@@ -44,13 +44,14 @@ fn main() {
 
     extern "C" {
         async fn fe1(); //~ ERROR functions in `extern` blocks cannot have qualifiers
-        unsafe fn fe2();
+        unsafe fn fe2(); //~ ERROR items in unadorned `extern` blocks cannot have safety qualifiers
         const fn fe3(); //~ ERROR functions in `extern` blocks cannot have qualifiers
         extern "C" fn fe4(); //~ ERROR functions in `extern` blocks cannot have qualifiers
         const async unsafe extern "C" fn fe5();
         //~^ ERROR functions in `extern` blocks
         //~| ERROR functions in `extern` blocks
         //~| ERROR functions in `extern` blocks
         //~| ERROR functions cannot be both `const` and `async`
+        //~| ERROR items in unadorned `extern` blocks cannot have safety qualifiers
     }
 }
diff --git a/tests/ui/parser/fn-header-semantic-fail.stderr b/tests/ui/parser/fn-header-semantic-fail.stderr
@@ -78,6 +78,15 @@ LL |     extern "C" {
 LL |         async fn fe1();
    |         ^^^^^ help: remove this qualifier
 
+error: items in unadorned `extern` blocks cannot have safety qualifiers
+  --> $DIR/fn-header-semantic-fail.rs:47:9
+   |
+LL |     extern "C" {
+   |     ---------- help: add unsafe to this `extern` block
+LL |         async fn fe1();
+LL |         unsafe fn fe2();
+   |         ^^^^^^^^^^^^^^^^
+
 error: functions in `extern` blocks cannot have qualifiers
   --> $DIR/fn-header-semantic-fail.rs:48:9
    |
@@ -96,6 +105,15 @@ LL |     extern "C" {
 LL |         extern "C" fn fe4();
    |         ^^^^^^^^^^ help: remove this qualifier
 
+error: items in unadorned `extern` blocks cannot have safety qualifiers
+  --> $DIR/fn-header-semantic-fail.rs:50:9
+   |
+LL |     extern "C" {
+   |     ---------- help: add unsafe to this `extern` block
+...
+LL |         const async unsafe extern "C" fn fe5();
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
 error: functions in `extern` blocks cannot have qualifiers
   --> $DIR/fn-header-semantic-fail.rs:50:15
    |
@@ -132,6 +150,6 @@ LL |         const async unsafe extern "C" fn fe5();
    |         |     `async` because of this
    |         `const` because of this
 
-error: aborting due to 15 previous errors
+error: aborting due to 17 previous errors
 
 For more information about this error, try `rustc --explain E0379`.
diff --git a/tests/ui/parser/no-const-fn-in-extern-block.rs b/tests/ui/parser/no-const-fn-in-extern-block.rs
@@ -3,6 +3,7 @@ extern "C" {
     //~^ ERROR functions in `extern` blocks cannot have qualifiers
     const unsafe fn bar();
     //~^ ERROR functions in `extern` blocks cannot have qualifiers
+    //~| ERROR items in unadorned `extern` blocks cannot have safety qualifiers
 }
 
 fn main() {}
diff --git a/tests/ui/parser/no-const-fn-in-extern-block.stderr b/tests/ui/parser/no-const-fn-in-extern-block.stderr
@@ -6,6 +6,15 @@ LL | extern "C" {
 LL |     const fn foo();
    |     ^^^^^ help: remove this qualifier
 
+error: items in unadorned `extern` blocks cannot have safety qualifiers
+  --> $DIR/no-const-fn-in-extern-block.rs:4:5
+   |
+LL | extern "C" {
+   | ---------- help: add unsafe to this `extern` block
+...
+LL |     const unsafe fn bar();
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
 error: functions in `extern` blocks cannot have qualifiers
   --> $DIR/no-const-fn-in-extern-block.rs:4:5
    |
@@ -15,5 +24,5 @@ LL | extern "C" {
 LL |     const unsafe fn bar();
    |     ^^^^^ help: remove this qualifier
 
-error: aborting due to 2 previous errors
+error: aborting due to 3 previous errors
 
diff --git a/tests/ui/rust-2024/unsafe-extern-blocks/safe-unsafe-on-unadorned-extern-block.rs b/tests/ui/rust-2024/unsafe-extern-blocks/safe-unsafe-on-unadorned-extern-block.rs
@@ -0,0 +1,10 @@
+extern "C" {
+    safe fn test1(i: i32);
+    //~^ ERROR items in unadorned `extern` blocks cannot have safety qualifiers
+}
+
+fn test2(i: i32) {
+    test1(i);
+}
+
+fn main() {}
diff --git a/tests/ui/rust-2024/unsafe-extern-blocks/safe-unsafe-on-unadorned-extern-block.stderr b/tests/ui/rust-2024/unsafe-extern-blocks/safe-unsafe-on-unadorned-extern-block.stderr
@@ -0,0 +1,10 @@
+error: items in unadorned `extern` blocks cannot have safety qualifiers
+  --> $DIR/safe-unsafe-on-unadorned-extern-block.rs:2:5
+   |
+LL | extern "C" {
+   | ---------- help: add unsafe to this `extern` block
+LL |     safe fn test1(i: i32);
+   |     ^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+

Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@ extern "C" {`
`3`	`3`	//~^ ERROR functions in `extern` blocks cannot have qualifiers
`4`	`4`	`const unsafe fn bar();`
`5`	`5`	//~^ ERROR functions in `extern` blocks cannot have qualifiers
	`6`	+ //~\| ERROR items in unadorned `extern` blocks cannot have safety qualifiers
`6`	`7`	`}`
`7`	`8`
`8`	`9`	`fn main() {}`