-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: expose-fn-type
#3476
Open
DasLixou
wants to merge
16
commits into
rust-lang:master
Choose a base branch
from
DasLixou:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
RFC: expose-fn-type
#3476
Changes from 5 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
f43c78b
Add `impl-trait-for-fn`
DasLixou ae0d137
Add PR number
DasLixou b2c592b
Make impl for more described functions clearer
DasLixou add0ca5
Fix mistakes
DasLixou 2f7c0bd
Add more examples for more cases
DasLixou f13f239
Elaborate where to bind the trait to
DasLixou 7b7aebb
Elaborate the RFC to a larger topic
DasLixou 32dfb82
Add examples for functions with generics
DasLixou adb2c95
Update text/3476-expose-fn-type.md
DasLixou 19dba36
Update text/3476-expose-fn-type.md
DasLixou fc69e90
Apply suggestions
DasLixou 9549283
Fix example
DasLixou 9428e88
Update 3476-expose-fn-type.md
DasLixou 55b39fc
Example and Motivation work
DasLixou 9a89cad
Add Todo for consistent syntax
DasLixou 0804b52
I think we can drop those
DasLixou File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
- Feature Name: `impl-trait-for-fn` | ||
- Start Date: 2023-08-20 | ||
- RFC PR: [rust-lang/rfcs#3476](https://github.com/rust-lang/rfcs/pull/3476) | ||
- Rust Issue: N/A | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
Support for implementing traits on functions | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
I was trying to make something similar to bevy's system functions. And for safety reasons, they check for conflicts between SystemParams, so that a function requiring `Res<A>` and `ResMut<A>` [panic](https://github.com/bevyengine/bevy/blob/main/crates/bevy_ecs/src/system/system_param.rs#L421). | ||
|
||
Then after I heard about axum's [`#[debug_handler]`](https://docs.rs/axum/latest/axum/attr.debug_handler.html) I wanted to do something similar to my copy of bevy systems, so that I get compile time errors when there is a conflict. I wanted even more, I wanted to force the user to mark the function with a specific proc attribute macro in order to make it possible to pass it into my code and call itself a system. | ||
|
||
# Guide-level explanation | ||
[guide-level-explanation]: #guide-level-explanation | ||
|
||
As we all know, you can implement a trait for a struct with the following syntax | ||
```rust | ||
struct Timmy; | ||
impl Person for Timmy { | ||
fn greet() { | ||
println!("Hey it's me, Timmy!"); | ||
} | ||
} | ||
``` | ||
And we can also implement a trait for all functions with a specific signature like this | ||
```rust | ||
impl<F> ValidSignature for F | ||
where F: Fn(i32) -> bool | ||
{ | ||
/* ... */ | ||
} | ||
``` | ||
Now we can also implement traits for specific functions only | ||
```rust | ||
fn valid() {} | ||
impl ValidFunction for fn valid { | ||
/* ... */ | ||
} | ||
``` | ||
|
||
# Reference-level explanation | ||
[reference-level-explanation]: #reference-level-explanation | ||
|
||
It gives the possibility to also implement a trait directly scoped to a function instead of generic implementation of multiple. Other than that, it basically behaves the same. | ||
|
||
|
||
When the function has parameters, modifiers or a return type, it should not be included in the impl block, because the path is already unique | ||
```rust | ||
async fn request_name(id: PersonID) -> String { .. } | ||
|
||
impl Requestable for fn request_name { | ||
/* ... */ | ||
} | ||
``` | ||
When the function is for example in a different mod, it should be referenced by its path | ||
```rust | ||
mod sub { | ||
fn sub_mod_fn() { .. } | ||
} | ||
impl OutOfNamesRunnable for fn sub::sub_mod_fn { | ||
/* ... */ | ||
} | ||
``` | ||
Just like how a fn inside an impl block still implements the Fn trait, it should also be possible to implement traits for them | ||
```rust | ||
struct MyStruct; | ||
impl MyStruct { | ||
fn new() -> Self { Self } | ||
} | ||
impl Creatable for fn MyStruct::new { | ||
/* ... */ | ||
} | ||
``` | ||
It should also be possible to implement them via proc attribute macros: | ||
```rust | ||
#[impl_debug_name = "Greeting"] | ||
fn greet() { | ||
/* ... */ | ||
} | ||
// should expand to something like | ||
fn greet() { | ||
/* ... */ | ||
} | ||
impl FnDebugName for fn greet { | ||
fn debug_name() -> &'static str { | ||
"Greeting" | ||
} | ||
} | ||
``` | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
i dont know any | ||
|
||
# Rationale and alternatives | ||
[rationale-and-alternatives]: #rationale-and-alternatives | ||
|
||
I think it's a easy task because we can already implement traits for a group of functions with the same signature, so why shouldn't we also implement single scoped impls? | ||
|
||
# Prior art | ||
[prior-art]: #prior-art | ||
|
||
i dont know any | ||
|
||
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
- Is the syntax good? I feel like we could drop the `fn` to from `impl Trait for fn function` to `impl Trait for function`. | ||
DasLixou marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- What about closures? They don't even have names so targetting them would be quite difficult. I wouldn't want to use the compiler generated mess of a name like `[closure@src/main.rs:13:18: 13:20]`. It would also contain line numbers which would be changing quite often so thats not ideal. | ||
|
||
# Future possibilities | ||
[future-possibilities]: #future-possibilities | ||
|
||
- also make it possible to implement traits for closures directly. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a lot more elaboration on what it actually means.
impl
accepts types. Does this mean thatfn valid
is now a type referring to the fndef ZST? Can it be used anywhere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, what do you mean by fndef ZST?
And yes, as to my knowledge this is "makes it" a type just as the implementation of
Fn
also requires it to be.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every function already has an implicit type. So I assume that what you mean is that
fn x
is syntax to refer to the type ofx
. The RFC should elaborate that, instead of just being around trait impls.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry 😅 yes thats exactly what I want. Also: in the unresolved questions tab i wasnt sure if we could drop the
fn
in the impl block. But I think it is unique, right? So what do you think about dropping thefn
?Also: when you say elaborating to the type and not just about trait impls, another question came to my mind: Do we maybe also want to impl directly on a function (I currently work on smth that would really benefit from such a thing)?
Then together with a dropped fn this would look smth like
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nilstrieb can this be marked as resolved?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically what I'm saying is that "allow impls for function definition types" is the wrong way to approach this. The real feature here is being able to refer to function definition types, so the RFC should focus on that. Mentioning that function definition types are considered local for coherence and can therefore be used in trait impls is then just a small sidenote.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nilstrieb ohhh... thats a great point but I'm not sure if i can rewrite it for it that much because the motivation / goal is more on trait impls directly. But ill try to make it focus more on this point.
Thanks for the feedback 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nilstrieb may you recheck the RFC? I rewrote it but i'm unsure if it's still too focused on the impl thing 😅