Allow closure expressions to expand to a & or &mut temporary

[nikomatsakis]

Modify the || expression sugar so that it can expand to either F, &F, or &mut F, where F is a fresh struct type implementing one of the Fn/FnMut/FnOnce traits.

Rendered view.

[petrochenkov]

I think it's not common enough to deserve a special case.
It reminds me the story with a special coercion [T; N] => &[T], that were removed not so long ago.
Indeed, it made a lot of code using fixed-size arrays slightly shorter (in fact most of arrays are used as references and not as values), but in the end the consistency won. This case looks very similar.

If this RFC is accepted, then nothing will stop people from asking for more special cases. Why, for example, generic integer literals can't be a bit more generic and turn into references to integral types when required?

Ultimately, I'd support a general auto-ref coercion T => &T (it would solve this and many other usability problems), but not attempts to patch the system (which is already not very consistent) on case by case basis.
Even the alternative with passing a DST "by value" involves auto-ref + passing by "move reference", because nothing is really copied.

[sinistersnare]

Is there reading somewhere that explains why creating a trait object is recommended over creating a generic and statically dispatched parameter? Looked at the motivations to find these, do you mind expanding?

For example, closure objects reduce code bloat, they work better with object safety restrictions, and they avoid infinite monomorphic expansion for recursion functions.

[nikomatsakis]

This question was raised by @japaric on rust-lang/rust#21699:

(Assume that impl<F: ?Sized> FnMut for &mut F where F: FnMut is in the stdlib)

If you have a function fn foo<F: FnMut>(_: F) and you call foo(|| { .. }), would the compiler prefer to pass the unboxed closure by value? Because it could pass either F or &mut F (or even &mut FnMut) in this case, right?

The answer here is already defined by the coercion semantics. In this case, you'd be coercing to the type F (actually the type _, since F would be instantiated with a type variable). This would not trigger any kind of "auto-ref" behavior. If the function had been written:

fn foo<F:FnMut>(x: &mut F)

and the caller wrote foo(|| ...), that would trigger || to expand to an autoref, even though no trait objects would ultimately be involved.

[nikomatsakis]

@sinistersnare I wouldn't go so far as to say that one is recommended over the other, but there are definitely specific scenarios where a trait object is preferred:

• reduce code bloat: generics require a distinct copy of the callee per call site, effectively, since each call site is supply a unique closure as argument.
• object safety restrictions: traits that include generic methods (like fn do<F:FnMut>(...)) cannot be used as objects, since virtual dispatch and monomorphization are not compatible.
• infinite monomorphic expansion: this is related to the code bloat above. Because the callee must be expanded per call site, if the callee calls itself, then you have an infinite set of call sites, because each new expanded copy also becomes a new call site. This eventually leads to an overflow in the compiler (which has internal limits designed to prevent such expansion).

[nikomatsakis]

@petrochenkov I guess that's the question. It seems worthwhile to me to add sugar for specific cases. I am rather negative on a generic autoref expansion. I personally prefer to see where moves and borrows occur; I found generic autoref more confusing than helpful when we used to have it (well, we had a more limited form, auto-cross-borrow). However, in this case the closure expression is an rvalue, so this information is not particularly helpful or interesting. (That said, the only kind of autoref that I am strictly opposed to is automatic &mut referencing.)

[petrochenkov]

I personally prefer to see where moves and borrows occur

And at the same time most of them occur invisibly in method calls and operators, including the &mut ones.

However, in this case the closure expression is an rvalue, so this information is not particularly helpful or interesting.

Hmm, auto-ref for all rvalues.. Looks like a nice idea actually.

[tikue]

I agree with @petrochenkov. If it's decided that it's a good idea to add auto-ref for closure rvalues, it'd probably be just as good to have auto-ref for all rvalues.

[nikomatsakis]

On Tue, Feb 03, 2015 at 02:42:31PM -0800, Tim wrote:

I agree with @petrochenkov. If it's decided that it's a good idea to add auto-ref for closure rvalues, it'd probably be just as good to have auto-ref for all rvalues.

