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

Generic closures #1650

Closed
wants to merge 5 commits into from
Closed

Generic closures #1650

wants to merge 5 commits into from

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Jun 15, 2016

This RFC adds the ability to define generic closures:

<T: Debug>|x: T| println!("{:?}", x);

Rendered

@durka
Copy link
Contributor

durka commented Jun 15, 2016

Can you make the closure generic over lifetimes as well?

Are there any potential ambiguities with this syntax? Should we have the generics introduced with the for keyword?

The generated closure type will have generic implementations of `Fn`, `FnMut` and `FnOnce` with the provided type bounds. This is similar to the way closures currently have generic implementations over lifetimes.

# Drawbacks
[drawbacks]: #drawbacks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am suspicious of any proposal that has zero drawbacks, alternatives, and unanswered questions.

@Amanieu
Copy link
Member Author

Amanieu commented Jun 15, 2016

Can you make the closure generic over lifetimes as well?

Of course. Note that this is already possible:

|x: &(i32, i32)| -> &i32 { &x.0 }

This closure is generic over all possible lifetimes for the input reference.

Are there any potential ambiguities with this syntax? Should we have the generics introduced with the for keyword?

There should be no ambiguities. It is already possible to invoke a function using the <T>::method() syntax, so a leading < character should not be a problem.

@kennytm
Copy link
Member

kennytm commented Jun 16, 2016

Where do we put the move?

move <T: Debug> |x: T| ...
<T: Debug> move |x: T| ...

@F001
Copy link
Contributor

F001 commented Jun 16, 2016

I doubt the feasibility here is that generic functions can't be values.
We can't declare a variable with the type Vec<T>, unless we specify a concrete type for the type parameter T.
So, how can we store this <T: Debug>|x: T| println!("{:?}", x); in a variable ? Closure is just a syntax sugar for anonymous types. Can you elaborate the underline implementation without syntax sugar?

@Amanieu
Copy link
Member Author

Amanieu commented Jun 16, 2016

@F001 The closure only has a single type, it's just that it implements the Fn traits generically. See the example code linked in the RFC for a working example without syntax sugar.

@durka
Copy link
Contributor

durka commented Jun 16, 2016

Would this let you do:

let foo = <T: Debug>|x: T| println!("{:?}", x);
foo(42);
foo("bar");

