-
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: Allow coercing non-capturing closures to function pointers. #1558
Conversation
This is my first RFC — should I modify 0401-coercions as well? EDIT: After a short conversation on |
I much prefer the version that doesn't introduce any new syntax. How would this interact with the existing closure types? Would the following code compile? fn main() {
let x = |var: &mut u32| {};
foo(&x);
bar(x);
}
fn foo(_: &Fn(&mut u32)) {}
fn bar(_: fn(&mut u32)) {} I suppose what this is asking is, is the type of fn main() {
let x: fn(&mut u32) = |var| {};
// alternatively:
let x = |var| {} as fn(&mut u32);
bar(x);
}
fn bar(_: fn(&mut u32)) {} |
Whether a closure's actual type becomes For example, this will compile and run fine: fn foobar(_: &mut u32) {}
fn main() {
let x = foobar;
foo(&x);
bar(x);
}
fn foo(_: &Fn(&mut u32)) {}
fn bar(_: fn(&mut u32)) {} Strictly according to the RFP, though, the variable would be a closure that later coerces to a function pointer, which is probably for the better to prevent any breakage of silly unsafe code. |
Does the type system need some way to distinguish between normal closures and "bare" closures? Can we achieve all this without runtime checks? |
Yes, ideally the type system would keep track of closures that do not capture their environment. Runtime checking is not a solution at all, in my opinion, nor is it necessary. |
Am I right in that there would be no semantic difference between the existing |
After a quick check of rust's source, you're right. There really isn't any way to transmute a non-capturing closure (TIL it's zero-sized!), so no unsafe code could be broken by this. In that case, there wouldn't be any downside after all to simply using a |
I remember I used to talk about this with cc @P1start over a year ago; I think @eddyb too, perhaps regarding bounds-targetting coercions? I don't remember. It'd be nice to have in some form. I also would prefer it to be automatic if possible. |
This can all be done easily at the type level: allow coercions from closures with no captures (a fact which is recorded in their very type). The coercion you want is called a "reification" coercion and it's what turns functions into As for trans, well, the |
FWIW I wouldn't mind having |
@glaebhoerl FWIW there are no more items with |
[EDIT: I've realized the double boxing trick might be avoidable and boxing a ZST is a no-op, so ignore this.] It could be useful to specialize for non-capturing closures (together with usual
|
This RFC is hasn't seen much activity in a long time, so it'd be good to get it resolved. I'm mildly in favor of merging, but I have concerns that are enough that I haven't asked rfcbot to fcp it. Here are my thoughts. Breaking changesMy biggest concern is that this could make adding a captured variable to a closure a breaking change in a library; it would be very unfortunate for users to have to determine if subtle changes like that could break a downstream crate, rather than just always knowing that they're fine. To me, if this were the case, this RFC would be a no go. Fortunately, I can't think of a way to expose a closure to clients except through the If there are no ways to expose values of the anonymous type to clients, then this should be fine. But I want to make sure we've thought about this. Accidental guaranteesA more subtle concern is one in which the user accidentally guarantees this coercion, and then later is forced to break their API to perform the upgrade they intended. This basically introduces two ways to return closures, one of which is strictly more general than the other: fn incr() -> fn(i32) -> i32 {
|x| x + 1
}
fn incr() -> impl Fn(i32) -> i32 {
|x| x + 1
} My concern is that the first of these is more natural to write than the second. Users could write functions which return This seems a pretty serious trade off to me - we want the language to just enable you to do the right thing. The current behavior is unergonomic and confounding, but I'm not 100% sure that the footguns this behavior would introduce are less of a problem. |
@wycats So have |
@glaebhoerl assuming that @ was at me 😉, I think the downsides of a new lambda syntax are greater than the downsides of the current behavior or the RFC. |
Oh, sorry! And I made the opposite mistake just a couple of hours ago... (but thankfully twitter does the reply-tos automatically;) |
|
For what it's worth, I agree, and I've added a comment to the RFC making that explicitly clear.
I agree that this could happen, and I've added this to the drawbacks section of the RFC, but we do already have situations in which a Rust programmer might take the "easy route" and accidentally constrain their own API. (See example in RFC.) I do think that to some extent, we expect a Rust programmer to be aware of the limitations they create for themselves. In this case we're talking about a much more niche feature than |
@rfcbot fcp close We discussed this at the lang meeting this week. We considered both the coercion option the RFC proposed and the alternative function literal syntax, as well as allowing only explicit In general, folks felt most favorably about the function syntax (though speaking personally I think I'm more against it than other people were), but there were concerns about how it would work. If the function syntax infers your types (especially the return type), it behaves significantly differently from named function declarations. If it doesn't, it doesn't save you much in comparison to writing out a named function. No one loves the current situation, but we felt that the language as it exists right now does not strongly encourage you to use fn pointers, so its not clear how often some new syntax or coercion would actually be used for its intended purpose. |
Team member @withoutboats has proposed to close this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
Hmm, it seems like this is fundamentally a judgement call. There are definitely legitimate cases where you actively want a function pointer and not a closure (e.g., interfacing with C code, as well as wanting to guarantee that there is no environment, though that latter case could be achieved by adding some kind of "zero-sized" trait). But making those cases more ergonomic comes at the cost of making it easier to write the wrong thing in your public APIs. I have to admit I am not so worried about that. I think beginners probably will make mistakes, but as they learn rust, they will learn the better fix, and their APIs will improve and be updated. It seems likely that many early APIs also make dubious choices about whether to take ownership of something or to borrow it, and so forth, as well. |
@nikomatsakis I agree. I'd also note that I'm going to go ahead and move toward acceptance of this RFC, given recent discussion, but of course @withoutboats and others should feel free to raise concerns. @rfcbot fcp merge |
Team member @aturon has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
@withoutboats As long as we sufficiently emphasize the fact that function pointers are completely self-contained, I don't anticipate this to be a problem. |
Ping @nrc @pnkfelix @withoutboats |
I've marked this reviewed to merge. I think my biggest concern has to do with stabilizing this before |
I have marked this reviewed to merge, but I have to admit that I was confused when re-reading the Summary sentence:
What's up with that definition of "non-capturing"? I would have thought the definition would be broader, to include referencing local variables (even without moving nor cloning them). (And I'm pretty sure that that broader definition of "non-capturing" must be what everyone else is using, because the only way the proposal makes sense is for the generated code to not have any dynamic environment that it needs to access...) |
Yeah, "capture by reference" is pretty standard at this point, the parenthesis is the confusing bit there IMO. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
Ah, that was an oversight. I'll fix the summary. |
The final comment period is now complete. |
The final comment period has elapsed without any additional commentary, so at this point we're pretty much ready to merge the RFC. However, I noticed that @nikomatsakis has an outstanding review request and I want to make sure he feels the issue has been resolved at this point. |
Ah, hmm, yes I forgot about that. So my concern was that I would expect this coercion (from closure to fn ptr) to be driven by the top-down coercion process. I wrote this:
And @aturon replied:
Then @eddyb wrote:
I suppose that thinking about it more there is another alternative. We could consider this a variant of upvar inference (the thing that decides, based on how it uses its upvars, when a closure implements I'd be ok with that, I guess, though I'm not totally psyched about it, just because I feel like it's yet another ad-hoc bit of obligation tracking we'd be doing in the type-checker (rather than having it all consolidated into a new-and-improved trait system sort of thing). Just examining the expected type at the time when we create the closure feels way easier. But yeah I can go either way I guess. It's fine. |
The set of "upvars" is already known before type-checking begins, so that shouldn't be a problem. |
Per post-FCP discussion, this RFC has been merged! |
The "Rendered" link is broken |
Accepted RFCs are available at https://rust-lang.github.io/rfcs/1558-closure-to-fn-coercion.html |
Rendered
Closes #1555.