Implement Fn traits for safe functions of any ABI

[Jules-Bertholet]

View all comments

This brings the behavior of the language in line with its documentation:

rust/library/core/src/primitive_docs.rs

Lines 1930 to 1931 in 1a833e1

	/// In addition, all *safe* function pointers implement [`Fn`], [`FnMut`], and [`FnOnce`], because
	/// these traits are specially known to the compiler.

ABIs that are not callable, or that are internal implementation details, are excluded.

@rustbot label T-lang needs-fcp A-ABI A-extern-fn I-lang-nominated

[rustbot]

miri is developed in its own repository. If the Miri part of this change can be broken out, consider making this change to rust-lang/miri instead. However, if Miri needs adjusting for rustc changes, just ignore this message.

cc @rust-lang/miri

[rustbot]

r? @jackh726

rustbot has assigned @jackh726.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 74 candidates
• Random selection from 16 candidates

[ChayimFriedman2]

Don't you need to generate shims or Happy Things will happen?

[Jules-Bertholet]

I don't think so? The basic tests I added work fine, and Miri doesn't complain. But I don't really know that part of the codebase, am relying on Cunningham's Law

[ChayimFriedman2]

I suggest you check with &dyn Fn(), things are more likely to break there.

[Jules-Bertholet]

Nothing broke…

[ChayimFriedman2]

🤷 Maybe it is fine...

[ChayimFriedman2]

Although the practical ABI for &isize and isize is identical for C and Rust, so it will compile fine and I don't know if Miri can detect a mismatch - maybe try something like [usize; 2], that appears to have a different ABI between Rust and C?

[ds84182]

Probably should make sure the function that calls the &dyn Fn is not inlined.

[RalfJung]

Yeah I'd expect the rust-call / argument (un)tupling to go wrong here as that needs the Rust ABI.

Does this actually solve a problem? If not I'd rather update the docs.

[Jules-Bertholet]

Modified the tests to use a function that I confirmed with Godbolt has a completely different "Rust" vs. "C" ABI. And everything still works

[Jules-Bertholet]

Does this actually solve a problem?

With new ABIs like "tail", extern is not just for FFI anymore.

[RalfJung]

I guess what happens is that we synthesize a call/call_once body for the traits when implementing them for a fn item, which discards the self argument, and which in this case also serves as an ABI adapter?

But someone should dig into where that happens.

[Jules-Bertholet]

Suggested change

	!self.c_variadic() && self.safety().is_safe() && self.abi().is_fn_trait_compatible()
	self.safety().is_safe() && self.abi().is_fn_trait_compatible()

We could try to remove the !self.c_variadic() condition. But there's not much point, as any actual use of variadics is necessarily unsafe. It would also be a bunch of work, as the trait solver currently assumes no type has more than one impl of each Fn trait.

View changes since the review

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.