-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Allow impl Fn() -> impl Trait
#93082
Conversation
This allows writing the following function signatures: ```rust fn f0() -> impl Fn() -> impl Trait; fn f1(_: impl Fn() -> impl Trait); fn f2<F: Fn() -> impl Trait>(_: F); fn f3() -> &'static dyn Fn() -> impl Trait; fn f4(_: &dyn Fn() -> impl Trait); ``` All of the above are already allowed with common traits and associated types, there is no reason why `Fn*` traits should be special in this regard.
cc @rust-lang/lang this seems like a very straight-forward stabilization, especially considering there are workarounds that allow the same thing. |
Eventually I'd like this to work: fn print_some_things_times_two(times_two: impl Fn(impl Add) -> impl Debug) {
println!("{:?}", times_two(1u8));
println!("{:?}", times_two(1u32));
}
print_some_things_times_two(|x| x + x); Note that the return |
@cramertj shouldn't the type be Note that this actually requires type-HRTB (or is it HKT?...) (as well as generic closures, oh wow): for<T>
where
T: Add<Output = impl Debug> + Copy
impl Fn(T) -> impl Debug Personally, I think that fn f(_: impl Fn(impl Tr)) {} Should desugar to fn f<F, A>(_: F)
where
F: Fn(A),
A: Tr,
{} As this is more consistent with But I believe that this PR is compatible with both options. |
@WaffleLapkin It's exactly type- |
So, if we decide to do this, we're deciding on an associativity for For instance, what happens if you write I would expect it to mean Could you please add a test case demonstrating that associativity? Given such a test case, I'd be happy to propose this for FCP. |
(Removing T-compiler in order to do a T-lang FCP) |
@WaffleLapkin Thanks, that test case looks great! @rust-lang/lang: Do we want to commit to right-associativity (rather than non-associativity) for @rfcbot merge |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
Once a majority of reviewers approve (and at most 2 approvals are outstanding), 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. |
Would it be possible to update the documentation at https://github.com/rust-lang/reference/blob/master/src/types/impl-trait.md to make it clear this is allowed (and any other details that seem worthy to add)? |
Will this allow: fn new() -> impl for<'a> FnOnce(&'a i32) -> (impl Future<Output = &'a i32> + 'a) {
|x| async move { x }
} instead of the current: fn old() -> impl for<'a> FnOnce(&'a i32) -> Box<dyn Future<Output = &'a i32> + 'a> {
|x| Box::new(async move { x })
} ? |
@rfcbot concern forward-compatibility-with-apit-type-hrtb See #93082 (comment) |
@ehuss it should be possible to update the reference, yes (it also doesn't seem to mention @veber-alex this will allow the following: fn new() -> impl FnOnce(i32) -> impl Future<Output = i32> {
|x| async move { x }
} Your example with lifetime currently segfaults compiler, see #88236. But when this bug will be fixed, your example will be fine too. |
Actually I'm now thinking, when #88236 is fixed, will |
We have been specifically ruling this out because we wanted a clear determination of what it means to use I do want to permit this, but I want to incorporate it into https://rust-lang.github.io/impl-trait-initiative/ with a clearly documented set of rules. We should also permit @rfcbot concern niko-wants-to-do-a-good-review |
@nikomatsakis If I understand correctly, in For example |
@WaffleLapkin ok, +1, and I agree they should have the same meaning. |
I ran into a segfault when trying to return very deeply nested function compositions (with a depth of 560). I filed #93237 for that. |
Has anyone nailed down the answer to Josh’s question about the desired spec for associativity? |
I've created a PR to the reference, that clarifies existing and added in this PR rules on where |
That argument is persuasive to me. Just to confirm, it's only about output position here? It's not enabling |
@rfcbot resolve niko-wants-to-do-a-good-review @rfcbot concern interaction-of-parens Discussing in the @rust-lang/lang meeting and came up with some concerns. I am trending against stabilizing now, based on these. Let me try to spell it out. The first concern is this: I have generally wanted |
@rfcbot concern what-should-return-in-arg-mean We discussed what
@cramertj pointed out that what you really want is probably something like I can whip up some tests here. |
BTW, @cramertj pointed out that this particular case would be ok: fn foo() -> impl Fn() -> impl Debug That seems probably true, although it is still somewhat more limited than what it could eventually support. |
@scottmcm Yes, here is the test for this:
|
Well, it's actually pretty easy to come up with an example, where we want higher-bound regions --
I don't even think that we can currently create a type that would satisfy such bound, at least on stable.
TIL that you can't write |
> because having an inconsistency between Fn() -> impl Trait and Fn<(), Output = impl Trait> seems very unpleasant. I thought that at first, but then I remembered that -- indeed -- that is part of the point of |
So, it seems like we don't want If I understood the conversation right, return position So, am I right, that the way forward is to update this PR to only allow return-position- |
@rfcbot fcp cancel We discussed this in our @rust-lang/lang meeting today. The conclusion was that we would like to do two things:
@WaffleLapkin if you are up to do the latter, that would be great! 💜 I'm going to close this PR for now, though, so that we can have a fresh discussion once you've got that ready. |
One potential source of weirdness is the possible ambiguity created by lifetimes in the |
@nikomatsakis sure, I'll try to update the PR to allow specifically RPITIRPIT :) |
The return frog joins the turbo fish in guiding the young crab in this new exciting adventure of finding the never type |
New PR that only allows |
…errors Allow `impl Fn() -> impl Trait` in return position _This was originally proposed as part of rust-lang#93082 which was [closed](rust-lang#93082 (comment)) due to allowing `impl Fn() -> impl Trait` in argument position._ This allows writing the following function signatures: ```rust fn f0() -> impl Fn() -> impl Trait; fn f3() -> &'static dyn Fn() -> impl Trait; ``` These signatures were already allowed for common traits and associated types, there is no reason why `Fn*` traits should be special in this regard. `impl Trait` in both `f0` and `f3` means "new existential type", just like with `-> impl Iterator<Item = impl Trait>` and such. Arrow in `impl Fn() ->` is right-associative and binds from right to left, it's tested by [this test](https://github.com/WaffleLapkin/rust/blob/a819fecb8dea438fc70488ddec30a61e52942672/src/test/ui/impl-trait/impl_fn_associativity.rs). There even is a test that `f0` compiles: https://github.com/rust-lang/rust/blob/2f004d2d401682e553af3984ebd9a3976885e752/src/test/ui/impl-trait/nested_impl_trait.rs#L25-L28 But it was changed in [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-ccecca938872d65ffe8cd1c3ef1956e309fac83bcda547d8b16b89257e53a437R37) to test the opposite, probably unintentionally given [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-5a02f1ed43debed1fd24f7aad72490064f795b9420f15d847bac822aa4621a1cR476-R477). r? `@nikomatsakis` ---- This limitation is especially annoying with async code, since it forces one to write this: ```rust trait AsyncFn3<A, B, C>: Fn(A, B, C) -> <Self as AsyncFn3<A, B, C>>::Future { type Future: Future<Output = Self::Out>; type Out; } impl<A, B, C, Fut, F> AsyncFn3<A, B, C> for F where F: Fn(A, B, C) -> Fut, Fut: Future, { type Future = Fut; type Out = Fut::Output; } fn async_closure() -> impl AsyncFn3<i32, i32, i32, Out = u32> { |a, b, c| async move { (a + b + c) as u32 } } ``` Instead of: ```rust fn async_closure() -> impl Fn(i32, i32, i32) -> impl Future<Output = u32> { |a, b, c| async move { (a + b + c) as u32 } } ```
…errors Allow `impl Fn() -> impl Trait` in return position _This was originally proposed as part of rust-lang#93082 which was [closed](rust-lang#93082 (comment)) due to allowing `impl Fn() -> impl Trait` in argument position._ This allows writing the following function signatures: ```rust fn f0() -> impl Fn() -> impl Trait; fn f3() -> &'static dyn Fn() -> impl Trait; ``` These signatures were already allowed for common traits and associated types, there is no reason why `Fn*` traits should be special in this regard. `impl Trait` in both `f0` and `f3` means "new existential type", just like with `-> impl Iterator<Item = impl Trait>` and such. Arrow in `impl Fn() ->` is right-associative and binds from right to left, it's tested by [this test](https://github.com/WaffleLapkin/rust/blob/a819fecb8dea438fc70488ddec30a61e52942672/src/test/ui/impl-trait/impl_fn_associativity.rs). There even is a test that `f0` compiles: https://github.com/rust-lang/rust/blob/2f004d2d401682e553af3984ebd9a3976885e752/src/test/ui/impl-trait/nested_impl_trait.rs#L25-L28 But it was changed in [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-ccecca938872d65ffe8cd1c3ef1956e309fac83bcda547d8b16b89257e53a437R37) to test the opposite, probably unintentionally given [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-5a02f1ed43debed1fd24f7aad72490064f795b9420f15d847bac822aa4621a1cR476-R477). r? ``@nikomatsakis`` ---- This limitation is especially annoying with async code, since it forces one to write this: ```rust trait AsyncFn3<A, B, C>: Fn(A, B, C) -> <Self as AsyncFn3<A, B, C>>::Future { type Future: Future<Output = Self::Out>; type Out; } impl<A, B, C, Fut, F> AsyncFn3<A, B, C> for F where F: Fn(A, B, C) -> Fut, Fut: Future, { type Future = Fut; type Out = Fut::Output; } fn async_closure() -> impl AsyncFn3<i32, i32, i32, Out = u32> { |a, b, c| async move { (a + b + c) as u32 } } ``` Instead of: ```rust fn async_closure() -> impl Fn(i32, i32, i32) -> impl Future<Output = u32> { |a, b, c| async move { (a + b + c) as u32 } } ```
…errors Allow `impl Fn() -> impl Trait` in return position _This was originally proposed as part of rust-lang#93082 which was [closed](rust-lang#93082 (comment)) due to allowing `impl Fn() -> impl Trait` in argument position._ This allows writing the following function signatures: ```rust fn f0() -> impl Fn() -> impl Trait; fn f3() -> &'static dyn Fn() -> impl Trait; ``` These signatures were already allowed for common traits and associated types, there is no reason why `Fn*` traits should be special in this regard. `impl Trait` in both `f0` and `f3` means "new existential type", just like with `-> impl Iterator<Item = impl Trait>` and such. Arrow in `impl Fn() ->` is right-associative and binds from right to left, it's tested by [this test](https://github.com/WaffleLapkin/rust/blob/a819fecb8dea438fc70488ddec30a61e52942672/src/test/ui/impl-trait/impl_fn_associativity.rs). There even is a test that `f0` compiles: https://github.com/rust-lang/rust/blob/2f004d2d401682e553af3984ebd9a3976885e752/src/test/ui/impl-trait/nested_impl_trait.rs#L25-L28 But it was changed in [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-ccecca938872d65ffe8cd1c3ef1956e309fac83bcda547d8b16b89257e53a437R37) to test the opposite, probably unintentionally given [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-5a02f1ed43debed1fd24f7aad72490064f795b9420f15d847bac822aa4621a1cR476-R477). r? `@nikomatsakis` ---- This limitation is especially annoying with async code, since it forces one to write this: ```rust trait AsyncFn3<A, B, C>: Fn(A, B, C) -> <Self as AsyncFn3<A, B, C>>::Future { type Future: Future<Output = Self::Out>; type Out; } impl<A, B, C, Fut, F> AsyncFn3<A, B, C> for F where F: Fn(A, B, C) -> Fut, Fut: Future, { type Future = Fut; type Out = Fut::Output; } fn async_closure() -> impl AsyncFn3<i32, i32, i32, Out = u32> { |a, b, c| async move { (a + b + c) as u32 } } ``` Instead of: ```rust fn async_closure() -> impl Fn(i32, i32, i32) -> impl Future<Output = u32> { |a, b, c| async move { (a + b + c) as u32 } } ```
Allow `impl Fn() -> impl Trait` in return position _This was originally proposed as part of #93082 which was [closed](rust-lang/rust#93082 (comment)) due to allowing `impl Fn() -> impl Trait` in argument position._ This allows writing the following function signatures: ```rust fn f0() -> impl Fn() -> impl Trait; fn f3() -> &'static dyn Fn() -> impl Trait; ``` These signatures were already allowed for common traits and associated types, there is no reason why `Fn*` traits should be special in this regard. `impl Trait` in both `f0` and `f3` means "new existential type", just like with `-> impl Iterator<Item = impl Trait>` and such. Arrow in `impl Fn() ->` is right-associative and binds from right to left, it's tested by [this test](https://github.com/WaffleLapkin/rust/blob/a819fecb8dea438fc70488ddec30a61e52942672/src/test/ui/impl-trait/impl_fn_associativity.rs). There even is a test that `f0` compiles: https://github.com/rust-lang/rust/blob/2f004d2d401682e553af3984ebd9a3976885e752/src/test/ui/impl-trait/nested_impl_trait.rs#L25-L28 But it was changed in [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-ccecca938872d65ffe8cd1c3ef1956e309fac83bcda547d8b16b89257e53a437R37) to test the opposite, probably unintentionally given [PR 48084 (lines)](https://github.com/rust-lang/rust/pull/48084/files#diff-5a02f1ed43debed1fd24f7aad72490064f795b9420f15d847bac822aa4621a1cR476-R477). r? `@nikomatsakis` ---- This limitation is especially annoying with async code, since it forces one to write this: ```rust trait AsyncFn3<A, B, C>: Fn(A, B, C) -> <Self as AsyncFn3<A, B, C>>::Future { type Future: Future<Output = Self::Out>; type Out; } impl<A, B, C, Fut, F> AsyncFn3<A, B, C> for F where F: Fn(A, B, C) -> Fut, Fut: Future, { type Future = Fut; type Out = Fut::Output; } fn async_closure() -> impl AsyncFn3<i32, i32, i32, Out = u32> { |a, b, c| async move { (a + b + c) as u32 } } ``` Instead of: ```rust fn async_closure() -> impl Fn(i32, i32, i32) -> impl Future<Output = u32> { |a, b, c| async move { (a + b + c) as u32 } } ```
Is there a tracking issue for this in an argument position? I specifically want to accept something like fn foo(
f: impl for<'a> FnOnce(&'a Args) -> impl 'a + std::future::Future<Res>,
) |
@kevincox as far as I know, there isn't, and it would be hard to allow what you want, since |
@kevincox the following works on Nightly with #![feature(unboxed_closures)]
use std::future::Future;
struct Args;
struct Res;
fn foo<F>(f: F)
where
F: for<'a> FnOnce<(&'a Args,)>,
for<'a> <F as FnOnce<(&'a Args,)>>::Output: Future<Output = Res>,
{
todo!()
}
fn main() {
foo(|_: &Args| async { Res })
} |
Filed rust-lang/impl-trait-initiative#15 to track |
This allows writing the following function signatures:
All of the above are already allowed with common traits and associated
types, there is no reason why
Fn*
traits should be special in thisregard.
There even is a test that
f0
compiles:rust/src/test/ui/impl-trait/nested_impl_trait.rs
Lines 25 to 28 in 2f004d2
But it was changed in PR 48084 (lines) to test the opposite, probably unintentionally given PR 48084 (lines).
r? @oli-obk
This limitation is especially annoying with async code, since it forces one to write this:
Instead of: