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

Unboxed closures #114

Merged
merged 2 commits into from
Jul 30, 2014
Merged

Unboxed closures #114

merged 2 commits into from
Jul 30, 2014

Conversation

nikomatsakis
Copy link
Contributor

A relatively complete description of by reference closures that aims to encompass entirety of current thinking, except for bound lifetimes.

cc @pcwalton

}

trait FnOnce<A,R> {
fn call(&self, args: A) -> R;
Copy link
Member

Choose a reason for hiding this comment

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

Since this is the FnOnce trait, its method should be fn call(self, args: A) -> R, right? (That is, self, not &self.)

Copy link
Member

Choose a reason for hiding this comment

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

If it is self, which I believe it should be, it will be impossible to use FnOnce boxed, because trait objects cannot use self-by-value.

Choose a reason for hiding this comment

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

There's no inherent reason we can't make by-value self work with Box<Trait>. It just needs to generate a wrapper function to use in the vtable. An extra direct call after the indirect call would have a bit of overhead, but since a dynamic memory allocation is always paired with each call for non-zero size self, it doesn't seem important.

@huonw
Copy link
Member

huonw commented Jun 11, 2014

Could this have a more descriptive title?

Therefore, an expression like `a(...)` will be desugar into an
invocation of one of the following traits:

trait Fn<A,R> {
Copy link
Member

Choose a reason for hiding this comment

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

There was a good suggestion for how these traits can inherit: #97 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure that we want a trait inheritance relationship. That would imply (e.g.) that one could implement those other traits differently, so that your function behaved in different ways. That might be useful in some cases but is also sort of surprising. Another option would be to include impls like:

impl<T:FnShare> FnOnce for T { ... }

and so forth. Unfortunately since coherence doesn't know that FnShare, Fn, and FnOnce are mutually exclusive that might run into some problems of its own.

Copy link

Choose a reason for hiding this comment

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

If we don't have a subtyping relationship on the closure traits, then we'll just end up with a needless variants of higher-order functions.

For example, if I want to write foo(f: ||) { f(); f(); } and I don't really care whether f mutates its environment or not, then I'd like to be able to pass both Fn<(),()> and FnShare<(),()> to foo. But I can't do that because there's no subtyping! Instead I'll need to provide foo_imm(f: |&:|) { f(); f(); } and foo_mut(f: |&mut:|) { f(); f(); }. If I only called f once in the body of foo, then I'd need to add a foo_once version as well to be as general as possible. If I have multiple closures involved, then I'll need to provide variants for every permutation of the relevant types. This gets out of hand very quickly.

Not having subtyping will either:

