Remove clean::Function::header and calculate it on-demand

[yuvaldolev]

Addresses #89673

Remove clean::Function::header and calculate it on-demand based on the DefId.

r? @jyn514

[jyn514]

This is awesome ❤️ Thank you for working on this!

[jyn514]

 error: internal compiler error: compiler/rustc_ty_utils/src/ty.rs:493:9: asyncness: expected fn-like node but got `DefId(0:36 ~ foo[cf51]::PubTrait::function)`

thread 'rustc' panicked at 'Box<dyn Any>', compiler/rustc_errors/src/lib.rs:1169:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Hmm, can you run this locally with RUST_BACKTRACE set to see how it's happening? I wonder if we need to avoid calling asyncness for associated functions?

[jyn514]

Maybe also look at fn_kind to see why it returns None: https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_hir/hir.rs.html#3267-3291

[yuvaldolev]

@rustbot label -S-waiting-on-author +S-waiting-on-review

[yuvaldolev]

@rustbot label +S-waiting-on-author -S-waiting-on-review

[jyn514]

This looks mostly good from me, but I'd like to have someone from the compiler team review - @petrochenkov would you be willing to look over the HIR changes?

[jyn514]

This new variant shouldn't be necessary - if you look at ItemFn above it allows for an ABI field.

[yuvaldolev]

But it requires a FnHeader, which is not available for ForeignItem functions.
Should I just create a header with all items such as asyncness and constness set to false?

[jyn514]

This is a good question for petrochenkov, I'm not sure. https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/hir/enum.ForeignItemKind.html has a FnDecl but not FnHeader. Maybe try implementing that for now and we can change it later if necessary.

[petrochenkov]

The HIR changes do not look right, the new variant is not integrated into the HIR visitor (intravisit.rs), so it's not constructed by the compiler, only by rustdoc.

As you can see FnKind is currently used only for functions with bodies (it's not used for trait functions without bodies TraitItemKind::Fn(_, TraitFn::Required(_)), for example) and is a part of fn visit_fn logic which includes visiting of function bodies.
You can try migrating HIR visitor to visit_fn/FnKind for functions without bodies as well (I can't predict whether it will be an improvement or not), but I suggest doing it in a PR separate from the rustdoc changes.

r? @jyn514

[jyn514]

@petrochenkov how can we find this information for functions without bodies without using fn_sig? In particular, asyncness, constness, and abi - I guess we can just hard-code that trait and foreign functions aren't async for now, but I'm not sure how to get the constness or abi.

[petrochenkov]

The constness is always non-const for foreign functions, and the ABI can be taken from their parent item - the extern block (at least locally). It's better to look for examples of their use in the compiler and do the same thing, I'm pretty sure codegen takes the ABI of foreign functions from somewhere.

[bors]

☔ The latest upstream changes (presumably #91291) made this pull request unmergeable. Please resolve the merge conflicts.

[camelid]

how can we find this information for functions without bodies without using fn_sig? In particular, asyncness, constness, and abi - I guess we can just hard-code that trait and foreign functions aren't async for now, but I'm not sure how to get the constness or abi.

I'm probably missing some context, but what's wrong with using tcx.fn_sig()? It looks like you can get unsafety and ABI from it.

[jyn514]

@camelid it panics on trait functions without a default implementation

[camelid]

But don't those still have signatures?

[JohnCSimon]

Ping from triage:
@yuvaldolev can you post your status on this PR?
If it's ready for review send a message containing
@rustbot ready

[JohnCSimon]

@yuvaldolev
Ping from triage:
Closing this due to inactivity because the author hasn't posted in several months.
Please reopen when you are ready to continue with this. Thank you.

@rustbot label: +S-inactive

[GuillaumeGomez]

Just gave a try to this issue and I think this is the wrong approach. I'll open a PR shortly once I'm done with the cleanup.