(I'm guessing no.)

@Amanieu
Copy link
Member Author

Amanieu commented Jun 16, 2016

@durka Yes it would.

@F001
Copy link
Contributor

F001 commented Jun 16, 2016

I see. The generic type parameter can only used in the arguments, not the captured values.

@Amanieu
Copy link
Member Author

Amanieu commented Jun 16, 2016

@kennytm The move would go in the front (your first example). I've amended the RFC to specify that. The only thing I'm not sure about is how this interacts with &move, but I don't think there should be a problem.

@durka
Copy link
Contributor

durka commented Jun 16, 2016

That's really neat. I'm fully in favor of this. 👍

@glaebhoerl
Copy link
Contributor

glaebhoerl commented Jun 16, 2016

Functionality-wise I agree that this would be a good thing to have.

The proposed syntax strikes me as arcane and haphazard, although I'm not sure whether it's possible to do any better. Two things in particular bother me about it:

  • Heretofore generics parameter lists have only ever appeared "attached" to some keyword or identifier (impl<T>, fn foo<T>, struct Foo<T>, for<'a>). I think this is a sensible policy. Here, it would be "suddenly appearing out of nowhere". I really don't like this: I think it's bad for readability. Reading the code left-to-right, you have to read quite a bit ahead and backtrack several times before you can realize "oh, so this thing here is a closure; and all of that punctuation back there was a type parameter list for it".

    One alternative would be to introduce it with for, as with HRTBs at the type level (as mentioned by @durka): so for<T: Debug> |x: T| println!("{:?}", x). The potential drawback here is that, thus far, although we've already been using for to mean different things, it's meant one thing at the value level inside function bodies (for..in loops), and another thing at the type level outside function bodies (for<'a> HRTBs), reducing the potential for confusion between them. Now you'd have both of them appearing in the same context. Although not all is lost: presumably for..in would only ever appear as a statement, while for<T> |x: T| would only ever appear as a subexpression (e.g. as a HOF argument), so you would still only ever expect to see one of them at any given point.

    In any case, I think I do prefer this alternative.

  • Similarly, up to now where clauses have only ever appeared at the item level, and this would be a completely new use for them. So far they don't even appear in the syntax for types! Not to even mention terms. That's something @nikomatsakis has mentioned he's concerned about when discussing the possibility of extending HRTBs to work with type parameters as well (Higher-ranked types in trait bounds #1481): you'd have F: for<T: Foo> Fn(T) -> T.... and then F: for<T> where T: Foo Fn(T) -> T, or F: for<T> Fn(T) -> T where T: Foo? The two questions seem to be closely related: if the syntax we use here is for<T: Debug> |x: T| ..., then the question of "where does the where clause go" is the same in both cases.

(The syntax for specifying closure types |u: u8| -> u8 { u*8 } is already a strange amalgam of type- and value-level syntax, so these would not be without precedent, but they would exacerbate the issue.)

It's interesting that if we wanted to be able to write a type annotation for a generic closure, we'd need #518 (just like for non-generic closures), and #1481 as well (which was mentioned above). If we did have both of those, another way to write a generic closure would be let f: impl for<T: Debug> Fn(T) = |x| println!("{:?}", x) or (|x| println!("{:?}", x)): impl for<T: Debug> Fn(T) using type ascription.

Takeaway: this seems closely related #1481 in terms of both syntax and functionality, so maybe there should be some cross-pollination going on between them.

@durka
Copy link
Contributor

durka commented Jun 16, 2016

@glaebhoerl does it conflict with potential HKT syntax? (or is this just HKT-for-closures as it is?)

@glaebhoerl
Copy link
Contributor

HKT is totally separate from this I think. It's "higher-ranked trait bounds quantified over types and not just lifetimes" (iow, "for<T> not just for<'a>") which is related in different ways to both this and HKT.

@durka
Copy link
Contributor

durka commented Jun 16, 2016

Right. So... if we end up using for<T, U> for HKT and then we also use it for these closures, it might be confusing. But I also agree with you that with no keyword it seems to require a lot of lookahead (possibly more than the parser currently supports).

@comex
Copy link

comex commented Jun 16, 2016

I don't think it would be confusing, because the meaning matches up. The expression for<T> |t: T| -> T { ... } would satisfy the higher-ranked trait bound for<T> Fn(T) -> T.

@glaebhoerl
Copy link
Contributor

if we end up using for<T, U> for HKT and then we also use it for these closures, it might be confusing.

We wouldn't. for<T, U> ... isn't higher-kinded types, it's higher-ranked types (or bounds/constraints in this case). Think of the difference as higher-ranked types being "higher-order generic functions", and higher-kinded types being "higher-order generic types". So higher-ranked types means being able to have (generic) functions which take other generic functions (like these here generic lambdas) as arguments, while higher-kinded types means being able to have generic types which take other generic types as type arguments. The latter would look like Foo<Option> or Bar<Vec>, not for<T>.

@glaebhoerl glaebhoerl mentioned this pull request Jun 17, 2016
@tikue
Copy link
Contributor

tikue commented Jun 21, 2016

Can the text of this RFC please mention some alternative syntaxes that don't have the lookahead problem? I feel the same as @sfackler that the lack of drawbacks/alternatives is not duly diligent.

@aturon aturon added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jun 21, 2016
@solson
Copy link
Member

solson commented Jun 22, 2016

If we did have both of those, another way to write a generic closure would be let f: impl for<T: Debug> Fn(T) = |x| println!("{:?}", x) or (|x| println!("{:?}", x)): impl for<T: Debug> Fn(T) using type ascription.

It's interesting to note what it could look like with the any/some proposal from #1522 (comment):

let f: some Fn(any Debug) = |x| println!("{:?}", x);

Or in a call:

foo(|x: any Debug| println!("{:?}", x));

This is potentially a stronger argument for the any/some syntax than anything in that thread, since generic closures with explicit type parameter lists are quite ugly. @glaebhoerl's description of "arcane and haphazard" seems apt, and I agree that it's hard to do better than @Amanieu's syntax - unless you anonymize the type parameters with any.

@durka
Copy link
Contributor

durka commented Jun 22, 2016

Noooooo don't pull this RFC thread into that neverending one.

But anyway, that syntax doesn't seem fully general since you can't refer to
the generic type, i.e. you can't call .collect::<Vec<T>>(). So there
would still have to be a way to expand it to something like that proposed
here, which seems fine to me if a keyword is added in front.

On Tue, Jun 21, 2016 at 9:54 PM, Scott Olson notifications@github.com
wrote:

If we did have both of those, another way to write a generic closure would
be let f: impl for Fn(T) = |x| println!("{:?}", x) or (|x| println!("{:?}",
x)): impl for Fn(T) using type ascription.

It's interesting to note what it could look like with the any/some
proposal from #1522 (comment)
#1522 (comment):

let f: some Fn(any Debug) = |x| println!("{:?}", x);

Or in a call:

foo(|x: any Debug| println!("{:?}", x));

This is potentially a stronger argument for the any/some syntax than
anything in that thread, since generic closures with explicit argument
lists are quite ugly. @glaebhoerl https://github.com/glaebhoerl's
description of "arcane and haphazard" seems apt, and I agree that it's hard
to do better than @Amanieu https://github.com/Amanieu's syntax - unless
you anonymize the type parameters with any.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1650 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAC3n-Dvc5lP_ugmtzdXV4n2sHUK5fkIks5qOJXggaJpZM4I2Wcq
.

@solson
Copy link
Member

solson commented Jun 22, 2016

@durka Sorry. 😛 any/some needs its own discussion area - but I thought it was interesting that it could help solve an ergonomics problem in a separate area.

any/some isn't fully general in any of the places its been proposed to be used, so I'm not suggesting it as an alternative, just a more ergonomic shorthand that hopefully applies in the majority of cases. (Similarly, the minimalistic form of impl Trait is not fully general and also requires an explicit form.)

This, of course, is something that could be added after this RFC is accepted, but it might make some people feel less worried about the syntax proposed by this RFC if a future shorthand is possible.

@Ericson2314
Copy link
Contributor

for<..> |..| syntax may conflict with #1657 as we'd be representing Λ and ∀ the same way.

@canndrew
Copy link
Contributor

+1 for this RFC. I'm in a situation today where I have a closure which is generic over a lifetime and I need to refer to that lifetime explicitly, but there's no syntax to do that.

In other words I have this: |foo: &Foo| { ... }
But I need: for<'a> |foo: &'a Foo| { /* So that I can write 'a here */ }

Just adding this would be a good first step and would be simpler than adding closures which are generic over types aswell.

@eddyb
Copy link
Member

eddyb commented Jun 23, 2016

@canndrew In fact, we support that today, if the closure is an argument to a generic function which has a Fn{,Mut,Once} bound on the type of the closure.
In fact, I tell people to never put closures anywhere other than function arguments, if they can, for this reason.

@Ericson2314
Copy link
Contributor

If the for<..> probably I speculate on exists. |<..> ..| .. syntax should work, but is pretty ugly.


## Implementation

The generated closure type will have generic implementations of `Fn`, `FnMut` and `FnOnce` with the provided type bounds. This is similar to the way closures currently have generic implementations over lifetimes.
Copy link
Member

@pnkfelix pnkfelix Jun 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way that closures are generic over lifetimes corresponds to types of the form for <'a> Fn(&'a u8) (for example), where the client code can instantiate that type at different lifetimes for 'a.

Is that analogy what you intend to apply here, keeping in mind that the above lifetimes are erased at compile time?

I ask because when I first saw this proposal, I had assumed that part of the intention was to enable one to write rank-N higher-order functions (think forall in Haskell), where a function passed as an argument is able to be instantiated at multiple concrete types.

A concrete example of what I mean:

fn hof_example<F>(f: F) -> i32
    where F: for <T: fmt::Debug> Fn(T) -> i32
{
    f(0.0) + f(true)
}

Note: the above is certainly doable under a monomorphization strategy.

  • It becomes ... harder when one tries to generalize it to object-types (f: Box<for <T: fmt::Debug> Fn(T)>).

I'm just trying to understand the scope of what you're proposing.


In other words:

Can you speak more on the actual type assigned to these generic closures, or at least about the generic implementations here?

I.e. what type are you assigning to the expression

<T: Debug> |x: T| { println!("{:?}, x); }

and what impls are generated for it?

I'm trying to figure out if this ends up being something like:

impl<T: Debug> Fn(T) for [closure@pnk-example] { ... }

or if there is something broader being implicitly proposed here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The closure is as generic as the function that created it, the impl can be more generic (with the usual requirement that the additional parameters can be deduced from the trait parameter types).

Copy link
Member

@pnkfelix pnkfelix Jun 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(after reading the comment thread a bit, it seems like rank-N types are not what is being proposed here. I still think the RFC text must be clarified to make that more clear.)

@reddraggone9
Copy link

WIth this proposal, would this be possible?

@eddyb
Copy link
Member

eddyb commented Jul 21, 2016

@reddraggone9 AFAICT, yes it would.

@strega-nil
Copy link

ping @Amanieu @pnkfelix

status?

@withoutboats
Copy link
Contributor

I've been thinking about this RFC. Here are a few thoughts:

  1. It feels wrong to implement this without HRTB of type parameters. <T>|x| x is a type which implements for<T> Fn(T) -> T. Essentially I don't like that the genericity of these closures can't be preserved across function boundaries.
  2. The fact that this parametricity can't be inferred (because how we would know what the bound is?) is a downside to this RFC to me. Currently, closures are almost never type ascribed; this would be a stark contrast to that.

Despite the second point, I think overall we should do this, once/if we have HRTB of type arguments. It seems like a logical extension with that feature in place.

@canndrew
Copy link
Contributor

Despite the second point, I think overall we should do this, once/if we have HRTB of type arguments. It seems like a logical extension with that feature in place.

We could start by just doing it for lifetimes.

@withoutboats
Copy link
Contributor

@canndrew we do allow the construction of such closures, but we don't have syntax for ascribing the lifetime parameter.

https://is.gd/B8EXO9

    let closure = |x: &u32| *x;
    {
        let x = 0;
        closure(&x);
    }
    {
        let y = 0;
        closure(&y);
    }

@canndrew
Copy link
Contributor

but we don't have syntax for ascribing the lifetime parameter.

That's what I mean though. I've needed that syntax before.

@aturon
Copy link
Member

aturon commented Jan 31, 2017

I suggest we postpone this RFC until we're a good bit further along with the impending trait system revamp, which I think should make this kind of thing much easier to implement and reason about.

@rfcbot fcp postpone

@rfcbot
Copy link
Collaborator

rfcbot commented Jan 31, 2017

Team member @aturon has proposed to postpone 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.

@rfcbot
Copy link
Collaborator

rfcbot commented Feb 23, 2017

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Feb 23, 2017
@rfcbot
Copy link
Collaborator

rfcbot commented Mar 5, 2017

The final comment period is now complete.

@aturon
Copy link
Member

aturon commented Mar 6, 2017

The FCP has elapsed without any further comments. I'm going to go ahead and close as postponed, to be revisited when the new trait system implementation is in place. Thanks @Amanieu for the RFC!

@aturon aturon closed this Mar 6, 2017
@burdges
Copy link

burdges commented Jun 10, 2017

Just ran into a situation that'd benefit form these.

@varkor
Copy link
Member

varkor commented Mar 22, 2018

This should probably be labelled as "postponed" for reference.

@kennytm
Copy link
Member

kennytm commented Mar 22, 2018

Oh, the suggestion in #1650 (comment) is actually now viable since universal_impl_trait has been implemented (can't compile yet)

#![feature(universal_impl_trait)]

use std::fmt::Debug;

// fn f(x: impl Debug) {
//     println!("{:?}", x);
// }

fn main() {
    let f = |x: impl Debug| println!("{:?}", x);                                                     //"
    // (currently an error)
    //~^^ ERROR [E0562]: `impl Trait` not allowed outside of function and inherent method return types
    f(1);
    f(true);
    f((4, 5, 6));
}

@Centril Centril added the postponed RFCs that have been postponed and may be revisited at a later time. label Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. postponed RFCs that have been postponed and may be revisited at a later time. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet