38

I have C++ code that wraps an arbitrary lambda and returns the result of the lambda.

template <typename F>
auto wrapAndRun(F fn) -> decltype(F()) {
    // foo();
    auto result = fn();
    // bar();
    return result;
}

This works unless F returns void (error: variable has incomplete type 'void'). I thought of using a ScopeGuard to run bar, but I don't want bar to run if fn throws. Any ideas?

P.S. I found out later there's a proposal to fix this inconsistency.

Community
  • 1
  • 1

7 Answers7

25

You can write a simple wrapper class that handles this part of it:

template <class T>
struct CallAndStore {
    template <class F>
    CallAndStore(F f) : t(f()) {}
    T t;
    T get() { return std::forward<T>(t); }
};

And specialize:

template <>
struct CallAndStore<void> {
    template <class F>
    CallAndStore(F f) { f(); }
    void get() {}
};

You can improve usability with a small factory function:

template <typename F>
auto makeCallAndStore(F&& f) -> CallAndStore<decltype(std::declval<F>()())> {
    return {std::forward<F>(f)};
}

Then use it.

template <typename F>
auto wrapAndRun(F fn) {
    // foo();
    auto&& result = makeCallAndStore(std::move(fn));
    // bar();
    return result.get();
}

Edit: with the std::forward cast inside get, this also seems to handle returning a reference from a function correctly.

Curious
  • 20,870
  • 8
  • 61
  • 146
Nir Friedman
  • 17,108
  • 2
  • 44
  • 72
  • I like your solution better but are there some limitations on returned type based on fact it must be stored in your case? – Slava Dec 27 '17 at 18:26
  • Sorry, what exactly is the limitation? It just needs to be movable (let me add a std::move). – Nir Friedman Dec 27 '17 at 18:36
  • 1
    Maybe not limitations but differences, for example I am not sure destructor of `T` can be avoided in your case as RVO cannot be applied – Slava Dec 27 '17 at 19:51
  • @Slava That's a good point. An alternative would be write a function instead, `callAndReturn`, that takes two lambdas, with two overloads. Call the first lambda, store the result if non-void type, then call the second. This scales well (better than other answers) if `bar()` is actually a standin for a few lines of code. – Nir Friedman Dec 27 '17 at 20:30
  • You can also use functions to do the type deduction without having to wait for type deduction for constructors – Curious Dec 28 '17 at 04:38
  • @Curious Yeah, In that case though I'd want to be sure it doesn't trigger any more un-elided moves of the return type. I've already introduced one extra move here, definitely don't want to introduce another. – Nir Friedman Dec 28 '17 at 14:48
  • @NirFriedman you don't need any of that, even with C++14, https://wandbox.org/permlink/EaThZXFeddyi4oEv Also the `decltype(F())` could be improved for the same reason `result_of` is deprecated. See how I get the return type in the linked code – Curious Dec 28 '17 at 17:12
  • 1
    @Curious Nice trick, I agree that's strictly better. Feel free to edit the answer to include your improvements (and delete my comment about deduction guides). – Nir Friedman Dec 28 '17 at 17:36
  • 1
    @Curious I added your improvements to my answer, thanks! – Nir Friedman Dec 28 '17 at 19:31
22

The new C++17 if constexpr addition may be helpful here. You can choose whether to return fn()'s result at compile-time:

#include <type_traits>

template <typename F>
auto wrapAndRun(F fn) -> decltype(fn())
{
    if constexpr (std::is_same_v<decltype(fn()), void>)
    {
        foo();
        fn();
        bar();
    }
    else
    {
        foo();
        auto result = fn();
        bar();
        return result;
    }
}

As you said C++20 is an option as well, you could also make use of concepts, putting a constraint on the function:

template <typename F>
  requires requires (F fn) { { fn() } -> void }
void wrapAndRun(F fn)
{
    foo();
    fn();
    bar();
}

