2

I'm trying to create a generic collection for events so that it'll be reusable for different kind of event-sets. While playing around with variadic templates, I came across THIS answer, which helped me for my example here:

#include <boost/test/unit_test.hpp>

#include <string>
#include <unordered_map>

namespace
{
struct Event3 {
    static const int event_type = 3;
    int a;
};

struct Event5 {
    static const int event_type = 5;
    double d;
};

struct Event7 {
    static const int event_type = 7;
    std::string s;
};


template <class ...K>
void gun(K...) {}

template <class... Ts>
class EventCollection
{
    template <typename T>
    void update_map(std::unordered_map<int, size_t> & map, const T &)
    {
        BOOST_CHECK(map.find(T::event_type) == map.end());
        map[T::event_type] = sizeof(T);
    }


public:
    std::unordered_map<int, size_t> curr_map;

    EventCollection(Ts... ts)
    {
        gun(update_map(curr_map, ts)...); // will expand for each input type
    }
};

} // namespace

BOOST_AUTO_TEST_CASE( test_01 )
{
    Event3 x{13};
    Event5 y{17.0};
    Event7 z{"23"};

    EventCollection<Event3, Event5, Event7> hoshi(x, y, z);
    BOOST_CHECK_EQUAL(hoshi.curr_map.size(), 3);
}

However, the line

gun(update_map(curr_map, ts)...); // will expand for each input type

gives me an 'error: invalid use of void expression'. Can anybody tell me, how to solve this?

Community
  • 1
  • 1

2 Answers2

4

The problem is that your update_map returns void. Hence you cannot write this:

gun(update_map(curr_map, ts)...); 

because the return values of update_map is supposed to be passed to gun as arguments.

The fix is to pass something to gun as argument, so you can do this:

gun( (update_map(curr_map, ts),0)...); 

Now the expresssion (update_map(curr_map, ts),0) turns out to be 0 which is passed as argument to gun. That should work. You can think of this as:

T argmument = (update_map(curr_map, ts),0);  //argument is 0, and T is int

--

Also, as the other answer pointed out that the order of evaluation of arguments to gun() are unspecified (means the order in which the function update_map is called, is unspecified) which may lead to undesired result. The other solution has given a solution to this problem. Here is another one (which is a bit tricky and easy!):

//ensure that the size of the below array is at least one.
int do_in_order[] = {0, (update_map(curr_map, ts),0)...};

Because the order of initialization of array elements are well-defined (from left-to-right), now all the calls to update_map happens in well-defined order.

Nawaz
  • 353,942
  • 115
  • 666
  • 851
0

update_map is a function that returns void.

That line consists of calling update_map, and then passing the return value to gun.

You cannot pass a void return value to another function.

Hence "invalid use of void expression".

There are many ways to fix this, including having update_map return struct empty {};

Note that your code results in the calls of update_map happening in an unspecified order. This can easily lead to unexpected behavior.

Might I suggest:

void do_in_order();
template<typename F0, typename... Functors>
void do_in_order( F0&& f0, Functors&& funcs... ) {
  f0();
  do_in_order( std::forward<Functors>(funcs)... );
}

then replace the call to gun with:

do_in_order([&]{update_map(curr_map, ts);}...); // will expand for each input type

which packages up the things to do into lambdas, which are then called in order that they are passed.

Now, this also does away with the need for an update_map function entirely:

do_in_order([&]{
  BOOST_CHECK(curr_map.find(ts::event_type) == curr_map.end());
  map[ts::event_type] = sizeof(ts);
}...);

which is awesome.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • nice tip, but I don't really need the ordering (but I need different input types or more precisely: different event_types - maybe I should use a return of map_update to check that) – user2081073 Feb 17 '13 at 19:16
  • I think that is overkill. To enforce well-defined order, you can use array-initialization trick, as shown in my answer. :-) – Nawaz Feb 17 '13 at 19:21
  • @Nawaz except every time you do that, you have to say "I am doing this in order to enforce in-order evaluation of the tasks" in a comment. The `do_in_order` function both says what it does and then does it. The `forget` solution uses `int forget[]=` boilerplate, and `(X,0)` boilerplate, a total of 11 non-documenting boilerplate characters (+6 for `forget`), while `do_in_order` uses 8 non-documenting boilerplate characters (+11 for `do_in_order`). It is lighter weight, once the `do_in_order` has been written once! :) And I challenge you to find a situation where it will run slower. – Yakk - Adam Nevraumont Feb 17 '13 at 19:27
  • @Yakk: Is `int do_in_order[] = {(update_map(curr_map, ts),0)...};` better? – Nawaz Feb 17 '13 at 19:32
  • @Nawaz at least it is correct and does not fail for zero size event collections. – Johannes Schaub - litb Feb 17 '13 at 19:38
  • @JohannesSchaub-litb: Alright. Here you go : `int do_in_order[] = {0, (update_map(curr_map, ts),0)...};`. Fixed? – Nawaz Feb 17 '13 at 19:56
  • @Nawaz Too heavyweight: you just added more boilerplate! Plus, still fragile -- if `update_map` returns a type that overloads `operator,`, you no longer have an ordering guarantee. So you need another 6 characters of boilerplate to deal with that issue.. And even without that, the solution is just hackey: creating an array for no purpose other than exploiting ordering guarantees? If there was no other solution, I'd go for it. Meanwhile, take a look at the newest `do_in_order` use above -- I removed `update_map`, and `do_in_order` solution now takes fewer characters than the unordered sltn! – Yakk - Adam Nevraumont Feb 17 '13 at 20:49