Skip to content
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.

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

A FunctionPointer trait to represent all fn types #23

Closed
nikomatsakis opened this issue Jun 11, 2020 · 5 comments
Closed

A FunctionPointer trait to represent all fn types #23

nikomatsakis opened this issue Jun 11, 2020 · 5 comments
Labels
help wanted We're looking for volunteers! major-change Major change proposal

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 11, 2020

WARNING

The Major Change Process was proposed in RFC 2936 and is not yet in
full operation. This template is meant to show how it could work.

Proposal

Summary

Create a FunctionPointer trait that is "fundamental" (in the coherence sense) and built-in to the compiler. It is automatically implemented for all fn types, regardless of any other details (ABI, argument types, and so forth).

Motivation

You can't write an impl that applies to any function pointer

It is not possible to write an impl that is parameteric over all fn types today. This is for a number of a reasons:

  • You can't write an impl that is generic over ABI.
  • You can't write an impl that is generic over the number of parameters.
  • You can't write an impl that is generic over where binding occurs.

We are unlikely to ever make it possible to write an impl generic over all of those things.

And yet, there is a frequent need to write impls that work for any function pointer. For example, it would be nice if all function pointers were Ord, just as all raw pointers are Ord.

To work around this, it is common to find a suite of impls that attempts to emulate an impl over all function pointer types. Consider this code from the trace crate, for example:

trace_acyclic!(<X> fn() -> X);

trace_acyclic!(<A, X> fn(&A) -> X);
trace_acyclic!(<A, X> fn(A) -> X);

trace_acyclic!(<A, B, X> fn(&A, &B) -> X);
trace_acyclic!(<A, B, X> fn(A, &B) -> X);
trace_acyclic!(<A, B, X> fn(&A, B) -> X);
trace_acyclic!(<A, B, X> fn(A, B) -> X);
...

Or this code in the standard library.

Bug fixes in rustc endanger existing approaches

As part of the work to remove the leak-check in the compiler, we introduced a warning about potential overlap between impls like

impl<T> Trait for fn(T)
impl<U> Trait for fn(&U)

This is a complex topic. Likely we will ultimately accept those impls as non-overlapping, since wasm-bindgen relies on this pattern, as do numerous other crates -- though there may be other limitations. But many of the use cases where those sorts of impls exist would be better handled with an opaque FunctionPointer trait anyhow, since what they're typically really trying to express is "any function pointer" (wasm-bindgen is actually somewhat different in this regard, as it has a special case for fns that taken references that is distinct from fns that taken ownership).

Proposal

Add in a trait FunctionPointer that is implemented for any fn type (but only fn types). It is built-in to the compiler, tagged as #[fundamental], and does not permit user-defined implementations. It offers a core operation, as_usize, for converting to a usize, which in turn can be used to implement the various built-in traits:

#[fundamental]
pub trait FunctionPointer: Copy + Ord + Eq {
    fn as_usize(self) -> usize; // but see alternatives below
}

impl<T: FunctionPointer> Ord for T {

}

impl<T: FunctionPointer> PartialEq for T {
    fn eq(&self, other: &T) -> bool {
        self.as_usize() == other.as_usize()
    }
}

impl<T: FunctionPointer> Eq for T { }

In terms of the implementation, this would be integrate into the rustc trait solver, which would know that only fn(_): FunctionPointer.

As with Sized, no user-defined impls would be permitted.

Concerns and alternative designs

  • Will we get negative coherence interactions because of the blanket impls?
    • I think that the #[fundamental] trait should handle that, but we have to experiment to see how smart the trait checker is.
  • Will function pointers always be representable by a usize?
    • On linux, dlsym returns a pointer, so in practice this is a pretty hard requirement.
    • Platforms that want more than a single pointer (e.g., AVR) generally implement that via trampolines or other techniques.
    • It's already possible to transmute from fn to usize (or to cast with as), so to some extent we've already baked in this dependency.
  • Seems rather ad-hoc, what about other categories of types, like integers?
    • Fair enough. However, function pointers have some unique challenges, as listed in the motivation.
    • We could pursue this path for other types if it proves out.
  • What about dyn Trait and friends?
    • It's true that those dyn types have similar challenges to fn types, since there is no way to be generic over all the different sorts of bound regions one might have (e.g., over for<'a> dyn Fn(&'a u32) and so forth).
    • Unlike fn types, their size is not fixed, so as_usize could not work, which might argue for the "extended set of operations" approach.
    • Specifically one might confuse &dyn Fn() for fn().
    • Perhaps adding a fundamental DynType trait would be a good addition.
  • What about FnDef types (the unique types for each function)
    • If we made FunctionPointer apply to FnDef types, that can be an ergonomic win and quite useful.
    • The as_usize could trigger us to reify a function pointer.
    • The trait name might then not be a good fit, as a FnDef is not, in fact, a function pointer, just something that could be used to create a function pointer.
  • What about const interactions?
    • I think we can provide const impls for the FunctionPointer trait, so that as_usize and friends can be used from const functions

Alternative designs

Instead of the as_usize method, we might have methods like ord(Self, Self) -> Ordering that can be uesd to implement the traits. That set can grow over time since no user-defined impls are permitted.

This is obviously less 'minimal' but might work better (as noted above) if we extend to exotic platforms or for dyn types.

However, it may be that there is extant code that relies on converting fn pointers to usize and such code could not be converted to use fn traits.

The Major Change Process

Once this MCP is filed, a Zulip topic will be opened for discussion. Ultimately, one of the following things can happen:

  • If this is a small change, and the team is in favor, it may be approved to be implemented directly, without the need for an RFC.
  • If this is a larger change, then someone from the team may opt to work with you and form a project group to work on an RFC (and ultimately see the work through to implementation).
  • Alternatively, it may be that the issue gets closed without being accepted. This could happen because:
    • There is no bandwidth available to take on this project right now.
    • The project is not a good fit for the current priorities.
    • The motivation doesn't seem strong enough to justify the change.

You can read [more about the lang-team MCP process on forge].

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@nikomatsakis nikomatsakis added help wanted We're looking for volunteers! major-change Major change proposal labels Jun 11, 2020
@rustbot
Copy link
Collaborator

rustbot commented Jun 11, 2020

Error: This repository is not enabled to use triagebot.
Add a triagebot.toml in the root of the master branch to enable it.

Please let @rust-lang/release know if you're having trouble with this bot.

@nikomatsakis nikomatsakis changed the title (My major change proposal) A FunctionPointer trait to represent all fn types Jun 11, 2020
@nikomatsakis
Copy link
Contributor Author

I have tagged this as "help-wanted" because I'm looking for someone to help push this design over the finish line. I would be happy to serve as the "liaison", in other words, but we need a lea! You can find some earlier discussion on this zulip stream.

@PoignardAzur

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Jun 16, 2020

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

@nikomatsakis
Copy link
Contributor Author

Discussed in the lang-team meeting:

  • There is a problem here to be solved, but it might be nice if we were able to permit more operations than just "treat as a usize".
  • If the compiler changes wind up breaking people's code, this goes up in priority.
  • We could implement it as an "unstable implementation detail" in libstd to experiment in the meantime.

But for now we will close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted We're looking for volunteers! major-change Major change proposal
Projects
None yet
Development

No branches or pull requests

4 participants