Auto merge of #95096 - GuillaumeGomez:rm-header-fn-field, r=camelid

Remove header field from clean::Function

Fixes https://github.com/rust-lang/rust/issues/89673.

This is another take on https://github.com/rust-lang/rust/issues/89673 (compared to https://github.com/rust-lang/rust/pull/91217) but very different on the approach: I moved the header call in one place but still require to have the `clean::Item` so I can use the `DefId` to get what is missing.

cc `@jyn514` (you reviewed the original so maybe you want to take a look?)
r? `@camelid`

src/librustdoc/clean/inline.rs

@@ -221,9 +221,6 @@ crate fn build_external_trait(cx: &mut DocContext<'_>, did: DefId) -> clean::Tra
 fn build_external_function(cx: &mut DocContext<'_>, did: DefId) -> clean::Function {
     let sig = cx.tcx.fn_sig(did);
 
-    let constness =
-        if cx.tcx.is_const_fn_raw(did) { hir::Constness::Const } else { hir::Constness::NotConst };
-    let asyncness = cx.tcx.asyncness(did);
     let predicates = cx.tcx.predicates_of(did);
     let (generics, decl) = clean::enter_impl_trait(cx, |cx| {
         // NOTE: generics need to be cleaned before the decl!
@@ -231,11 +228,7 @@ fn build_external_function(cx: &mut DocContext<'_>, did: DefId) -> clean::Functi
         let decl = clean_fn_decl_from_did_and_sig(cx, Some(did), sig);
         (generics, decl)
     });
-    clean::Function {
-        decl,
-        generics,
-        header: hir::FnHeader { unsafety: sig.unsafety(), abi: sig.abi(), constness, asyncness },
-    }
+    clean::Function { decl, generics }
 }
 
 fn build_enum(cx: &mut DocContext<'_>, did: DefId) -> clean::Enum {

src/librustdoc/clean/mod.rs

@@ -12,7 +12,6 @@ crate mod utils;
 
 use rustc_ast as ast;
 use rustc_attr as attr;
-use rustc_const_eval::const_eval::is_unstable_const_fn;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
 use rustc_hir::def::{CtorKind, DefKind, Res};
@@ -26,8 +25,6 @@ use rustc_middle::{bug, span_bug};
 use rustc_span::hygiene::{AstPass, MacroKind};
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{self, ExpnKind};
-use rustc_target::spec::abi::Abi;
-use rustc_typeck::check::intrinsic::intrinsic_operation_unsafety;
 use rustc_typeck::hir_ty_to_ty;
 
 use std::assert_matches::assert_matches;
@@ -813,13 +810,6 @@ fn clean_fn_or_proc_macro(
         }
         None => {
             let mut func = clean_function(cx, sig, generics, body_id);
-            let def_id = item.def_id.to_def_id();
-            func.header.constness =
-                if cx.tcx.is_const_fn(def_id) && is_unstable_const_fn(cx.tcx, def_id).is_none() {
-                    hir::Constness::Const
-                } else {
-                    hir::Constness::NotConst
-                };
             clean_fn_decl_legacy_const_generics(&mut func, attrs);
             FunctionItem(func)
         }
@@ -869,7 +859,7 @@ fn clean_function(
         let decl = clean_fn_decl_with_args(cx, sig.decl, args);
         (generics, decl)
     });
-    Function { decl, generics, header: sig.header }
+    Function { decl, generics }
 }
 
 fn clean_args_from_types_and_names(
@@ -998,12 +988,7 @@ impl Clean<Item> for hir::TraitItem<'_> {
                     AssocConstItem(ty.clean(cx), default)
                 }
                 hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Provided(body)) => {
-                    let mut m = clean_function(cx, sig, &self.generics, body);
-                    if m.header.constness == hir::Constness::Const
-                        && is_unstable_const_fn(cx.tcx, local_did).is_some()
-                    {
-                        m.header.constness = hir::Constness::NotConst;
-                    }
+                    let m = clean_function(cx, sig, &self.generics, body);
                     MethodItem(m, None)
                 }
                 hir::TraitItemKind::Fn(ref sig, hir::TraitFn::Required(names)) => {
@@ -1014,13 +999,7 @@ impl Clean<Item> for hir::TraitItem<'_> {
                         let decl = clean_fn_decl_with_args(cx, sig.decl, args);
                         (generics, decl)
                     });
