Check unsafe impls on safe EIIs

compiler/rustc_ast_lowering/src/item.rs

@@ -155,7 +155,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
         hir::attrs::EiiImpl {
             span: self.lower_span(*span),
             inner_span: self.lower_span(*inner_span),
-            impl_marked_unsafe: self.lower_safety(*impl_safety, hir::Safety::Safe).is_unsafe(),
+            impl_unsafe_span: match *impl_safety {
+                Safety::Unsafe(span) => Some(self.lower_span(span)),
+                Safety::Safe(_) | Safety::Default => None,
+            },
             is_default: *is_default,
             resolution,
         }

compiler/rustc_hir/src/attrs/data_structures.rs

@@ -40,7 +40,7 @@ pub enum EiiImplResolution {
 #[derive(Copy, Clone, Debug, StableHash, Encodable, Decodable, PrintAttribute)]
 pub struct EiiImpl {
     pub resolution: EiiImplResolution,
-    pub impl_marked_unsafe: bool,
+    pub impl_unsafe_span: Option<Span>,
     pub span: Span,
     pub inner_span: Span,
     pub is_default: bool,

compiler/rustc_passes/src/check_attr.rs

@@ -473,43 +473,56 @@ impl<'tcx> CheckAttrVisitor<'tcx> {
     }
 
     fn check_eii_impl(&self, impls: &[EiiImpl], target: Target) {
-        for EiiImpl { span, inner_span, resolution, impl_marked_unsafe, is_default: _ } in impls {
+        for EiiImpl { span, inner_span, resolution, impl_unsafe_span, is_default: _ } in impls {
             match target {
                 Target::Fn | Target::Static => {}
                 _ => {
                     self.dcx().emit_err(diagnostics::EiiImplTarget { span: *span });
                 }
             }
 
-            let needs_unsafe = match resolution {
-                EiiImplResolution::Macro(eii_macro) => {
-                    find_attr!(self.tcx, *eii_macro, EiiDeclaration(EiiDecl { impl_unsafe, .. }) if *impl_unsafe)
-                }
-                EiiImplResolution::Known(foreign_item_did) => {
-                    let foreign_item_did = *foreign_item_did;
-                    self.tcx
-                        .externally_implementable_items(foreign_item_did.krate)
-                        .get(&foreign_item_did)
-                        .map(|(decl, _)| decl.impl_unsafe)
-                        .unwrap_or(false)
-                }
-                EiiImplResolution::Error(_) => false,
+            let impl_unsafe = match resolution {
+                EiiImplResolution::Macro(eii_macro) => find_attr!(
+                    self.tcx,
+                    *eii_macro,
+                    EiiDeclaration(EiiDecl { impl_unsafe, .. }) => *impl_unsafe
+                ),
+                EiiImplResolution::Known(foreign_item_did) => self
+                    .tcx
+                    .externally_implementable_items(foreign_item_did.krate)
+                    .get(foreign_item_did)
+                    .map(|(decl, _)| decl.impl_unsafe),
+                EiiImplResolution::Error(_) => None,
+            };
+            let Some(needs_unsafe) = impl_unsafe else {
+                continue;
             };
 
-            if needs_unsafe && !impl_marked_unsafe {
-                let name = match resolution {
-                    EiiImplResolution::Macro(eii_macro) => self.tcx.item_name(*eii_macro),
-                    EiiImplResolution::Known(def_id) => self.tcx.item_name(*def_id),
-                    EiiImplResolution::Error(_) => unreachable!(),
-                };
-                self.dcx().emit_err(diagnostics::EiiImplRequiresUnsafe {
-                    span: *span,
-                    name,
-                    suggestion: diagnostics::EiiImplRequiresUnsafeSuggestion {
-                        left: inner_span.shrink_to_lo(),
-                        right: inner_span.shrink_to_hi(),
-                    },
-                });
+            let name = match resolution {
+                EiiImplResolution::Macro(eii_macro) => self.tcx.item_name(*eii_macro),
+                EiiImplResolution::Known(def_id) => self.tcx.item_name(*def_id),
+                EiiImplResolution::Error(_) => unreachable!(),
+            };
+
+            match (needs_unsafe, *impl_unsafe_span) {
+                (true, None) => {
+                    self.dcx().emit_err(diagnostics::EiiImplRequiresUnsafe {
+                        span: *span,
+                        name,
+                        suggestion: diagnostics::EiiImplRequiresUnsafeSuggestion {
+                            left: inner_span.shrink_to_lo(),
+                            right: inner_span.shrink_to_hi(),
+                        },
+                    });
+                }
+                (false, Some(unsafe_span)) => {
+                    self.dcx().emit_err(diagnostics::EiiImplCannotBeUnsafe {
+                        impl_span: *span,
+                        unsafe_span,
+                        name,
+                    });
+                }
+                _ => {}
             }
         }
     }

compiler/rustc_passes/src/diagnostics.rs

@@ -515,10 +515,10 @@ impl<'a, G: EmissionGuarantee> Diagnostic<'a, G> for NoMainErr {
         if self.add_teach_note {
             diag.note(msg!("if you don't know the basics of Rust, you can go look to the Rust Book to get started: https://doc.rust-lang.org/book/"));
         }
+
         diag
     }
 }
-
 pub(crate) struct DuplicateLangItem {
     pub local_span: Option<Span>,
     pub lang_item_name: Symbol,
@@ -1082,6 +1082,16 @@ pub(crate) struct EiiImplRequiresUnsafeSuggestion {
     pub right: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag("`{$name}` is not unsafe to implement")]
+pub(crate) struct EiiImplCannotBeUnsafe {
+    #[primary_span]
+    pub impl_span: Span,
+    #[label("`unsafe` is not allowed here")]
+    pub unsafe_span: Span,
+    pub name: Symbol,
+}
+
 #[derive(Diagnostic)]
 #[diag("`#[{$name}]` {$kind} required, but not found")]
 pub(crate) struct EiiWithoutImpl {

tests/ui/eii/default/auxiliary/impl1.rs

@@ -6,7 +6,7 @@
 extern crate decl_with_default as decl;
 
 
-#[unsafe(decl::eii1)] //~ ERROR multiple implementations of `#[eii1]`
+#[decl::eii1] //~ ERROR multiple implementations of `#[eii1]`
 fn other(x: u64) {
     println!("1{x}");
 }

tests/ui/eii/duplicate/auxiliary/impl1.rs

@@ -5,7 +5,7 @@
 extern crate decl;
 
 
-#[unsafe(decl::eii1)]
+#[decl::eii1]
 fn other(x: u64) {
     println!("1{x}");
 }

tests/ui/eii/duplicate/auxiliary/impl2.rs

@@ -5,7 +5,7 @@
 extern crate decl;
 
 
-#[unsafe(decl::eii1)]
+#[decl::eii1]
 fn other(x: u64) {
     println!("2{x}");
 }

tests/ui/eii/duplicate/auxiliary/impl3.rs

@@ -5,7 +5,7 @@
 extern crate decl;
 
 
-#[unsafe(decl::eii1)]
+#[decl::eii1]
 fn other(x: u64) {
     println!("3{x}");
 }

tests/ui/eii/duplicate/auxiliary/impl4.rs

@@ -5,7 +5,7 @@
 extern crate decl;
 
 
-#[unsafe(decl::eii1)]
+#[decl::eii1]
 fn other(x: u64) {
     println!("4{x}");
 }

tests/ui/eii/duplicate/dylib_default_duplicate.rs

@@ -11,7 +11,7 @@
 
 extern crate dylib_default;
 
-#[unsafe(dylib_default::eii1)]
+#[dylib_default::eii1]
 fn other(x: u64) {
     //~^ ERROR multiple implementations of `#[eii1]`
     println!("1{x}");

tests/ui/eii/safe_eii_unsafe_impl.rs

@@ -0,0 +1,14 @@
+// Tests that safe EIIs reject `unsafe(...)` implementation attributes.
+#![feature(extern_item_impls)]
+
+#[eii]
+fn foo(x: u64) -> u64;
+
+#[unsafe(foo)] //~ ERROR `foo` is not unsafe to implement
+fn foo_impl(x: u64) -> u64 {
+    x
+}
+
+fn main() {
+    foo(0);
+}

tests/ui/eii/safe_eii_unsafe_impl.stderr

@@ -0,0 +1,10 @@
+error: `foo` is not unsafe to implement
+  --> $DIR/safe_eii_unsafe_impl.rs:7:1
+   |
+LL | #[unsafe(foo)]
+   | ^^------^^^^^^
+   |   |
+   |   `unsafe` is not allowed here
+
+error: aborting due to 1 previous error
+

tests/ui/eii/type_checking/cross_crate_type_ok.rs

@@ -9,7 +9,7 @@
 
 extern crate cross_crate_eii_declaration;
 
-#[unsafe(cross_crate_eii_declaration::foo)]
+#[cross_crate_eii_declaration::foo]
 fn other(x: u64) -> u64 {
     x
 }

tests/ui/eii/type_checking/cross_crate_wrong_ty.rs

@@ -8,7 +8,7 @@
 
 extern crate cross_crate_eii_declaration;
 
-#[unsafe(cross_crate_eii_declaration::foo)]
+#[cross_crate_eii_declaration::foo]
 fn other() -> u64 {
 //~^ ERROR `other` has 0 parameters but #[foo] requires it to have 1
     0

tests/ui/eii/type_checking/cross_crate_wrong_ty.stderr

@@ -1,8 +1,8 @@
 error[E0806]: `other` has 0 parameters but #[foo] requires it to have 1
   --> $DIR/cross_crate_wrong_ty.rs:12:1
    |
-LL | #[unsafe(cross_crate_eii_declaration::foo)]
-   | ------------------------------------------- required because of this attribute
+LL | #[cross_crate_eii_declaration::foo]
+   | ----------------------------------- required because of this attribute
 LL | fn other() -> u64 {
    | ^^^^^^^^^^^^^^^^^ expected 1 parameter, found 0
    |