Split out common behavior of `FnStatic` and `FnPtr` into a new trait: `FnAddr`

[WaffleLapkin]

Proposal

Problem statement

ACP #589 (tracked at rust-lang/rust#148768) introduced two new traits:

// core::ops

// Implemented only for function pointers.
pub unsafe trait FnPtr {
    fn addr(self) -> usize;
    fn as_ptr(self) -> NonNull<Code>;
    unsafe fn from_ptr(NonNull<Code>) -> Self;
}

// Implemented for all types that coerce to function pointers.
pub unsafe trait FnStatic<Args>: Fn<Args> {
    type FnPtr: FnPtr;
    fn as_fn() -> Self::FnPtr;
    fn addr() -> usize;
    fn as_ptr() -> NonNull<Code>;
    extern "rust-call" fn call_static(args: Args) -> Self::Output;
}

They share some functionality that can be refactored into a separate trait that could be used for both function pointers and types which coerce to them (function definitions and closures without captures), but can't be since the traits are mutually exclusive.

Motivating examples or use cases

See ACP #836 that would like to be able to get an address of both function pointers and function definitions.

Solution sketch

Add a new trait with the shared functionality:

// Implemented for function pointers and types that coerce to function pointers
pub unsafe trait FnAddr {
    fn addr(self) -> usize;
    fn as_ptr(self) -> NonNull<Code>;
}

And then use it as the super trait of both FnStatic and FnPtr:

 // Implemented only for function pointers.
-pub unsafe trait FnPtr {
+pub unsafe trait FnPtr: FnAddr {
-    fn addr(self) -> usize;
-    fn as_ptr(self) -> NonNull<Code>;
     unsafe fn from_ptr(NonNull<Code>) -> Self;
 }

 // Implemented for all types that coerce to function pointers.
-pub unsafe trait FnStatic<Args>: Fn<Args> {
+pub unsafe trait FnStatic<Args>: Fn<Args> + FnAddr {
     type FnPtr: FnPtr;
     fn as_fn() -> Self::FnPtr;
-    fn addr() -> usize;
-    fn as_ptr() -> NonNull<Code>;
     extern "rust-call" fn call_static(args: Args) -> Self::Output;
 }

Alternatives

• Keep things as is?

Links and related work

• Tracking Issue for FnStatic and FnPtr::addr/from_ptr/as_ptr and Code rust#148768 / API for explicitly getting an opaque pointer of a function #589
• ptr::fn_addr #836
• Add forbidden function casts RFC rfcs#3526

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[programmerjake]

if you have fn f<T: FnStatic()>(), previously you could easily just call T::addr(), now that's less ergonomic since you have to call T::as_fn().addr()

[programmerjake]

maybe we should also adjust the associated type trait bounds while we're at it like so:

pub unsafe trait FnStatic<Args>: Fn<Args> + FnAddr {
    type FnPtr: FnPtr + Fn<Args, Output = Self::Output>;
    ...
 }

[WaffleLapkin]

if you have fn f<T: FnStatic()>(), previously you could easily just call T::addr(), now that's less ergonomic since you have to call T::as_fn().addr()

Ah, I did not notice the FnStatic ones don't take self...

1. One option would be to add const VALUE: Self like Tracking Issue for FnStatic and FnPtr::addr/from_ptr/as_ptr and Code rust#148768 (comment) proposed.

2. Another would be to keep FnStatic functions.

[programmerjake]

if you have fn f<T: FnStatic()>(), previously you could easily just call T::addr(), now that's less ergonomic since you have to call T::as_fn().addr()

Ah, I did not notice the FnStatic ones don't take self...

1. One option would be to add const VALUE: Self like Tracking Issue for FnStatic and FnPtr::addr/from_ptr/as_ptr and Code rust#148768 (comment) proposed.
2. Another would be to keep FnStatic functions.

we could do both of those, that way we can do both T::addr() and opt.map(T::VALUE)

[Amanieu]

I would actually make several more changes here.

// Implemented for function pointers and types that coerce to function pointers
pub unsafe trait FnAddr {
    fn addr(self) -> usize;
    fn as_ptr(self) -> NonNull<Code>;
}

// Implemented only for function pointers.
pub unsafe trait FnPtr: FnAddr {
    unsafe fn from_ptr(NonNull<Code>) -> Self;
}

// Implemented for all types that coerce to function pointers.
pub unsafe trait FnStatic: FnAddr {
    const INSTANCE: Self;
    type FnPtr: FnPtr;
    fn as_fn(self) -> Self::FnPtr;
}

Notable changes:

• Adds INSTANCE as a way to instantiate a FnStatic out of thin air.
• Adds self to as_fn: you can just use INSTANCE now.
• Removed FnStatic: Fn. This is important since unsafe fn doesn't actually implement Fn, yet we still want to be able to get a function pointer from them.

Also these should all be sealed traits, at least initially. We can discuss unsealing them once we stabilize implementing Fn on arbitrary types.

[programmerjake]

there should be an easier way to call it, with that API you'd have to do this monstrosity:

pub fn call_it<T: FnStatic<FnPtr = fn(u32, i32, f32) -> String>>(a: u32, b: i32, c: f32) -> String {
    T::INSTANCE.fn_ptr()(a, b, c)
}

[Amanieu]

You can add the Fn bound separately.

pub fn call_it<T: FnStatic + Fn(u32, i32, f32) -> String>(a: u32, b: i32, c: f32) -> String {
    T::INSTANCE(a, b, c)
}

[WaffleLapkin]

Since #836 got closed in favor of this, I would also like to propose adding the standalone function:

// core::ptr
pub fn fn_addr<F: FnAddr>(f: F) -> usize {
    f.addr()
}

As a convenience / have a smaller surface API that can be stabilized separately from all the complicated traits here (and support the work on rust-lang/rfcs#3526).

---

@Amanieu why make as_fn take self? Wouldn't it be both easier and nicer to have const FN_PTR: Self::FnPtr?

Otherwise your further changes seem reasonable :)

[Amanieu]

So that you can do my_func.as_fn() without needing to name the type of my_func.

[WaffleLapkin]

I guess there are two use-cases here. If you are working with an explicit function, .as_fn() is nicer, but if you parametrize over T: FnStatic, then a constant is nicer. We could maybe have both?