Do not allow safe usafe on static and fn items

spastorino · spastorino · commit 3d6d9e29a6cd · 2024-06-20T18:39:22.000-03:00
diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl
@@ -30,6 +30,9 @@ ast_passes_auto_super_lifetime = auto traits cannot have super traits or lifetim
 
 ast_passes_bad_c_variadic = only foreign or `unsafe extern "C"` functions may be C-variadic
 
+ast_passes_bare_fn_invalid_safety = function pointers cannot be declared with `safe` safety qualifier
+    .suggestion = remove safe from this item
+
 ast_passes_body_in_extern = incorrect `{$kind}` inside `extern` block
     .cannot_have = cannot have a body
     .invalid = the invalid body
@@ -167,6 +170,9 @@ ast_passes_invalid_unnamed_field_ty =
     unnamed fields can only have struct or union types
     .label = not a struct or union
 
+ast_passes_item_invalid_safety = items outside of `unsafe extern {"{ }"}` cannot be declared with `safe` safety qualifier
+    .suggestion = remove safe from this item
+
 ast_passes_item_underscore = `{$kind}` items in this context need a name
     .label = `_` is not a valid name for this `{$kind}` item
 
diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs
@@ -456,15 +456,29 @@ impl<'a> AstValidator<'a> {
         }
     }
 
