Auto merge of #126761 - GuillaumeGomez:unsafe_extern_blocks, r=spastorino

rustdoc: Add support for `missing_unsafe_on_extern` feature

Follow-up of #124482.

Not sure if the `safe` keyword is supposed to be displayed or not though? For now I didn't add it in the generated doc, only `unsafe` as usual.

cc `@spastorino`
r? `@fmease`

Author: bors

src/librustdoc/clean/mod.rs

@@ -3077,23 +3077,20 @@ fn clean_maybe_renamed_foreign_item<'tcx>(
     let def_id = item.owner_id.to_def_id();
     cx.with_param_env(def_id, |cx| {
         let kind = match item.kind {
-            // FIXME(missing_unsafe_on_extern) handle safety of foreign fns.
-            // Safety was added as part of the implementation of unsafe extern blocks PR #124482
-            hir::ForeignItemKind::Fn(decl, names, generics, _) => {
+            hir::ForeignItemKind::Fn(decl, names, generics, safety) => {
                 let (generics, decl) = enter_impl_trait(cx, |cx| {
                     // NOTE: generics must be cleaned before args
                     let generics = clean_generics(generics, cx);
                     let args = clean_args_from_types_and_names(cx, decl.inputs, names);
                     let decl = clean_fn_decl_with_args(cx, decl, None, args);
                     (generics, decl)
                 });
-                ForeignFunctionItem(Box::new(Function { decl, generics }))
-            }
-            // FIXME(missing_unsafe_on_extern) handle safety of foreign statics.
-            // Safety was added as part of the implementation of unsafe extern blocks PR #124482
-            hir::ForeignItemKind::Static(ty, mutability, _) => {
-                ForeignStaticItem(Static { type_: clean_ty(ty, cx), mutability, expr: None })
+                ForeignFunctionItem(Box::new(Function { decl, generics }), safety)
             }
+            hir::ForeignItemKind::Static(ty, mutability, safety) => ForeignStaticItem(
+                Static { type_: clean_ty(ty, cx), mutability, expr: None },
+                safety,
+            ),
             hir::ForeignItemKind::Type => ForeignTypeItem,
         };
 

src/librustdoc/clean/types.rs

@@ -639,14 +639,14 @@ impl Item {
             hir::FnHeader { safety: sig.safety(), abi: sig.abi(), constness, asyncness }
         }
         let header = match *self.kind {
-            ItemKind::ForeignFunctionItem(_) => {
+            ItemKind::ForeignFunctionItem(_, safety) => {
                 let def_id = self.def_id().unwrap();
                 let abi = tcx.fn_sig(def_id).skip_binder().abi();
                 hir::FnHeader {
                     safety: if abi == Abi::RustIntrinsic {
                         intrinsic_operation_unsafety(tcx, def_id.expect_local())
                     } else {
-                        hir::Safety::Unsafe
+                        safety
                     },
                     abi,
                     constness: if tcx.is_const_fn(def_id)
@@ -842,9 +842,9 @@ pub(crate) enum ItemKind {
     StructFieldItem(Type),
     VariantItem(Variant),
     /// `fn`s from an extern block
-    ForeignFunctionItem(Box<Function>),
+    ForeignFunctionItem(Box<Function>, hir::Safety),
     /// `static`s from an extern block
-    ForeignStaticItem(Static),
+    ForeignStaticItem(Static, hir::Safety),
     /// `type`s from an extern block
     ForeignTypeItem,
     MacroItem(Macro),
@@ -893,8 +893,8 @@ impl ItemKind {
             | TyMethodItem(_)
             | MethodItem(_, _)
             | StructFieldItem(_)
-            | ForeignFunctionItem(_)
-            | ForeignStaticItem(_)
+            | ForeignFunctionItem(_, _)
+            | ForeignStaticItem(_, _)
             | ForeignTypeItem
             | MacroItem(_)
             | ProcMacroItem(_)
@@ -924,8 +924,8 @@ impl ItemKind {
                 | StaticItem(_)
                 | ConstantItem(_, _, _)
                 | TraitAliasItem(_)
-                | ForeignFunctionItem(_)
-                | ForeignStaticItem(_)
+                | ForeignFunctionItem(_, _)
+                | ForeignStaticItem(_, _)
                 | ForeignTypeItem
                 | MacroItem(_)
                 | ProcMacroItem(_)

src/librustdoc/fold.rs

@@ -84,8 +84,8 @@ pub(crate) trait DocFolder: Sized {
             | TyMethodItem(_)
             | MethodItem(_, _)
             | StructFieldItem(_)
-            | ForeignFunctionItem(_)
-            | ForeignStaticItem(_)
+            | ForeignFunctionItem(..)
+            | ForeignStaticItem(..)
             | ForeignTypeItem
             | MacroItem(_)
             | ProcMacroItem(_)

src/librustdoc/html/render/print_item.rs

@@ -254,7 +254,7 @@ pub(super) fn print_item(cx: &mut Context<'_>, item: &clean::Item, buf: &mut Buf
 
     match &*item.kind {
         clean::ModuleItem(ref m) => item_module(buf, cx, item, &m.items),
-        clean::FunctionItem(ref f) | clean::ForeignFunctionItem(ref f) => {
+        clean::FunctionItem(ref f) | clean::ForeignFunctionItem(ref f, _) => {
             item_function(buf, cx, item, f)
         }
         clean::TraitItem(ref t) => item_trait(buf, cx, item, t),
@@ -265,7 +265,8 @@ pub(super) fn print_item(cx: &mut Context<'_>, item: &clean::Item, buf: &mut Buf
         clean::MacroItem(ref m) => item_macro(buf, cx, item, m),
         clean::ProcMacroItem(ref m) => item_proc_macro(buf, cx, item, m),
         clean::PrimitiveItem(_) => item_primitive(buf, cx, item),
-        clean::StaticItem(ref i) | clean::ForeignStaticItem(ref i) => item_static(buf, cx, item, i),
+        clean::StaticItem(ref i) => item_static(buf, cx, item, i, None),
+        clean::ForeignStaticItem(ref i, safety) => item_static(buf, cx, item, i, Some(*safety)),
         clean::ConstantItem(generics, ty, c) => item_constant(buf, cx, item, generics, ty, c),
         clean::ForeignTypeItem => item_foreign_type(buf, cx, item),
         clean::KeywordItem => item_keyword(buf, cx, item),
@@ -491,11 +492,14 @@ fn item_module(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Item, items:
                 }
 
                 let unsafety_flag = match *myitem.kind {
-                    clean::FunctionItem(_) | clean::ForeignFunctionItem(_)
+                    clean::FunctionItem(_) | clean::ForeignFunctionItem(..)
                         if myitem.fn_header(tcx).unwrap().safety == hir::Safety::Unsafe =>
                     {
                         "<sup title=\"unsafe function\">⚠</sup>"
                     }
+                    clean::ForeignStaticItem(_, hir::Safety::Unsafe) => {
+                        "<sup title=\"unsafe static\">⚠</sup>"
+                    }
                     _ => "",
                 };
 
@@ -1957,13 +1961,22 @@ fn item_fields(
     }
 }
 
-fn item_static(w: &mut impl fmt::Write, cx: &mut Context<'_>, it: &clean::Item, s: &clean::Static) {
+fn item_static(
+    w: &mut impl fmt::Write,
+    cx: &mut Context<'_>,
+    it: &clean::Item,
+    s: &clean::Static,
+    safety: Option<hir::Safety>,
+) {
     wrap_item(w, |buffer| {
         render_attributes_in_code(buffer, it, cx);
         write!(
             buffer,
-            "{vis}static {mutability}{name}: {typ}",
+            "{vis}{safe}static {mutability}{name}: {typ}",
             vis = visibility_print_with_space(it, cx),
+            safe = safety
+                .map(|safe| if safe == hir::Safety::Unsafe { "unsafe " } else { "" })
+                .unwrap_or(""),
             mutability = s.mutability.print_with_space(),
             name = it.name.unwrap(),
             typ = s.type_.print(cx)

src/librustdoc/json/conversions.rs

@@ -310,14 +310,16 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum {
         EnumItem(e) => ItemEnum::Enum(e.into_tcx(tcx)),
         VariantItem(v) => ItemEnum::Variant(v.into_tcx(tcx)),
         FunctionItem(f) => ItemEnum::Function(from_function(f, true, header.unwrap(), tcx)),
-        ForeignFunctionItem(f) => ItemEnum::Function(from_function(f, false, header.unwrap(), tcx)),
+        ForeignFunctionItem(f, _) => {
+            ItemEnum::Function(from_function(f, false, header.unwrap(), tcx))
+        }
         TraitItem(t) => ItemEnum::Trait((*t).into_tcx(tcx)),
         TraitAliasItem(t) => ItemEnum::TraitAlias(t.into_tcx(tcx)),
         MethodItem(m, _) => ItemEnum::Function(from_function(m, true, header.unwrap(), tcx)),
         TyMethodItem(m) => ItemEnum::Function(from_function(m, false, header.unwrap(), tcx)),
         ImplItem(i) => ItemEnum::Impl((*i).into_tcx(tcx)),
         StaticItem(s) => ItemEnum::Static(s.into_tcx(tcx)),
-        ForeignStaticItem(s) => ItemEnum::Static(s.into_tcx(tcx)),
+        ForeignStaticItem(s, _) => ItemEnum::Static(s.into_tcx(tcx)),
         ForeignTypeItem => ItemEnum::ForeignType,
         TypeAliasItem(t) => ItemEnum::TypeAlias(t.into_tcx(tcx)),
         OpaqueTyItem(t) => ItemEnum::OpaqueTy(t.into_tcx(tcx)),

src/librustdoc/visit.rs

@@ -33,8 +33,8 @@ pub(crate) trait DocVisitor: Sized {
             | TyMethodItem(_)
             | MethodItem(_, _)
             | StructFieldItem(_)
-            | ForeignFunctionItem(_)
-            | ForeignStaticItem(_)
+            | ForeignFunctionItem(..)
+            | ForeignStaticItem(..)
             | ForeignTypeItem
             | MacroItem(_)
             | ProcMacroItem(_)

tests/rustdoc/unsafe-extern-blocks.rs

@@ -0,0 +1,30 @@
+// Test to ensure the feature is working as expected.
+
+#![feature(unsafe_extern_blocks)]
+#![crate_name = "foo"]
+
+// @has 'foo/index.html'
+
+// First we check that both the static and the function have a "sup" element
+// to tell they're unsafe.
+
+// @count - '//ul[@class="item-table"]//sup[@title="unsafe static"]' 1
+// @has - '//ul[@class="item-table"]//sup[@title="unsafe static"]' '⚠'
+// @count - '//ul[@class="item-table"]//sup[@title="unsafe function"]' 1
+// @has - '//ul[@class="item-table"]//sup[@title="unsafe function"]' '⚠'
+
+unsafe extern {
+    // @has 'foo/static.FOO.html'
+    // @has - '//pre[@class="rust item-decl"]' 'pub static FOO: i32'
+    pub safe static FOO: i32;
+    // @has 'foo/static.BAR.html'
+    // @has - '//pre[@class="rust item-decl"]' 'pub unsafe static BAR: i32'
+    pub static BAR: i32;
+
+    // @has 'foo/fn.foo.html'
+    // @has - '//pre[@class="rust item-decl"]' 'pub extern "C" fn foo()'
+    pub safe fn foo();
+    // @has 'foo/fn.bar.html'
+    // @has - '//pre[@class="rust item-decl"]' 'pub unsafe extern "C" fn bar()'
+    pub fn bar();
+}