  • Create this entirely pointless explosion of variants
  • Encourage people to put an artificially restrictive Fn bound on an otherwise FnShareable closure because no one is bothering to write the FnShare variants. FnOnce would probably still be used due to its utility so we'd have the foo / foo_once explosion still.
  • No one bothers to make variants at all and we have to explain forevermore why you can't pass your closure to this function when it's perfectly safe to do so.

Yeah it's weird if people implement the traits with different bodies, but it's not any different than when people don't follow the rules for other kind of operator overloading. Slightly different bodies are actually necessary for a FnOnce instance versus Fn/FnShare because of the values vs refs.

Copy link
Member

Choose a reason for hiding this comment

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

There is a serious issue with using trait inheritance, which is the vtable size. It must be kept to 1 method, otherwise performance will suffer (though, Fn and FnShare would use the same function for their methods, so something may be done with that in mind).

I wonder if what @anasazi is pointing out still holds if closure bodies were to implement either FnOnce, FnOnce + Fn or FnOnce + Fn + FnShare (instead of just one trait).

Encourage people to put an artificially restrictive Fn bound on an otherwise FnShareable closure ...

I believe not, if the above is the only remaining issue, because Fn wouldn't be restrictive, as all FnShare closures would also implement it.

Choose a reason for hiding this comment

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

It will always be pointer -> [destructor_pointer, method_pointer] anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately since coherence doesn't know that FnShare, Fn, and FnOnce are mutually exclusive that might run into some problems of its own.

Does #48 cover this at all?

(As much as it pains me to say, would it be at all reasonable to make Fn/FnShare/... special in this regard?)

Copy link

Choose a reason for hiding this comment

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

@eddyb I don't understand what you mean here. How is one opaque and the other not?

Copy link
Member

Choose a reason for hiding this comment

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

I did say that byref/byvalue is not the best wording for this.
The vtable optimization for one method is specific to &Trait and &mut Trait, it's not an universal optimization by any means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Thu, Jun 12, 2014 at 02:03:31AM -0700, Huon Wilson wrote:

Does #48 cover [exclusive traits] at all?

Not in and of itself, though it does lay the groundwork. I think the
way to express exclusive traits would be to add a negative trait
bound, so that you could do something like

trait Even { }
trait Odd : !Even { }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Wed, Jun 11, 2014 at 08:57:28PM -0700, Eric Reed wrote:

If we don't have a subtyping relationship on the closure traits,
then we'll just end up with a needless variants of higher-order
functions.

I agree this situation is to be avoided. Subtrait relationships are
not necessarily the only way to do that, however (though they may be a
good way).

My biggest concern is ergonomic and points at a general weak point in
our trait system as compared to a traditional OO setup. Basically,
although we use OO terminology like "supertrait", we don't in fact
offer an OO system where subtraits can override supertrait methods and
so on. This basically means that people who manually implement the
close traits will have to implement more than one trait in many
(most?) cases.

In other words, if we had a closure hierarchy like:

trait FnOnce<A,R> {
    fn call_once(self, arg: A) -> R;
}

trait Fn<A,R> : FnOnce<A, R> {
    fn call(&mut self, arg: A) -> R;
}

trait FnShare<A,R> : Fn<A, R> {
    fn call_share(&self, arg: A) -> R;
}

And now I have some type that implements FnShare:

struct Foo;

impl FnShare<(),()> for Foo {
    fn call_share(&self, arg: ()) -> () { ... }
}

This will yield an error because there is no impl FnOnce
nor Fn. I'd have to write those manually:

impl Fn<(),()> for Foo {
    fn call(&mut self, arg: ()) -> () {
        self.call_share(arg)
    }
}

impl FnOnce<(),()> for Foo {
    fn call_once(&mut self, arg: ()) -> () {
        self.call_share(arg)
    }
}

Maybe this doesn't matter, since it will be uncommon to implement
these traits manually, and perhaps we will at some point grow the
ability for a "subtrait" to specify default impls for
supertraits. This would take some careful design to get right though.

If we did instead have an impl like:

impl<A,R,T:FnShare<A,R>> Fn<A,R> for T { ... }

then there'd be no such issue. Still, that'd require some other
extensions to permit.

@nikomatsakis nikomatsakis changed the title Initial commit Unboxed closures Jun 11, 2014

# Summary

- Convert function call `a(b, ..., z)` into an overloadable operator
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this have any impact on bare functions? Would they effectively implement FnShare?

@nikomatsakis
Copy link
Contributor Author

It was pointed out that to support Box<FnOnce()> we need rust-lang/rust#10672

@bharrisau
Copy link

Does the explicit 'ref' keyword indicate that by-ref is more expensive or in some way less preferred than by-value? Or are they much the same and 'ref' was just used because it already exists as a keyword?

@thestinger
Copy link

By-value is preferred because it doesn't tie the closure to a stack frame, and most values are small so it's usually less expensive to capture by-value. In many cases, there are already references and it doesn't make sense to use two layers of indirection.

This change gives user control over virtual vs static dispatch. This
works in the same way as generic types today:

fn foo(x: &mut Fn<int,int>) -> int {
Copy link
Member

Choose a reason for hiding this comment

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

This (and the next couple of example functions) should be Fn<(int,), int> rather than Fn<int, int>, I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Wed, Jun 11, 2014 at 03:37:11PM -0700, Chris Morgan wrote:

This (and the next couple of example functions) should be Fn<(int,), int> rather than Fn<int, int>, I think?

Sadly yes. Thanks.

@chris-morgan
Copy link
Member

I presume that this would not affect the behaviour of method lookup with .? Sometimes it’d be rather nice if I could easily have self.field(…) for a field that implements Fn, rather than (self.field)(…).

@chris-morgan
Copy link
Member

How about overloading? Would something like this, reminiscent of Qt’s overloaded getters and setters, be possible?

struct Foo {
    value: Bar,
}

impl<'a> FnShare<(), &'a Bar> for Foo {
    fn call(&'a self, _: ()) -> &'a Bar {  // uh oh, I guess 'a is problematic
        self.value
    }
}

impl Fn<(Bar,), ()> for Foo {
    fn call(&mut self, (value,): (Bar,)) {
        self.value = value

Ignoring the problem of the lifetime for a moment—would foo() and foo(Bar) both be expected to work? (We can declare both implementations to be of Fn or of FnShare if we like. It’s just the concept, whether duality of call signatures would be thus permitted).

Returning to the self lifetime, I can imagine being able to express that being a corner-case desirable.

from the environment by value. As usual, this is either a copy or
move depending on whether the type of the upvar implements `Copy`.
- Specifying receiver mode (orthogonal to capture mode above):
- `|a, b, c| expr` is equivalent to `|&mut: a, b, c| expr`
Copy link

Choose a reason for hiding this comment

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

I don't like this default. It's neither the most relaxed bound for what you can do in bodies (that's FnOnce), nor is it the most relaxed bound for when you can call the closure (that's FnShare).

Copy link
Member

Choose a reason for hiding this comment

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

Me neither. If we don't use something flexible, it will bite us in the back. And we already have a solution. One that we can actually implement relatively easily, too, not something impossible.

@bstrie
Copy link
Contributor

bstrie commented Jun 12, 2014

I would like to see some discussion from the devs regarding @eddyb's proposal in the prior RFC (#97 (comment)). AFAICT it has only been ignored thus far.

@nikomatsakis
Copy link
Contributor Author

On Thu, Jun 12, 2014 at 03:07:38PM -0700, Ben Striegel wrote:

I would like to see some discussion from the devs regarding @eddyb's
proposal in the prior RFC
(#97 (comment)). AFAICT
it has only been ignored thus far.

@bstrie thanks for bringing that up. I had in fact missed it amongst
the other comments. My thoughts are as follows:

  1. In order to achieve inference and obviate the need for explicit
    declaration of self type, I had in fact intended to wait until
    vtable resolution, as @eddyb suggests.
  2. Having the compiler implement multiple traits for a closure is an
    interesting thought. The disadvantage compared to supertraits or
    impl-based-adapters is that if I write generic code like fn foo<T:Fn<...>>, I can't reuse T in a context where FnMut is
    required without having an extra bound (T:Fn+FnMut) or making
    explicit use of an adapter. (It's also more complicated in the
    compiler itself.)
  3. The advantage of course is that it retains the property of having
    exactly one method which may permit some optimization. I'd like to
    see some microbenchmarks on the importance of this.

@nikomatsakis
Copy link
Contributor Author

On Wed, Jun 11, 2014 at 03:08:15PM -0700, Ben Harris wrote:

Does the explicit 'ref' keyword indicate that by-ref is more
expensive or in some way less preferred than by-value? Or are they
much the same and 'ref' was just used because it already exists as a
keyword?

I personally do not believe ref intends a value judgement either
way. In fact, creating a ref closure is the same cost (today) as a
by-value closure, and could be optimized to be cheaper (by storing a
pointer to the stack frame itself, rather than individual variables).

Rather, we chose to use the ref keyword because it was lightweight
and consistent with match statements and other parts of the language:
basically, all operations that create a reference into the stack now
employ either & or ref keyword.

@nikomatsakis
Copy link
Contributor Author

On Wed, Jun 11, 2014 at 03:39:57PM -0700, Chris Morgan wrote:

I presume that this would not affect the behaviour of method lookup
with .? Sometimes it’d be rather nice if I could easily have
self.field(…) for a field that implements Fn, rather than
(self.field)(…).

Correct, behavior of method lookup does not change.

@nikomatsakis
Copy link
Contributor Author

I added a commit describing @eddyb and @anasazi proposals for subtrait relationships. Let me know if summary is inaccurate.

@anasazi
Copy link

anasazi commented Jun 14, 2014

@nikomatsakis Looks accurate to me. Thanks for putting it in.


opt_int.map({
let in_vec = &in_vec;
let out_vec = &mut in_vec;
Copy link
Contributor

Choose a reason for hiding this comment

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

typo (out_vec)

## Closure sugar in trait references

The current type for closures, `|T1, T2| -> R`, will be repurposed as
syntactic sugar for a reference to the appropriate `Fn` trait. This
Copy link
Contributor

Choose a reason for hiding this comment

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

You are using the word "reference" here in its English, as opposed to Rustic, sense?

@glaebhoerl
Copy link
Contributor

One other question that I didn't see addressed in the text: if a function expects a boxed closure, would you have to explicitly take a reference to the (unboxed) closure at the call site? E.g. foo.do_for_each(&mut ref |x| { ... })? Or would there be some kind of auto-ref to avoid the &mut here?

@huonw
Copy link
Member

huonw commented Jun 16, 2014

I wonder if it's worth having something similar to C++'s mutable, e.g. mut |...| ....; and then mutation of by-value captures would only be valid when mut is specified.

@nikomatsakis
Copy link
Contributor Author

On Sun, Jun 15, 2014 at 03:04:42PM -0700, Gábor Lehel wrote:

One other question that I didn't see addressed in the text: if a
function expects a boxed closure, would you have to explicitly take
a reference to the (unboxed) closure at the call site?
E.g. foo.for_each(&mut ref |x| { ... })? Or would there be some
kind of auto-ref to avoid the &mut here?

At present there is no sugar for this. It's possible this would fit
into a more general auto-ref proposal.

FnOnce::call_once(a, (b, c, d))

To integrate with this, closure expressions are then translated into a
fresh struct that implements one of those three traits. The precise
Copy link
Member

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 good to include an explicit example of this translation/desugaring as part of the RFC presentation, just so a reader can see concretely how the closure expression turns into a struct construction.

In particular, someone might be wondering if the translation of foo({ let x: i32 = 3; |&: y:i32| -> i32 x + y }) is

struct Fresh { x: i32 }
impl FnShare<(i32,), i32> for Fresh { fn call_share(&self, y: i32) -> i32 { x + y }
foo(&Fresh { x: 3 } as &FnShare)

or if it is

struct Fresh { x: i32 }
impl FnShare<(i32,), i32> for Fresh { fn call_share(&self, y: i32) -> i32 { x + y }
foo(Fresh { x: 3 })

(I have deliberately left out the signature of foo, since my understanding is that the desugaring should not be affected by the type of foo.)

@brson
Copy link
Contributor

brson commented Jul 30, 2014

@brson
Copy link
Contributor

brson commented Jul 30, 2014

Unboxed closures are a complex topic that will probably evolve during the implementation. We're probably going to update the RFC as we go, and will probably also feature gate some of the corner cases that may not be fully-bakable yet.

@shepmaster
Copy link
Member

Accepted as RFC 43

Looks like it is actually RFC 44

withoutboats pushed a commit to withoutboats/rfcs that referenced this pull request Jan 15, 2017
@Centril Centril added A-closures Proposals relating to closures / lambdas. A-typesystem Type system related proposals & ideas A-borrowck Borrow checker related proposals & ideas labels Nov 23, 2018
wycats pushed a commit to wycats/rust-rfcs that referenced this pull request Mar 5, 2019
wycats pushed a commit to wycats/rust-rfcs that referenced this pull request Mar 5, 2019
Add ember-cli-template-lint to default app & addon blueprints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-borrowck Borrow checker related proposals & ideas A-closures Proposals relating to closures / lambdas. A-typesystem Type system related proposals & ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.