template <typename F>
decltype(auto) wrapAndRun(F fn)
{
    foo();
    auto result = fn();
    bar();
    return result;
}
Mário Feroldi
  • 3,463
  • 2
  • 24
  • 49
  • 1
    Can this be done without repeating the calls to `foo` and `bar`? – Nir Friedman Dec 27 '17 at 18:52
  • 1
    @NirFriedman Yes, but I have the opinion that the trade-off is not good. After all, this code isn't suffering because of repetition, and is simple enough. – Mário Feroldi Dec 27 '17 at 18:55
  • 1
    Sorry, what's the trade-off exactly? Also, it's an assumption that there are literally two function calls. In the OP's case or in the general case, there may be more code there, that the user may or may not feel like factoring out into a function. – Nir Friedman Dec 27 '17 at 19:18
  • 1
    @NirFriedman: if there's more code there, he should probably refactor that out, into `foo()` and `bar()` – Mooing Duck Dec 27 '17 at 20:15
  • @MooingDuck Err, why? Somewhere you have to have code, you know, it can't all be function calls. Such a factoring may be the right call, maybe even most of the time, but perhaps not all of the time. A solution that basically forces you to factor out into `foo` and `bar` is worse than one that doesn't (all other things being equal, of course). – Nir Friedman Dec 27 '17 at 20:22
  • @NirFriedman it could be refactored to lambdas inside the function template. – Mário Feroldi Dec 27 '17 at 22:01
  • Main purpose of generic code is to avoid code duplicate. Write duplicated code in generic - what could be worse? – Slava Dec 28 '17 at 16:50
6

You can have a look at the ScopeGuard by Alexandrescu: ScopeGuard.h It executes code only when there was no exception.

template<class Fn>
decltype(auto) wrapAndRun(Fn&& f) {
  foo();
  SCOPE_SUCCESS{ bar(); }; //Only executed at scope exit when there are no exceptions.
  return std::forward<Fn>(f)();
}

So in case of no errors, the execution order is: 1. foo(), 2. f(), 3. bar(). And in case of an exception the order is: 1. foo(), 2. f()

Kilian
  • 533
  • 4
  • 11
  • 1
    This is interesting. I didn't realize it was possible to write `uncaught_exceptions` (notice the s) prior to 17 in user space; this is a prerequisite for things like SCOPE_SUCCESS. Nice answer! – Nir Friedman Dec 28 '17 at 14:52
5

Another trick might be to exploit the comma operator, something like:

struct or_void {};

template<typename T>
T&& operator,( T&& x, or_void ){ return std::forward<T>(x); }

template <typename F>
auto wrapAndRun(F fn) -> decltype(fn()) {
    // foo();
    auto result = ( fn(), or_void() );
    // bar();
    return decltype(fn())(result);
}
Massimiliano Janes
  • 5,524
  • 1
  • 10
  • 22
  • 9
    @MassimilianoJanes Overloading the comma operator is one of those things that a decent chunk of the C++ community (self included) will just tell you never to do. Yes, I understood the idea, but this is 100%, dead on, the kind of black magic overloading that so, so many people and style guides have recommended against. If there was really, really no other way it might be worth considering but there's 4 solutions already here that are all preferable. – Nir Friedman Dec 27 '17 at 22:20
  • 5
    @NirFriedman respectfully, I disagree; first, the comma operator *is* used in existing widely used libraries (boost, Eigen to cite the first that come to my mind), so claiming that *everybody* say to *never* use it, is simply false. Second, the reasons people should (rightfully) avoid overloading the comma operator do not apply here, you can easily make this safe. – Massimiliano Janes Dec 27 '17 at 22:38
  • 5
    third, we don't know the final aim and scope of the OP code; he might just need a technical solution to a problem, buried in implementation details. This answer is a valid solution to the OP question, period. Afaik, downvoting it just because it 'feels' bad to you goes against the meaning of voting. – Massimiliano Janes Dec 27 '17 at 22:39
  • 1
    You're entitled to disagree, and I'm entitled to downvote. It's a solution I would never recommend to anyone who needs to ask for a solution to OP's problem in the first place, put it this way. I'm allowed to downvote a solution I would never recommend, especially when there are multiple answers already up that I would recommend, aren't I? I'm obviously not the only one who feels this way about the comma operator: https://stackoverflow.com/questions/5602112/when-to-overload-the-comma-operator. – Nir Friedman Dec 27 '17 at 22:47
  • 2
    Beyond that, it also doesn't correctly handle functions that return reference type (easily fixable), and it also doubly invalidates RVO, i.e. when I run this code there are 2 extraneous destructor calls, as opposed to either one or zero found in most other solutions here. – Nir Friedman Dec 27 '17 at 22:58
  • 2
    @NirFriedman uhm, the very answer you linked to in your comment mentions that comma can be used as a metaprogramming technique (as is sometimes used indeed) among other legit uses. Frankly, I believe guidelines exist (or should exist) as a means to comunicate the *reasons* why something is bad, not to sell dogmas about one should always/never do. Regarding, the downvote issue, I think the voting mechanism already allow establishing a hierarchy of answers; if I read SO help and meta posts correctly, downvotes should be reserved to non useful or wrong answers only. – Massimiliano Janes Dec 27 '17 at 23:02
  • regarding the reference and RVO issue, these are solvable and have nothing to do with the answer idea ... anyway, it seems I triggered a vote war :) ...so I'll stop here – Massimiliano Janes Dec 27 '17 at 23:03
  • 1
    @MassimilianoJanes If you solve both issues I'll remove the downvote, how's that as a peace offering :-). – Nir Friedman Dec 27 '17 at 23:06
