Storing variable arguments to be forwarded later

Question

I am working on a small program that executes action sequentially using an action queue.

I would like to be able to store parameters in my actions until they are executed (parameters should then be accessible from the exec() method of the action).

I have a small example below :

#include <tuple>
#include <iostream>
#include <memory>
#include <utility>
#include <queue>

/**
 * Action interface
 */
struct Action {
    Action() {}
    virtual void exec() = 0;
};


/**
 * This action creates an object which type is given as template
 * and passes the parameters given to its ctor. On completion, it
 * prints its ID.
 */
template<class T, class... Ps>
struct CustomAction : public Action {
    // trying to store the variable arguments
    std::tuple<Ps...> _args;
    int _actionId;

    CustomAction(int id, Ps&&... args) : _actionId(id), 
        _args(std::make_tuple(std::forward<Ps>(args)...)) {
    }

    virtual void exec() override {
        T* item = new T(std::forward<Ps>(_args)...);

        std::cout << "done " << _actionId << std::endl;
    }
};

struct ActionQueue {

    std::queue<Action*> _queue;

    ActionQueue() {

    }

    void exec() {
        while(_queue.size()) {
            auto action = _queue.front();
            action->exec();
            _queue.pop();
        }
    }

    template<class T, class... Ps>
    void push(Ps&&... args) {
        auto action = new T(std::forward<Ps>(args)...);

        _queue.push(action);
    }
};

/**
 * Example item that is to be created. Nothing important here
 */
struct Item {

    int _b;

    Item(int b) : _b(b) {

    }
};

int main() {


    ActionQueue aq;

    int actionId = 5;
    int param = 2;

    aq.push<CustomAction<Item>>(actionId, param);

    // do stuff

    aq.exec();
}

In this example, I create an ActionQueue. I push a new CustomAction in the queue. This action simply creates an Item and gives its ctor the parameters that I had given when I pushed the action on the action queue.

My problem is that I don't know why the parameter given to the push() method is not available to use in the CustomAction class.

Compiling the above example gives me the following error :

<source>:56:27: error: no matching constructor for initialization of 'CustomAction<Item>'

        auto action = new T(std::forward<Ps>(args)...);

                          ^ ~~~~~~~~~~~~~~~~~~~~~~

<source>:82:8: note: in instantiation of function template specialization 'ActionQueue::push<CustomAction<Item>, int &, int &>' requested here

    aq.push<CustomAction<Item>>(actionId, param);

       ^

<source>:27:5: note: candidate constructor not viable: requires single argument 'id', but 2 arguments were provided

    CustomAction(int id, Ps&&... args) : _actionId(id), 

    ^

<source>:22:8: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 2 were provided

struct CustomAction : public Action {