-                    let mut t = Function { header: sig.header, decl, generics };
-                    if t.header.constness == hir::Constness::Const
-                        && is_unstable_const_fn(cx.tcx, local_did).is_some()
-                    {
-                        t.header.constness = hir::Constness::NotConst;
-                    }
-                    TyMethodItem(t)
+                    TyMethodItem(Function { decl, generics })
                 }
                 hir::TraitItemKind::Type(bounds, ref default) => {
                     let generics = enter_impl_trait(cx, |cx| self.generics.clean(cx));
@@ -1047,12 +1026,7 @@ impl Clean<Item> for hir::ImplItem<'_> {
                     AssocConstItem(ty.clean(cx), default)
                 }
                 hir::ImplItemKind::Fn(ref sig, body) => {
-                    let mut m = clean_function(cx, sig, &self.generics, body);
-                    if m.header.constness == hir::Constness::Const
-                        && is_unstable_const_fn(cx.tcx, local_did).is_some()
-                    {
-                        m.header.constness = hir::Constness::NotConst;
-                    }
+                    let m = clean_function(cx, sig, &self.generics, body);
                     let defaultness = cx.tcx.associated_item(self.def_id).defaultness;
                     MethodItem(m, Some(defaultness))
                 }
@@ -1127,40 +1101,13 @@ impl Clean<Item> for ty::AssocItem {
                     ty::TraitContainer(_) => self.defaultness.has_value(),
                 };
                 if provided {
-                    let constness = if tcx.is_const_fn_raw(self.def_id) {
-                        hir::Constness::Const
-                    } else {
-                        hir::Constness::NotConst
-                    };
-                    let asyncness = tcx.asyncness(self.def_id);
                     let defaultness = match self.container {
                         ty::ImplContainer(_) => Some(self.defaultness),
                         ty::TraitContainer(_) => None,
                     };
-                    MethodItem(
-                        Function {
-                            generics,
-                            decl,
-                            header: hir::FnHeader {
-                                unsafety: sig.unsafety(),
-                                abi: sig.abi(),
-                                constness,
-                                asyncness,
-                            },
-                        },
-                        defaultness,
-                    )
+                    MethodItem(Function { generics, decl }, defaultness)
                 } else {
-                    TyMethodItem(Function {
-                        generics,
-                        decl,
-                        header: hir::FnHeader {
-                            unsafety: sig.unsafety(),
-                            abi: sig.abi(),
-                            constness: hir::Constness::NotConst,
-                            asyncness: hir::IsAsync::NotAsync,
-                        },
-                    })
+                    TyMethodItem(Function { generics, decl })
                 }
             }
             ty::AssocKind::Type => {
@@ -2192,7 +2139,6 @@ fn clean_maybe_renamed_foreign_item(
     cx.with_param_env(def_id, |cx| {
         let kind = match item.kind {
             hir::ForeignItemKind::Fn(decl, names, ref generics) => {
-                let abi = cx.tcx.hir().get_foreign_abi(item.hir_id());
                 let (generics, decl) = enter_impl_trait(cx, |cx| {
                     // NOTE: generics must be cleaned before args
                     let generics = generics.clean(cx);
@@ -2200,20 +2146,7 @@ fn clean_maybe_renamed_foreign_item(
                     let decl = clean_fn_decl_with_args(cx, decl, args);
                     (generics, decl)
                 });
-                ForeignFunctionItem(Function {
-                    decl,
-                    generics,
-                    header: hir::FnHeader {
-                        unsafety: if abi == Abi::RustIntrinsic {
-                            intrinsic_operation_unsafety(item.ident.name)
-                        } else {
-                            hir::Unsafety::Unsafe
-                        },
-                        abi,
-                        constness: hir::Constness::NotConst,
-                        asyncness: hir::IsAsync::NotAsync,
-                    },
-                })
+                ForeignFunctionItem(Function { decl, generics })
             }
             hir::ForeignItemKind::Static(ref ty, mutability) => {
                 ForeignStaticItem(Static { type_: ty.clean(cx), mutability, expr: None })

src/librustdoc/clean/types.rs

@@ -13,6 +13,7 @@ use rustc_ast::attr;
 use rustc_ast::util::comments::beautify_doc_string;
 use rustc_ast::{self as ast, AttrStyle};
 use rustc_attr::{ConstStability, Deprecation, Stability, StabilityLevel};
+use rustc_const_eval::const_eval::is_unstable_const_fn;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_data_structures::thin_vec::ThinVec;
 use rustc_hir as hir;
@@ -29,6 +30,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol};
 use rustc_span::{self, FileName, Loc};
 use rustc_target::abi::VariantIdx;
 use rustc_target::spec::abi::Abi;
+use rustc_typeck::check::intrinsic::intrinsic_operation_unsafety;
 
 use crate::clean::cfg::Cfg;
 use crate::clean::external_path;
@@ -641,6 +643,48 @@ impl Item {
             _ => false,
         }
     }
+
+    /// Returns a `FnHeader` if `self` is a function item, otherwise returns `None`.
+    crate fn fn_header(&self, tcx: TyCtxt<'_>) -> Option<hir::FnHeader> {
+        fn build_fn_header(
+            def_id: DefId,
+            tcx: TyCtxt<'_>,
+            asyncness: hir::IsAsync,
+        ) -> hir::FnHeader {
+            let sig = tcx.fn_sig(def_id);
+            let constness =
+                if tcx.is_const_fn(def_id) && is_unstable_const_fn(tcx, def_id).is_none() {
+                    hir::Constness::Const
+                } else {
+                    hir::Constness::NotConst
+                };
+            hir::FnHeader { unsafety: sig.unsafety(), abi: sig.abi(), constness, asyncness }
+        }
+        let header = match *self.kind {
+            ItemKind::ForeignFunctionItem(_) => {
+                let abi = tcx.fn_sig(self.def_id.as_def_id().unwrap()).abi();
+                hir::FnHeader {
+                    unsafety: if abi == Abi::RustIntrinsic {
+                        intrinsic_operation_unsafety(self.name.unwrap())
+                    } else {
+                        hir::Unsafety::Unsafe
+                    },
+                    abi,
+                    constness: hir::Constness::NotConst,
+                    asyncness: hir::IsAsync::NotAsync,
+                }
+            }
+            ItemKind::FunctionItem(_) | ItemKind::MethodItem(_, _) => {
+                let def_id = self.def_id.as_def_id().unwrap();
+                build_fn_header(def_id, tcx, tcx.asyncness(def_id))
+            }
+            ItemKind::TyMethodItem(_) => {
+                build_fn_header(self.def_id.as_def_id().unwrap(), tcx, hir::IsAsync::NotAsync)
+            }
+            _ => return None,
+        };
+        Some(header)
+    }
 }
 
 #[derive(Clone, Debug)]
@@ -1253,7 +1297,6 @@ crate struct Generics {
 crate struct Function {
     crate decl: FnDecl,
     crate generics: Generics,
-    crate header: hir::FnHeader,
 }
 
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]

src/librustdoc/html/render/mod.rs

@@ -48,7 +48,6 @@ use std::string::ToString;
 use rustc_ast_pretty::pprust;
 use rustc_attr::{ConstStability, Deprecation, StabilityLevel};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::DefId;
 use rustc_hir::Mutability;
@@ -806,7 +805,6 @@ fn assoc_type(
 fn assoc_method(
     w: &mut Buffer,
     meth: &clean::Item,
-    header: hir::FnHeader,
     g: &clean::Generics,
     d: &clean::FnDecl,
     link: AssocItemLink<'_>,
@@ -814,6 +812,7 @@ fn assoc_method(
     cx: &Context<'_>,
     render_mode: RenderMode,
 ) {
+    let header = meth.fn_header(cx.tcx()).expect("Trying to get header from a non-function item");
     let name = meth.name.as_ref().unwrap();
     let href = match link {
         AssocItemLink::Anchor(Some(ref id)) => Some(format!("#{}", id)),
@@ -972,10 +971,10 @@ fn render_assoc_item(
     match *item.kind {
         clean::StrippedItem(..) => {}
         clean::TyMethodItem(ref m) => {
-            assoc_method(w, item, m.header, &m.generics, &m.decl, link, parent, cx, render_mode)
+            assoc_method(w, item, &m.generics, &m.decl, link, parent, cx, render_mode)
         }
         clean::MethodItem(ref m, _) => {
-            assoc_method(w, item, m.header, &m.generics, &m.decl, link, parent, cx, render_mode)
+            assoc_method(w, item, &m.generics, &m.decl, link, parent, cx, render_mode)
         }
         clean::AssocConstItem(ref ty, _) => {
             assoc_const(w, item, ty, link, if parent == ItemType::Trait { "    " } else { "" }, cx)

src/librustdoc/html/render/print_item.rs

@@ -365,8 +365,9 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                 }
 
                 let unsafety_flag = match *myitem.kind {
-                    clean::FunctionItem(ref func) | clean::ForeignFunctionItem(ref func)
-                        if func.header.unsafety == hir::Unsafety::Unsafe =>
+                    clean::FunctionItem(_) | clean::ForeignFunctionItem(_)
+                        if myitem.fn_header(cx.tcx()).unwrap().unsafety
+                            == hir::Unsafety::Unsafe =>
                     {
                         "<a title=\"unsafe function\" href=\"#\"><sup>⚠</sup></a>"
                     }
@@ -462,16 +463,17 @@ fn extra_info_tags(item: &clean::Item, parent: &clean::Item, tcx: TyCtxt<'_>) ->
 }
 
 fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::Function) {
-    let vis = it.visibility.print_with_space(it.def_id, cx).to_string();
-    let constness = print_constness_with_space(&f.header.constness, it.const_stability(cx.tcx()));
-    let asyncness = f.header.asyncness.print_with_space();
-    let unsafety = f.header.unsafety.print_with_space();
-    let abi = print_abi_with_space(f.header.abi).to_string();
+    let header = it.fn_header(cx.tcx()).expect("printing a function which isn't a function");
+    let constness = print_constness_with_space(&header.constness, it.const_stability(cx.tcx()));
+    let unsafety = header.unsafety.print_with_space().to_string();
+    let abi = print_abi_with_space(header.abi).to_string();
+    let asyncness = header.asyncness.print_with_space();
+    let visibility = it.visibility.print_with_space(it.def_id, cx).to_string();
     let name = it.name.unwrap();
 
     let generics_len = format!("{:#}", f.generics.print(cx)).len();
     let header_len = "fn ".len()
-        + vis.len()
+        + visibility.len()
         + constness.len()
         + asyncness.len()
         + unsafety.len()
@@ -487,7 +489,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::
                 w,
                 "{vis}{constness}{asyncness}{unsafety}{abi}fn \
                  {name}{generics}{decl}{notable_traits}{where_clause}",
-                vis = vis,
+                vis = visibility,
                 constness = constness,
                 asyncness = asyncness,
                 unsafety = unsafety,
@@ -495,7 +497,7 @@ fn item_function(w: &mut Buffer, cx: &Context<'_>, it: &clean::Item, f: &clean::
                 name = name,
                 generics = f.generics.print(cx),
                 where_clause = print_where_clause(&f.generics, cx, 0, true),
-                decl = f.decl.full_print(header_len, 0, f.header.asyncness, cx),
+                decl = f.decl.full_print(header_len, 0, header.asyncness, cx),
                 notable_traits = notable_traits_decl(&f.decl, cx),
             );
         });

src/librustdoc/json/conversions.rs

@@ -199,6 +199,8 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum {
     use clean::ItemKind::*;
     let name = item.name;
     let is_crate = item.is_crate();
+    let header = item.fn_header(tcx);
+
     match *item.kind {
         ModuleItem(m) => ItemEnum::Module(Module { is_crate, items: ids(m.items) }),
         ImportItem(i) => ItemEnum::Import(i.into_tcx(tcx)),
@@ -207,12 +209,12 @@ fn from_clean_item(item: clean::Item, tcx: TyCtxt<'_>) -> ItemEnum {
         StructFieldItem(f) => ItemEnum::StructField(f.into_tcx(tcx)),
         EnumItem(e) => ItemEnum::Enum(e.into_tcx(tcx)),
         VariantItem(v) => ItemEnum::Variant(v.into_tcx(tcx)),
-        FunctionItem(f) => ItemEnum::Function(f.into_tcx(tcx)),
-        ForeignFunctionItem(f) => ItemEnum::Function(f.into_tcx(tcx)),
+        FunctionItem(f) => ItemEnum::Function(from_function(f, header.unwrap(), tcx)),
+        ForeignFunctionItem(f) => ItemEnum::Function(from_function(f, header.unwrap(), tcx)),
         TraitItem(t) => ItemEnum::Trait(t.into_tcx(tcx)),
         TraitAliasItem(t) => ItemEnum::TraitAlias(t.into_tcx(tcx)),
-        MethodItem(m, _) => ItemEnum::Method(from_function_method(m, true, tcx)),
-        TyMethodItem(m) => ItemEnum::Method(from_function_method(m, false, tcx)),
+        MethodItem(m, _) => ItemEnum::Method(from_function_method(m, true, header.unwrap(), tcx)),
+        TyMethodItem(m) => ItemEnum::Method(from_function_method(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)),
@@ -300,17 +302,6 @@ fn convert_abi(a: RustcAbi) -> Abi {
     }
 }
 
-impl FromWithTcx<clean::Function> for Function {
-    fn from_tcx(function: clean::Function, tcx: TyCtxt<'_>) -> Self {
-        let clean::Function { decl, generics, header } = function;
-        Function {
-            decl: decl.into_tcx(tcx),
-            generics: generics.into_tcx(tcx),
-            header: from_fn_header(&header),
-        }
-    }
-}
-
 impl FromWithTcx<clean::Generics> for Generics {
     fn from_tcx(generics: clean::Generics, tcx: TyCtxt<'_>) -> Self {
         Generics {
@@ -555,12 +546,26 @@ impl FromWithTcx<clean::Impl> for Impl {
     }
 }
 
+crate fn from_function(
+    function: clean::Function,
+    header: rustc_hir::FnHeader,
+    tcx: TyCtxt<'_>,
+) -> Function {
+    let clean::Function { decl, generics } = function;
+    Function {
+        decl: decl.into_tcx(tcx),
+        generics: generics.into_tcx(tcx),
+        header: from_fn_header(&header),
+    }
+}
+
 crate fn from_function_method(
     function: clean::Function,
     has_body: bool,
+    header: rustc_hir::FnHeader,
     tcx: TyCtxt<'_>,
 ) -> Method {
-    let clean::Function { header, decl, generics } = function;
+    let clean::Function { decl, generics } = function;
     Method {
         decl: decl.into_tcx(tcx),
         generics: generics.into_tcx(tcx),

src/test/rustdoc/reexport-dep-foreign-fn.rs

@@ -0,0 +1,12 @@
+// aux-build:all-item-types.rs
+
+// This test is to ensure there is no problem on handling foreign functions
+// coming from a dependency.
+
+#![crate_name = "foo"]
+
+extern crate all_item_types;
+
+// @has 'foo/fn.foo_ffn.html'
+// @has - '//*[@class="docblock item-decl"]//code' 'pub unsafe extern "C" fn foo_ffn()'
+pub use all_item_types::foo_ffn;