4

Solution with SFINAE, the idea is to make internally a void function actually return int - hopefully the compiler will optimize this away. On outside wrapAndRun will return the same type as a wrapped function.

http://coliru.stacked-crooked.com/a/e84ff8f74b3b6051

#include <iostream>

template <typename F>
auto wrapAndRun1(F fn) -> std::enable_if_t<!std::is_same_v<std::result_of_t<F()>, void>, std::result_of_t<F()>> {
    return fn();
}


template <typename F>
auto wrapAndRun1(F fn) -> std::enable_if_t<std::is_same_v<std::result_of_t<F()>, void>, int> {
    fn();
    return 0;
}

template <typename F>
auto wrapAndRun(F fn) -> std::result_of_t<F()> {
    // foo();
    [[maybe_unused]] auto result = wrapAndRun1(fn);    
    // bar();        
    if constexpr (std::is_void_v<std::result_of_t<F()>>)
        return;
    else
        return result;
}

int main()
{
    wrapAndRun([] { std::cout << "with return \n"; return 0; });
    wrapAndRun([] { std::cout << "no return \n"; });
}
marcinj
  • 48,511
  • 9
  • 79
  • 100
  • I think OP wants to run `bar()` in case `fn()` return type is void – Slava Dec 27 '17 at 18:17
  • Not sure why the top enable_if seems to have some repetition? I think in any case the answer would benefit from writing a separate `returns_void` trait. Though, the significant disadvantage here is that you have to repeat `foo` and `bar` in two places (and indeed one was already forgotten). – Nir Friedman Dec 27 '17 at 18:18
  • 1
    Code repetition is ugly and I do not think that is a good solution if can be avoided. – Slava Dec 27 '17 at 18:19
  • @Nir Friedman - I have removed duplications. I think it looks quite nice now. – marcinj Dec 28 '17 at 10:36
  • @marcinj Sorry to be a pedant, but I'm a bit concerned that when the passed function returns void, the wrapper still returns `0`. This might be "good enough" for the OP but I feel like it's not optimal; could prevent catching a mistake at compile time. – Nir Friedman Dec 28 '17 at 14:55
  • @Nir Friedman I understand, I have updated answer - now it returns the same type as the wrapped function has. – marcinj Dec 28 '17 at 16:34
  • If you are already using `if constexpr`, how about this: http://coliru.stacked-crooked.com/a/705050e725bc72e9? Looks pretty good! Though this still doesn't handle functions returning reference types. Either way, have an upvote :-). – Nir Friedman Dec 28 '17 at 17:02
2

The most convenient way to do solve this problem is to wrap void in a type that you can actually use, along with a version of std::invoke that converts void to this regular type. Using my void library (C++17):

template <typename F>
auto wrapAndRun(F fn) -> vd::wrap_void<std::invoke_result_t<F&>>
    // foo();
    auto result = vd::invoke(fn);
    // bar();
    return result;
}

If fn() isn't void, this does the same thing that it did before. Returns whatever type. If fn() is void, then result has type vd::Void, which is just some empty type.

This is convenient because you put all the special-case void handling into one spot (in this case, the library) and then the rest of your code looks... normal. You don't have to write any bespoke special cases for each use, and you don't have to reorganize the logic of your code based on the solution you choose - you can still call foo() and bar() before and after fn just fine.

Barry
  • 286,269
  • 29
  • 621
  • 977
0

This is pretty awkward and not extendable solution but very short and simple and it supports RVO which could be significant for some return types:

template <typename F>
auto wrapAndRun(F fn) -> decltype(F()) {
    // foo();
    char c;
    auto runner = std::unique_ptr<char, decltype(bar)>( &c, bar );
    try {
        return fn();
    }
    catch( ... ) {
        runner.release();
        throw;
    }
}
Slava
  • 43,454
  • 1
  • 47
  • 90