17

I came up with the following code to transform a R()-like into a void()-like callable:

#include <utility>

template<class Callable>
auto discardable(Callable&& callable)
{ return [&]() { (void) std::forward<Callable>(callable)(); }; }
//        ^-- is it ok?

int main()
{
    auto f = discardable([n=42]() mutable { return n--; });
    f();
}

I'm worried about the capture by reference.

  1. Is it well-defined?
  2. Am I guaranteed that callable is never copied and never used after its lifetime has ended?

This is tagged C++14, but applies to all following standards.

YSC
  • 38,212
  • 9
  • 96
  • 149
  • 4
    Since `callable` can be an xvalue there is a chance that it gets destroyed before the lambda capture, hence leaving you with a dangling reference in the capture. – Maxim Egorushkin Jan 29 '19 at 10:47

3 Answers3

13

Lambdas are anonymous structs with an operator(), the capture list is a fancy way of specifying the type of its members. Capturing by reference really is just what it sounds like: you have reference members. It isn't hard to see the reference dangles.

This is a case where you specifically don't want to perfectly forward: you have different semantics depending on whether the argument is a lvalue or rvalue reference.

template<class Callable>
auto discardable(Callable& callable)
{
    return [&]() mutable { (void) callable(); };
}

template<class Callable>
auto discardable(Callable&& callable)
{
    return [callable = std::forward<Callable>(callable)]() mutable {  // move, don't copy
        (void) std::move(callable)();  // If you want rvalue semantics
    };
}
Passer By
  • 19,325
  • 6
  • 49
  • 96
  • 1
    Are you sure `forward` and `move` should not be reversed? – Maxim Egorushkin Jan 29 '19 at 12:40
  • @MaximEgorushkin nop. It is correct. But I'd rather use move on both; it is less error-prone. Forward should almost always get called with its type parameter explicitly provided, which is a source of errors due to chance to omit that parameter. – Red.Wave Jan 29 '19 at 13:46
  • @MaximEgorushkin The `forward` and `move` is like that because I find it easier to understand in those contexts. The first says "whatever type it was, use the same", the second says "cast this lvalue to xvalue". In the end, it's preference I guess. Fixed compile errors BTW. – Passer By Jan 29 '19 at 14:32
  • `(void) std::move(callable)();` - I don't know OP's intent is, but this basically means that in case `operator()` is a one-time operation on `Callable&&`, the lambda can only be called safely once, which does not feel right. – Holt Jan 29 '19 at 20:39
  • @Holt It may or may not be intentional by OP, I can't tell. The use of `std::move` and the comment should hopefully attract enough attention to the matter. – Passer By Jan 31 '19 at 15:36
7

Since callable can be an xvalue there is a chance that it gets destroyed before the lambda capture, hence leaving you with a dangling reference in the capture. To prevent that, if an argument is an r-value it needs to be copied.

A working example:

template<class Callable>
auto discardable(Callable&& callable) { // This one makes a copy of the temporary.
    return [callable = std::move(callable)]() mutable {
        static_cast<void>(static_cast<Callable&&>(callable)());
    };
}

template<class Callable>
auto discardable(Callable& callable) {
    return [&callable]() mutable {
        static_cast<void>(callable());
    };
}

You can still face lifetime issues if callable is an l-value reference but its lifetime scope is smaller than that of the lambda capture returned by discardable. So, it may be the safest and easiest to always move or copy callable.