       ^

<source>:22:8: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 2 were provided

1 error generated.

Compiler returned: 1

The error says that CustomAction needs one parameter while two were given, but CustomAction should accept the second argument in args.

What am i doing wrong here ?

Thanks

Maybe this could help? https://stackoverflow.com/a/10766422/1594913 — JBL, Oct 08 '18 at 13:03
Or this: https://stackoverflow.com/questions/7858817/unpacking-a-tuple-to-call-a-matching-function-pointer. `T(std::forward(_args)...)` will not unpack the tuple — NathanOliver, Oct 08 '18 at 13:05
Why not use a design based on stored lambda? That is a `std::queue>`. — Phil1970, Oct 08 '18 at 13:12
@max66 using c++17. And I cannot change the constructor of `Item` to accept tuples or anything. The constructor cannot be changed — Ebatsin, Oct 08 '18 at 13:12
In the above code, `Action` struct must have a virtual destructor. And when you remove items from the queue, you need to delete them. Preferred solution would be to use a smart pointer instead (for ex. `std::unique_ptr`. — Phil1970, Oct 08 '18 at 13:16
@Phil1970 The point of this example is to be short. In my code, the action is in a `shared_ptr` and I, indead, forgot to add a destructor. This was not my question though — Ebatsin, Oct 08 '18 at 13:29

score 4 · Accepted Answer · answered Oct 08 '18 at 13:29

What am i doing wrong here ?

Well... first of all you need to unpack the tuple; by example, using an helper function, something as follows (std::index_sequence and std::make_index_sequence are available from C++14; you're compiling C++17 so you can use they)

template <std::size_t ... Is>
void exec_helper (std::index_sequence<Is...> const &)
 { T* item = new T{std::get<Is>(_args)...}; }

virtual void exec() override {
    exec_helper(std::make_index_sequence<sizeof...(Ps)>{});

    std::cout << "done " << _actionId << std::endl;
}

But there are other problems.

By example: you can use perfect forwarding only with template arguments of the functions/methods, so not in this constructor

CustomAction(int id, Ps&&... args) : _actionId(id), 
    _args(std::make_tuple(std::forward<Ps>(args)...)) {
}

because Ps... are template arguments of the class, not of the constructor.

Should works something as

template <typename ... Us>
CustomAction (int id, Us && ... args) : _actionId{id}, 
    _args{std::forward<Us>(args)...}
 { }

And I suppose that

aq.push<CustomAction<Item>>(actionId, param);

should be

// ........................VVV
aq.push<CustomAction<Item, int>>(actionId, param);

I had never heard of `std::index_sequence` before and that it allowed you to unpack from `get`. It works, thanks ! — Ebatsin, Oct 08 '18 at 14:26

Richard Hodges · Answer 2 · 2018-10-08T13:49:08.983

Refactored and corrected with comments inline:

#include <tuple>
#include <iostream>
#include <memory>
#include <utility>
#include <queue>

/**
 * Action interface
 */
struct Action {
    Action() {}

    // Comment #8 - virtual destructor required for polymorphic objects!
    virtual ~Action() {}
    virtual void exec() = 0;
};


/**
 * This action creates an object which type is given as template
 * and passes the parameters given to its ctor. On completion, it
 * prints its ID.
 */
template<class T, class... Ps>
struct CustomAction : public Action {
    // Comment #1 - Ps will be full types. Not references.
    // trying to store the variable arguments
    std::tuple<Ps...> _args;
    int _actionId;

    // Comment #2 - deduced arguments in the constructor allow perfect forwarding.
    template<class...Args>
    CustomAction(int id, Args&&... args) 
    : _actionId(id)
    , _args(std::forward<Args>(args)...) {
    }

    virtual void exec() override {
        // Comment #3 - The use of a lambda allows argument expansion for a constructor.
        auto create = [](auto&&...args)
        {
            return new T(std::forward<decltype(args)>(args)...);
        };

        // Comment #4 - c++17's std::apply allows us to call the lambda with arguments forwarded out of the tuple
        T* item = std::apply(create, std::move(_args));
        // Comment #9 do something with this pointer...

        std::cout << "done " << _actionId << std::endl;
    }
};

struct ActionQueue {

    std::queue<Action*> _queue;

    ActionQueue() {

    }

    void exec() {
        while(_queue.size()) {
            auto action = _queue.front();
            action->exec();
            _queue.pop();
        }
    }

    // Comment #5 - separate concerns - the queue only cares about the Action interface
    void push(Action* pa)
    {
        _queue.push(pa);
    }
};

/**
 * Example item that is to be created. Nothing important here
 */
struct Item {

    int _b;

    Item(int b) : _b(b) {

    }
};

// Comment #6 - A helper function to deduce stored argument types
template<class Action, class...Args>
auto makeCustomAction(int id, Args&&...args)
{
    using CustomActionType = CustomAction<Action, std::decay_t<Args>...>;
    return new CustomActionType(id, std::forward<Args>(args)...);
}

int main() {


    ActionQueue aq;

    int actionId = 5;
    int param = 2;

    // Comment #7 - store-and-forwarded arguments are now auto deduced.
    aq.push(makeCustomAction<Item>(actionId, param));

    // do stuff

    aq.exec();
}

https://godbolt.org/z/ZLkfi5

As original code, that solution has memory leaks… See my comment to the question. — Phil1970, Oct 08 '18 at 13:38
moving member might result in unexpected result if call twice. — Jarod42, Oct 08 '18 at 13:39
Both comments noted and agreed. I guess I had to draw the line somewhere otherwise I'd have ended up posting an entire execution queue implementation. @Jarod42 yes, I have deliberately assumed the OP's intent was to perfectly forward arguments for a single execution. — Richard Hodges, Oct 08 '18 at 13:46

Storing variable arguments to be forwarded later

2 Answers2

Linked