-    fn check_foreign_item_safety(&self, item_span: Span, safety: Safety) {
-        if matches!(safety, Safety::Unsafe(_) | Safety::Safe(_))
-            && (self.extern_mod_safety == Some(Safety::Default)
-                || !self.features.unsafe_extern_blocks)
-        {
-            self.dcx().emit_err(errors::InvalidSafetyOnExtern {
-                item_span,
-                block: self.current_extern_span(),
-            });
+    fn check_item_safety(&self, span: Span, safety: Safety) {
+        match self.extern_mod_safety {
+            Some(extern_safety) => {
+                if matches!(safety, Safety::Unsafe(_) | Safety::Safe(_))
+                    && (extern_safety == Safety::Default || !self.features.unsafe_extern_blocks)
+                {
+                    self.dcx().emit_err(errors::InvalidSafetyOnExtern {
+                        item_span: span,
+                        block: self.current_extern_span(),
+                    });
+                }
+            }
+            None => {
+                if matches!(safety, Safety::Safe(_)) {
+                    self.dcx().emit_err(errors::InvalidSafetyOnItem { span });
+                }
+            }
+        }
+    }
+
+    fn check_bare_fn_safety(&self, span: Span, safety: Safety) {
+        if matches!(safety, Safety::Safe(_)) {
+            self.dcx().emit_err(errors::InvalidSafetyOnBareFn { span });
         }
     }
 
@@ -746,6 +760,7 @@ impl<'a> AstValidator<'a> {
     fn visit_ty_common(&mut self, ty: &'a Ty) {
         match &ty.kind {
             TyKind::BareFn(bfty) => {
+                self.check_bare_fn_safety(bfty.decl_span, bfty.safety);
                 self.check_fn_decl(&bfty.decl, SelfSemantic::No);
                 Self::check_decl_no_pat(&bfty.decl, |span, _, _| {
                     self.dcx().emit_err(errors::PatternFnPointer { span });
@@ -1174,11 +1189,15 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     });
                 }
             }
-            ItemKind::Static(box StaticItem { expr: None, .. }) => {
-                self.dcx().emit_err(errors::StaticWithoutBody {
-                    span: item.span,
-                    replace_span: self.ending_semi_or_hi(item.span),
-                });
+            ItemKind::Static(box StaticItem { expr, safety, .. }) => {
+                self.check_item_safety(item.span, *safety);
+
+                if expr.is_none() {
+                    self.dcx().emit_err(errors::StaticWithoutBody {
+                        span: item.span,
+                        replace_span: self.ending_semi_or_hi(item.span),
+                    });
+                }
             }
             ItemKind::TyAlias(
                 ty_alias @ box TyAlias { defaultness, bounds, where_clauses, ty, .. },
@@ -1212,7 +1231,6 @@ 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);
@@ -1233,7 +1251,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                 self.check_foreign_item_ascii_only(fi.ident);
             }
             ForeignItemKind::Static(box StaticForeignItem { expr, safety, .. }) => {
-                self.check_foreign_item_safety(fi.span, *safety);
+                self.check_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);
             }
@@ -1453,6 +1471,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
         };
         self.check_fn_decl(fk.decl(), self_semantic);
 
+        if let Some(&FnHeader { safety, .. }) = fk.header() {
+            self.check_item_safety(span, safety);
+        }
+
         self.check_c_variadic_type(fk);
 
         // Functions cannot both be `const async` or `const gen`
diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs
@@ -225,6 +225,20 @@ pub struct InvalidSafetyOnExtern {
     pub block: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(ast_passes_item_invalid_safety)]
+pub struct InvalidSafetyOnItem {
+    #[primary_span]
+    pub span: Span,
+}
+
+#[derive(Diagnostic)]
+#[diag(ast_passes_bare_fn_invalid_safety)]
+pub struct InvalidSafetyOnBareFn {
+    #[primary_span]
+    pub span: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(ast_passes_bound_in_context)]
 pub struct BoundInContext<'a> {
diff --git a/tests/ui/parser/fn-header-semantic-fail.stderr b/tests/ui/parser/fn-header-semantic-fail.stderr
@@ -105,15 +105,6 @@ 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
    |
@@ -141,6 +132,15 @@ LL |     extern "C" {
 LL |         const async unsafe extern "C" fn fe5();
    |                            ^^^^^^^^^^ 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 cannot be both `const` and `async`
   --> $DIR/fn-header-semantic-fail.rs:50:9
    |
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,23 +6,23 @@ LL | extern "C" {
 LL |     const fn foo();
    |     ^^^^^ help: remove this qualifier
 
-error: items in unadorned `extern` blocks cannot have safety qualifiers
+error: functions in `extern` blocks cannot have qualifiers
   --> $DIR/no-const-fn-in-extern-block.rs:4:5
    |
 LL | extern "C" {
-   | ---------- help: add unsafe to this `extern` block
+   | ---------- in this `extern` block
 ...
 LL |     const unsafe fn bar();
-   |     ^^^^^^^^^^^^^^^^^^^^^^
+   |     ^^^^^ help: remove this qualifier
 
-error: functions in `extern` blocks cannot have qualifiers
+error: items in unadorned `extern` blocks cannot have safety qualifiers
   --> $DIR/no-const-fn-in-extern-block.rs:4:5
    |
 LL | extern "C" {
-   | ---------- in this `extern` block
+   | ---------- help: add unsafe to this `extern` block
 ...
 LL |     const unsafe fn bar();
-   |     ^^^^^ help: remove this qualifier
+   |     ^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to 3 previous errors
 
diff --git a/tests/ui/rust-2024/safe-outside-extern.gated.stderr b/tests/ui/rust-2024/safe-outside-extern.gated.stderr
@@ -0,0 +1,32 @@
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:4:1
+   |
+LL | safe fn foo() {}
+   | ^^^^^^^^^^^^^^^^
+
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:7:1
+   |
+LL | safe static FOO: i32 = 1;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:11:5
+   |
+LL |     safe fn foo();
+   |     ^^^^^^^^^^^^^^
+
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:16:5
+   |
+LL |     safe fn foo() {}
+   |     ^^^^^^^^^^^^^^^^
+
+error: function pointers cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:20:14
+   |
+LL | type FnPtr = safe fn(i32, i32) -> i32;
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 5 previous errors
+
diff --git a/tests/ui/rust-2024/safe-outside-extern.rs b/tests/ui/rust-2024/safe-outside-extern.rs
@@ -0,0 +1,23 @@
+//@ revisions: gated ungated
+#![cfg_attr(gated, feature(unsafe_extern_blocks))]
+
+safe fn foo() {}
+//~^ ERROR: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+
+safe static FOO: i32 = 1;
+//~^ ERROR: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+
+trait Foo {
+    safe fn foo();
+    //~^ ERROR: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+}
+
+impl Foo for () {
+    safe fn foo() {}
+    //~^ ERROR: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+}
+
+type FnPtr = safe fn(i32, i32) -> i32;
+//~^ ERROR: function pointers cannot be declared with `safe` safety qualifier
+
+fn main() {}
diff --git a/tests/ui/rust-2024/safe-outside-extern.ungated.stderr b/tests/ui/rust-2024/safe-outside-extern.ungated.stderr
@@ -0,0 +1,32 @@
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:4:1
+   |
+LL | safe fn foo() {}
+   | ^^^^^^^^^^^^^^^^
+
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:7:1
+   |
+LL | safe static FOO: i32 = 1;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:11:5
+   |
+LL |     safe fn foo();
+   |     ^^^^^^^^^^^^^^
+
+error: items outside of `unsafe extern { }` cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:16:5
+   |
+LL |     safe fn foo() {}
+   |     ^^^^^^^^^^^^^^^^
+
+error: function pointers cannot be declared with `safe` safety qualifier
+  --> $DIR/safe-outside-extern.rs:20:14
+   |
+LL | type FnPtr = safe fn(i32, i32) -> i32;
+   |              ^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 5 previous errors
+