As a side note, although there are new specialised utilities that perfect-forward the value category of the function object, like std::apply, the standard library algorithms always copy function objects by accepting them by value. So that if one overloaded both operator()()& and operator()()&& the standard library would always use operator()()&.

Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
  • Does it need `std::move` in the capture thingie? So as to avoid a copy if unnecessary? Genuine question; I am a little behind on the latest gadgets. – Lightness Races in Orbit Jan 29 '19 at 11:01
  • Are you refering to `[capture.thingie]/2`? – YSC Jan 29 '19 at 11:02
  • @YSC: That's the badger – Lightness Races in Orbit Jan 29 '19 at 11:02
  • 2
    @LightnessRacesinOrbit Someone may declare `operator()()&&` along with `operator()()&`. IMO, that is brittle, but conceivable. Although the standard library just makes copies of function objects and is not concerned with such trifles. – Maxim Egorushkin Jan 29 '19 at 11:03
  • 1
    Just seems like a wasted copy for nothing. You can still invoke `operator()()&&` on it – Lightness Races in Orbit Jan 29 '19 at 11:04
  • @MaximEgorushkin I'd consider it a good answer: "Don't waste your time, copy the damn callable this is what the stdlib does." – YSC Jan 29 '19 at 11:05
  • 2
    *"the standard library always copies function objects by accepting them by value"*. That's what STL **did** when forwarding references didn't exist. [std::apply](https://en.cppreference.com/w/cpp/utility/apply) uses forwarding reference as counter-example. – Jarod42 Jan 29 '19 at 11:10
  • @Jarod42 And still does. See, for example, https://en.cppreference.com/w/cpp/algorithm/sort , it still takes `Compare comp` by value. – Maxim Egorushkin Jan 29 '19 at 11:14
  • @Jarod42 Reworded for you. – Maxim Egorushkin Jan 29 '19 at 11:20
  • 1
    So, why would you copy when you can move? I totally don't get it. `return [callable = std::move(callable)]() mutable {…}` – Arne Vogel Jan 29 '19 at 12:22
  • @MaximEgorushkin: For `std::sort`, for retro-compatibility for the first 3 overloads (between copy versus lvalue-reference of old code). For last one with `Policy`, I would say for coherency with the other overloads. – Jarod42 Jan 29 '19 at 13:33
  • @Jarod42 Show me a standard algorithm from https://en.cppreference.com/w/cpp/header/algorithm that takes the function object as forwarding reference. – Maxim Egorushkin Jan 29 '19 at 13:47
  • @Jarod42 With multi-threaded execution policy it has to make copies of the function object to avoid lock contention. – Maxim Egorushkin Jan 29 '19 at 14:03
  • @MaximEgorushkin: apart for historic reasons, there are no reasons to pass by value by default (as you said, some code as Policy execution may require it). Forwarding references was young in C++11 to use it by default, that might explain also why even C++11 functions use by copy. For backward compatibility, we cannot change that. – Jarod42 Jan 29 '19 at 14:12
  • @Jarod42 I do not think there is a need to pass functional objects by references, really. One can always use `std::ref()` to avoid copying the function object (that is the reason it has `operator()()`), but in my experience that has been rarely if ever necessary. – Maxim Egorushkin Jan 29 '19 at 14:30
6

Your program is UB as you use dangling reference of the captured lambda.

So to perfect forward capture in lambda, you may use

template<class Callable>
auto discardable(Callable&& callable)
{
    return [f = std::conditional_t<
             std::is_lvalue_reference<Callable>::value,
             std::reference_wrapper<std::remove_reference_t<Callable>>,
             Callable>{std::forward<Callable>(callable)}]
    { 
        std::forward<Callable>(f)(); 
    };
}

It move-constructs the temporary lambda.

Jarod42
  • 203,559
  • 14
  • 181
  • 302
  • 1
    Is forwarding captured variable really necessary? – bartop Jan 29 '19 at 10:54
  • IMO you should not forward `f` since you may want to call the lambda multiple times. – Holt Jan 29 '19 at 20:38
  • @Holt: That `forward`/`move` only changes category value of the functor. There is no assignment to transfer ownership. `operator()()&&` might invalidate itself, but in that case, presence of this kind of overload might mean that user wants to respect category value. – Jarod42 Jan 29 '19 at 20:48
  • @Jarod42 I am talking about `operator()()&&` invalidating `f` as you mention. I don't know OP's intent, I'm just saying this might be worth mentioning. – Holt Jan 29 '19 at 20:50