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.
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
#3476base: master
Are you sure you want to change the base?
RFC:
expose-fn-type
#3476Changes from 8 commits
f43c78b
ae0d137
b2c592b
add0ca5
2f7c0bd
f13f239
7b7aebb
32dfb82
adb2c95
19dba36
fc69e90
9549283
9428e88
55b39fc
9a89cad
0804b52
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Although I agree that this is where we want to end up, I think that this RFC is focusing too much on the
impl Trait for [FunctionType]
use-case.Maybe it would make sense to restrict the RFC to just figuring out the syntax for
fn
items? Or make a new RFC for just the syntax? I would be willing to drive that effort, if you think so?Implementing traits for functions have many other nuances, I'll just name a few that I don't think have been explored enough in this RFC:
IntoIterator
for my function? What if 10 years down the road the language wants to start implementingIntoIterator
for function items of the formfn() -> impl Iterator
?fn
pointers.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.
I tried to find some other examples of when you'd want to talk about the function item type instead of just using the function pointer, and I honestly couldn't really find a compelling example, which is supposedly why it doesn't exist yet.
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.
@madsmtm good point with the fn thing! I honestly wonder how that would integrate with generator functions...
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.
Note to self: The standard library uses the macro
impl_fn_for_zst!
to support exactly the use-case of naming a function item, because they store it in generic helper structs, and would like that storage to be zero-cost.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.
I think it would be useful to consider the (imaginary) desugaring of function items into structs. Take for example the following generic function:
This can roughly be implemented with:
This leads me to believe that the syntax for specifying a function item should be much simpler, just
foo<T>
, no precedingfn
. For associated functions,MyType<T>::my_function<U>
should suffice.Note that this doesn't solve
impl Trait
, but as said, you already can't use that instruct
s, so whatever solution is chosen there could just be retrofitted to apply here.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.
wow... that is a lot of thinking going in there... I suppose that you are going to take over the whole scenario with the more general syntax approach?
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.
I think it might make sense to explain this relating to the current reference docs on function items, and how we'd update that given that we now have syntax for talking about the function type.
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.
Do we use turbofish syntax if
MyStruct
takes a generic parameter? e.g.Do we write
fn MyStruct<T>::new
orfn MyStruct::<T>::new
? If we choose the latter, would it cause any inconsistency from thefn send<T>
syntax below (as opposed tofn send::<T>
)?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.
I'll look into that. I also see another problem here: Imagine this
is it
impl<T> fn MyStruct[::]<T>::new
or is it
impl<T> fn MyStruct::new[::]<T>
?
what do you think? is there a prefered situation for this in rust or should we maybe allow both? @SOF3
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.
definitely the former.
MyStruct::new<T>
doesn't make sense because the type parameter is on MyStruct not the associated function. This would go wrong ifnew
itself also accepts type parameters.As for whether to use turbofish, I guess this depends on how the compiler parses this expression. I'm not a parser expert, so this part needs some input from the compiler team.
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.
@SOF3 So I made some tests and would say that it's more "rusty" when we do
MyStruct::<T>::new
instead ofMyStruct<T>::new
(https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=316deb57a2d9552d8284a35fb56db2a0)What do you think?
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.
Yep, but since we are in type context not expression context here, turbofish is probably unnecessary. The turbofish syntax is required for expressions only because of ambiguity with the less-than operator (
<
), but we don't have such ambiguity if we specify thatfn
is always followed by a path instead of an expression.Of course, you might also want an expression there if it is a generic
typeof
operator, but this is not the case for the scope of this RFC.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.
Any potential parsing ambiguity if we are taking a fn pointer on an associated function of a fn pointer? e.g.
Definitely a very bad syntax that must be disallowed, but better specify it in the reference-level explanation.
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.
Yeah this should definitly be forbidden. If we do the
{}
syntax this would befn {fn {foo}::bar}
. Otherwise I'm currently not sure if we would wantfn (fn foo)::bar
orfn <fn foo>::bar
. I'll leave this open until we found a solution (maybe also for the{}
syntax)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.
Are there any existing workarounds for this, e.g. through macros in specific scenarios etc?
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.
I don't think so. I couldn't imagine how you could get behind a specific type of a function at all.
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.
Not familiar with compiler parsing internals, but could also consider
fn {path}
, which is slightly similar to the current compiler diagnostics.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.
isn't that what i already do? or do you mean with explicit
{}
wrapped around?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.
yes
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.
Consider including consistency of display names as well. Currently:
std::any::type_name_of_val(&drop::<()>)
evaluates to"core::mem::drop<()>"
Choices of display include "fn item" (as opposed to "fn pointer" if it is cast to
fn(())
first), the function path and a mix of function pointer +{path}
. It might be more consistent if the syntax eventually adopted in this RFC is consistent with the syntax in compiler diagnostics.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 is a fair point, but i feel like that this will just make more boilerplate. we already specified the function and i dont want to make my impl even longer... on the other side: WAIT A MINUTE! this could solve our "what to do with
impl Trait
type param" problem...So that we write
Ok that would be really cool.
So would you agree that boilerplate is okay here? @SOF3
"Problem" is that this RFC tends to turn into a more generic
type_of
thing where this syntax wouldn't be possible anymore.. but type_of is another story so what you requested might be the solution i neededThere 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.
So i am currently rewriting and for now I will do this partially but use
fn_name
rather than{fn_name}
because i think this could collide when we want to do something like thisI don't know if something like this is planned or if this is already marked as "no" (if so please say it to me so ill change it in the RFC), but if it isn't, then the compiler may not be able to differentiate between
fn() {name} {block}
andfn() {block}
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.
not a fan of this. two type expressions joined together without a delimiter is most likely not acceptable to the compiler team, considering we can't do the same with decl macro inputs either.
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.
what about just
fn {fn_name}
? orfn fn_name(Args)->Return
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.
I don't think that's a problem.
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.
So what about moving the name front?
fn my_func
andfn my_func(prms) -> ret
. But that would not match with the error syntax u sentThere 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.
actually my point was that we should make them consistent, but we could change the diagnostic display instead of the syntax
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.
So is the syntax that I currently have in the RFC ok?
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.
I added a
ToDo's
section at the end where i proposed a syntax change but I'm not sure if it is that well with the generic in the for "structure"? 😅