I see this point of view, though I consider closure expressions
distinct from other sorts of expressions. In particular, there is
already a ton of inference going on to determine various aspects of
what a closure expression expands to. Adding a little more doesn't
really bother me, and feels quite different from general autoref.

[pcwalton]

I worry about the pedagogical consequences of having a distinction between lvalues and rvalues for the purposes of auto-ref. In general, the lvalue/rvalue distinction is a distinction that compiler writers care about, not one that ordinary programmers care about. (Yes, you have to know the difference on some level, but for most people the distinction is purely intuitive and not something that people have to consciously think about; it exists primarily so that the compiler will reject nonsensical assignments like f(x) = 10.) Adding more special semantic differences between lvalues and rvalues is a recipe for confusion; witness how difficult it is to explain the nuances of rvalue references in C++11. I fear that having auto-ref apply to rvalues is an unfortunate middle ground that would end up more confusing than applying it only to closures or to all values.

[Kimundi]

I actually think its not that difficult to explain the difference between rvalues and lvalues in Rust, and it might even help pedagogically because they explain exactly when a move/copy happens, and when a temporary is created.

• rvalue used as a lvalue => temporary
• lvalue used as a rvalue => move/copy.

[nikomatsakis]

@pcwalton to be clear, though, autoref for all rvalues is not what is described in this RFC, which is rather more limited. As I wrote...somewhere, I don't really consider this a form of general autoref, but rather making the || sugar just a bit sweeter.

[nikomatsakis]

To rephrase, this proposal basically changes the explanation of || from:

"|| expands to an anonymous struct S containing the closure environment"

to:

"|| expands to an anonymous struct S (or reference to an anonymous struct S) containing the closure environment"

[petrochenkov]

expands to an anonymous struct S (or reference to an anonymous struct S)

Hmm, auto-ref for all literals, nice idea too :D
(Okay, no more spamming)

[aturon]

My feeling is that, without a change like this (or the DST-by-value change mentioned in Alternatives), there will be a large ergonomic tax for using boxed closures. I believe that unboxed closures are already overused, which leads to significant code bloat and longer compilation times, and think we should try to adjust this balance.

While I understand the consistency point some have raised, note that this is by no means a general coercion -- it is a relatively small modification to the || sugar, which already does a lot of inference. I believe the distinction between an owned closure value and a reference to one will only matter in contexts where this RFC wouldn't apply. If you're passing a piece of || sugar directly to a function that wants &Fn(A) -> B, you really don't care about having ownership or not.

Put differently, I think the argument shouldn't be about consistency so much as: does this negatively impact your ability to reason about real code? I am unable to come up with any examples where it does. As such, I'm in favor of this RFC as-is.

[rkjnsn]

Since 1.0 has already been released without this (and 1.1 and quite possibly 1.2 as well), I don't think the ergonomic gains are worth the extra complexity and inconsistencies are worth carrying around for ever more. Everywhere else a value is borrowed (even for temporary literals), it has to be explicit. I don't find having to add & and (much less often) &mut to be a significant burden. I think it is definitely worth waiting for DST-by-value, since that is planed, anyway.

[nikomatsakis]

Hear ye, hear ye. This RFC was moved into Final Comment Period as of Wed, June 10th. (Sorry, forgot to add this notice!)

UPDATE: Fixed date :)

[sinistersnare]

Wednesday June 11th

Today is Thursday June 11.

[nikomatsakis]

@rkjnsn

I think it is definitely worth waiting for DST-by-value, since that is planed, anyway.

DST by value would be great to have, but it seems somewhat orthogonal to this RFC. In particular, recursive functions that take closure arguments are still a pain to manage, even if no trait objects are involved at all.

Consider a pattern like this (common enough in the compiler, at least):

fn recursive<F:FnMut>(..., f: F) {
    match ... {
        Something(left, right) => { recursive(left, &mut f); recursive(right, &mut f); }
        ...
    }
}

