Skip to content

Remove header field from clean::Function #95096

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 29, 2022
Merged

Remove header field from clean::Function #95096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c4ce9d
Remove header field from clean::Function
GuillaumeGomez Mar 18, 2022
fd48ea0
Add test to ensure foreign functions from dependencies are correctly …
GuillaumeGomez Mar 19, 2022
8071332
Merge ItemKind::TyMethodItem branch with ItemKind::FunctionItem and I…
GuillaumeGomez Mar 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/librustdoc/clean/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,21 +221,14 @@ 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!
let generics = clean_ty_generics(cx, cx.tcx.generics_of(did), predicates);
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 {
Expand Down
81 changes: 7 additions & 74 deletions src/librustdoc/clean/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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 =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed several places where the constness is overrided like this. I don't quite understand why this code existed. Does the new, on-demand computation account for the old overriding behavior?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise, some places used is_const_fn_raw while others used is_const_fn. Is this handled properly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was more or less the same code done by different functions. We have a lot of rustdoc tests for functions and methods to ensure their signature is as expected (I know it because I broke a lot of them while working on this PR).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, seems good to me then. It's good to know the tests worked as intended and failed with previous versions of the code :)

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)
}
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)) => {
Expand All @@ -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));
Expand All @@ -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))
}
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -2192,28 +2139,14 @@ 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);
let args = clean_args_from_types_and_names(cx, decl.inputs, names);
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 })
Expand Down
45 changes: 44 additions & 1 deletion src/librustdoc/clean/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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)]
Expand Down
7 changes: 3 additions & 4 deletions src/librustdoc/html/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -806,14 +805,14 @@ fn assoc_type(
fn assoc_method(
w: &mut Buffer,
meth: &clean::Item,
header: hir::FnHeader,
g: &clean::Generics,
d: &clean::FnDecl,
link: AssocItemLink<'_>,
parent: ItemType,
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)),
Expand Down Expand Up @@ -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)
Expand Down
22 changes: 12 additions & 10 deletions src/librustdoc/html/render/print_item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>"
}
Expand Down Expand Up @@ -453,16 +454,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()
Expand All @@ -478,15 +480,15 @@ 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,
abi = abi,
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),
);
});
Expand Down
Loading