This code will fail during monomorphization due to infinite expansion. The reason is that it gets called first with the original closure type (let's call it C). Then called with &mut C. Then with &mut &mut C. And so forth. To solve it, you want to move the &mut into the type signature of the fn itself:

fn recursive<F:FnMut>(..., f: &mut F) {
    match ... {
        Something(left, right) => { recursive(left, f); recursive(right, f); }
        ...
    }
}

But now when I call recursive in the first place, I have to do recursive(..., &mut || ...), or else I have to declare a wrapper:

fn recursive<F:FnMut>(..., f: F) {
    recursive1(..., &mut f);
}

fn recursive1<F:FnMut>(..., f: &mut F) {
    // as above, but call `recursive1`, not `recursive`
}

[rkjnsn]

@nikomatsakis

But now when I call recursive in the first place, I have to do recursive(..., &mut || ...), or else I have to declare a wrapper

This doesn't seem like a problem, to me.

Edit: to explain a bit more, the first alternative seems very consistent with the rest rest of the language. Borrowing of function arguments is always explicit. If a function expects a &mut [u32], you have to say &mut [1, 2, 3, 4, 5] to pass it an array literal. I don't see a compelling reason that closures should be any different.

The second alternative is a very common pattern for recursive algorithms: one often needs to perform some initial processing on a function's arguments before calling the actual recursive function, and I, personally, find myself doing this quite often in any language. Rust adds even more reasons to desire a wrapper, such as using Into for extensible overloading. I really don't see why this particular case should be treated specially. You can even make the recursive part a nested function to avoid having recursive1 floating around in the same namespace as recursive.

[alexcrichton]

I feel that @aturon summed up my opinion quite well on this RFC. I've been trying to somewhat aggressively use &mut FnMut() instead of a generic in many functions that take a closure, but it's a constant annoyance for me that the &mut is explicitly required at all callsites.

My opinion on closures is that they're fundamentally designed to be ergonomic. All closures we have today can be replaced with an Env struct and a manual implementation of the Fn* traits, but we would of course never go through and actually write down this desugaring. The point of closures is not have to worry about any of these details and instead give a very clean and ergonomic interface to this primitive.

I would personally feel that the drawbacks to this RFC are far outweighed by putting boxed closures on equal footing with unboxed closures, so I'm in favor.

[rkjnsn]

@alexcrichton

it's a constant annoyance for me that the &mut is explicitly required at all callsites.

What makes this an annoyance? Specifically, what makes it more of an annoyance than everywhere else in the language where explicit borrows are required for literals and variables alike?

In my mind, closures are (and should continue to be) a sugary literal for an anonymous struct created by the compiler. This means that borrowing should be treated exactly the same as for struct literals (requiring &/&mut).

Ideally, I'd like to see passing a closure directly to a function and assigning it to a variable first work exactly the same in all cases. I realize this isn't 100% true, today, but at the very least we shouldn't be moving further from that goal.

It seems like there are two main motivations wanting to take an &mut: reducing code size and recursiveness. The first motivation will be completely addressed when we get by value DSTs, and I already explained in my previous post why I don't think the second motivation is particularly strong. Even if we were never going to get by-value DSTs, I fail to see why &mut |x| x * 2 is any less ergonomic than &mut [1, 2, 3].

Since adding this proposed sugar makes closures different from every other literal in the language, I don't think calling it an "ergonomic speed bump" is sufficient motivation. At the very least the RFC needs to explain why saying &mut on a closure is so much more painful than every other case in which it is required to justify being inconsistent.

I also want to note that until we get by value DSTs, it is possible even today to avoid the code bloat of many instantiations without requiring the calling function to add &mut: use a generic wrapper function that dispatches to the implementation taking a trait object. The wrapper will get inlined away during compilation, resulting in same generated code as the proposed change, here. This would only add a line or two of code in the implementation, and would allow &mut to be avoided by all of the callers.

@nikomatsakis As an aside, is there any reason

let closure = |x| x*2;
do_something(&mut closure)

can't me made to work with inference? It seems analogous to inferring the type of an integer literal or a vector based on its later usage.

[tikue]

Eh, I think there's a clear difference between array literals and closures. Array literal syntax exists because they're a primitive type; without the syntax you can't make an array. Closure syntax only exists to make it easy to use the Fn* traits. I don't think its unreasonable for them to have special handling as described in this RFC.

That being said, I also dont think it's unreasonable to hold off on this RFC for a while. It's a nice-to-have, and it may be that by-value DSTs eliminate a majority of the cases this would be useful. If there's no urgency, why not wait and see where the chips fall.

[alexcrichton]

Specifically, what makes it more of an annoyance than everywhere else in the language where explicit borrows are required for literals and variables alike?

The key difference here is that using a generic vs using a trait object has many compile-time implications, and using trait objects is currently hamstrung ergonomically by requiring &mut where taking a generic does not require this.

Closures are already a special case for "expand into something magical" and as @aturon mentioned earlier I don't think that this is really adding any more magic than is already present.

[rkjnsn]

@alexcrichton

The key difference here is that using a generic vs using a trait object has many compile-time implications, and using trait objects is currently hamstrung ergonomically by requiring &mut where taking a generic does not require this.

Again, how is this different from non-closure traits? Functions still have to decide whether to be generic or take a trait object, and if they choose the latter, anyone passing a variable or literal has to prepend &mut. By value DSTs address this second case, as well, without needlessly special-casing closures.

Closures are already a special case for "expand into something magical"

I very much disagree. One of the things I like about Rust closures is they are conceptually quite understandable and not particularly magical: each closure becomes an instance of an unnameable, compiler-defined struct type that implements one or more traits corresponding to the function call operator. You can even create your own structs that do the same thing (though I realize the exact syntax hasn't been stabilized, yet). All of the magic has to do with creating the struct: what data fields it has, whether they are values or references, which of the function traits it implements, etc. As I recall, most if not all of that is determined by the function body, what variables it uses, and how it uses them. Aside from the creation of the anonymous struct type, they currently behave just like struct literals, and I think that is important.

Losing this for some limited ergonomic gains that will be made redundant by planned future features seems ridiculous. Especially since it is possible to achieve the same result (callers don't have to use &mut, only one copy of the function using a trait object) today with very little extra effort by the interface creator.

[nikomatsakis]

I'm of two minds here. I hear and respect @rkjnsn's concern that having || be equivalent to a struct expression is a useful mental model and relatively easy to understand and explain. On the other hand, allowing || to desugar to either a struct or a reference to a struct, as needed, seems like it is only a slight change, and would still allow closures to be taught in the same fashion as before. I can imagine that this would be the sort of change that people don't even notice, in that they write code that they expect to work and... it works.

I guess the question is whether closures are different from other things, like arrays. I think that they are. It is great to understand that a closure can be desugared into a struct that implements a trait, but in terms of how they are used closures do not feel so very much like "data structures" but rather "control flow". This is why, for example, it makes sense to have closures default to by-ref and require the move keyword, and it might be what justifies additional sugar here. I think this is also why foo(&mut [1, 2, 3]) seems to me to be just fine, but foo(&mut || ...) feels odd -- it's kind of forcing me to acknowledge the desugaring of closures into data, whereas I'd prefer to ignore it most of the time. (And I might prefer if users could easily use closures without having to understand the desugaring, as well.)

[pythonesque]

Hm... I think after consideration I'm in favor of this RFC, even though I'm generally not in favor of such sugar. Because you can already replicate this behavior by dispatching inside a thin wrapper (as @rkjnsn pointed out), and it wouldn't apply to closures assigned to a variable, there isn't a situation where this would actually make your code harder to reason about, IMO.

[theemathas]

Huge -1 for this, since this problem is already solved by rust-lang/rust#23895 (discussion here), which was, incidentally, proposed and implemented after this RFC was proposed.

For example, this code from the RFC:

fn do_something(closure: &mut FnMut(i32) -> i32) {
    ...
}
...
do_something(&mut |x| x*2);

can be rewritten as:

fn do_something<F: FnMut(i32) -> i32>(closure: F) {
    ...
}
...
do_something(|x| x*2);

And for the case of recursive functions that take closures, I would say that explicitly writing out references and avoiding dynamic dispatch is the best, since you could accidentally change the function's asymptotic runtime otherwise. (with something similar to this)

[bluss]

@theemathas Letting it be caller's choice to use a trait object or not is my favourite solution, but how do you make sure do_something(&|x| x*2) uses an &Fn trait object and not a specific (anonymous) type?

[eddyb]

@bluss using type ascription, of course! E.g. do_something((&|x| x*2): &Fn(_)).

[eddyb]

I had a hunch that this PR was catering to an obsolete need, due to seeing only a couple occurrences of &mut |..| {...} and &mut FnMut(...) in the wild, so I used @brson's dl-cargo-packages.py script to get all crates.io sources and grep over them.
The results are at https://gist.github.com/eddyb/0b5bbdf8c1383ecec1a2.

There are around 200 relevant closure expressions and almost 90 Fn and FnMut trait object types, combined.
Meanwhile, single-simple-argument closures alone add up to over 24k and all the mentions to Fn and FnMut go a bit past 3.5k.

Maybe that's not enough reason to stop this PR, but it personally feels like a waste a time to cater for usecases that are going extinct, and it just adds up to technical debt.

The age of indirection and virtual calls has passed, rejoice the victory of where clauses in the fight for monomorphization!

[bluss]

In my experience, if you can use a &Fn(X) -> Y argument to avoid infinite regress in the signature for functions with recursively passed closures, you can normally use &F where F: Fn(X) -> Y the same way.

[nikomatsakis]

@eddyb you could view that as evidence that the annoyingness of writing functions that accept FnMut closures means nobody is using them, even when it might be beneficial. I know I rarely do, and I remember that when we ported the compiler we often had to do various contortions to handle recursive cases in particular (like introducing wrapper functions).

@bluss note that this change is equally helpful if you write fn foo<F>(x: &F) where F: Fn, in that the caller has merely to do foo(|| ...) vs foo(&|| ... ). Although the RFC describes the benefits primarily around trait objects, it is not limited to that case.

@theemathas can you elaborate a bit more on the example? I don't quite see how this is solving the problem yet. The "keep adding extra levels of indirection" problem is interesting, but strongly suggests that we should encourage people to handle recursive cases by pulling the reference into the fn signature, in which case this change makes that more ergonomic.

All that said, I continue to rest on a knife's edge here. Clearly this change makes the expansion of || more nuanced, and it's not clear that the need is dire enough. I do think we need to keep working on making it lighterweight to work with closures, but DST by value and impl Trait seem to me to be the most important steps in that regard; the case addressed here seems somewhat less, since it is primarily limited to recursive functions, particularly if DST by value comes to pass. Therefore, I'm leaning towards postponing this RFC at the moment, and revisiting once the story about by-value DST is somewhat clearer (that has complexities of its own...). That said, recursive functions are not particularly unusual, so making them easier (and less annoying) to get right may still be worth the trouble.

[brson]

I do want it to be more convenient and desirable to use virtual dispatch, since I believe there is great opportunity for profile-based tuning of function dispatch in Rust. This seems like it helps, but I haven't looked too closely.

[eddyb]

@brson yesterday on IRC I was mentioning adding static -> dynamic dispatch conversion to LLVM's mergefunc pass (so it can merge two functions that are identical except for some calls, which can be turned into indirect calls to a parameter of those functions).

@cmr informed me that there's an associated cost to indirect calls, at least on older CPU microarchitectures, mostly due to the fact that a call instruction cannot have its target cached for prediction (keyed by the actual location in memory of the call) if the call is indirect.

Now that you mention profile-guided optimizations, the kind of automatic virtualization I described seems perfect if the costs have been measured already.
I don't really know how hard it would be to add such a pass to LLVM, but there should be a [PGO infrastructure] to pull data from.

[nikomatsakis]

@eddyb I am somewhat skeptical that we ought to be automatically converting to static dispatch, but regardless we want users to be able to easily throw the switch themselves.

[eddyb]

@nikomatsakis What I was suggesting was optimizing away code bloat by turning static dispatch into dynamic dispatch, wherever there would be a measurable gain.
I know it's "sufficiently smart compiler" territory, but it seems more practical in the long run than manually changing generics to dynamic dispatch.

That said, if do we get some sugar, I'd prefer being able to pass trait objects by value, instead.

[nikomatsakis]

Per my last comment, I'm just going to withdraw this RFC for now. Perhaps we'll revisit this theme in the future after we've had more progress on closures in other areas. Thanks all for the comments and discussion!