C++11 std::forward a pointer

Question

I have a Signal class in my application that provides classes with an option to expose events (same as in .NET).

The class works and all is well.

Yesterday I saw this SO question (and its answer) and was familiarized with std::forward.

I decided to try to use it in my code so I changed every std::function<void(Args...)> to std::function<void(Args&&...)> and in the raise function (the operator()) I used the same logic I saw in the above link so now the function takes Args&&...args and the callback uses std::forward<Args>(args)...

Here's a simplified version of my Signal class (with some changes to make it a good example):

template<typename... Args> class Signal
{
public:
    int operator+=(const std::function<void(Args&&...)>& func)  {
        int token = getNewToken();
        m_subscribers.emplace(token, func);
        return token;
    }

    void operator()(Args&&... args) {
        for (auto it : m_subscribers) {
            it.second(std::forward<Args>(args)...);
        }
    }

private:
    std::map<int, std::function<void(Args&&...)>> m_subscribers;
};

int main() {
    int* six = new int(6);
    int seven = 7;

    Signal<int*> e1;
    e1 += [](int* x) { std::cout << *x; };

    Signal<int> e2;
    e2 += [](int x) { std::cout << x; };

    e1(&seven);
    e2(6);

    e1(six);  //Error C2664 'void Signal<int *>::operator ()(int *&&)': 
              //  cannot convert argument 1 from 'int *' to 'int *&&'
    e1(std::move(six)); //This is a workaround          
    return 0;
}

The issue I'm seeing is with classes (or main in this example) that try to raise events with pointers and I'm not sure how to solve this.

My main goal is to have the Signal class a general API and if the developers chose to use Signal<int*> I don't want him\her to raise with std::move.

What am I doing wrong here?

Answer 1

T&& is only a universal reference if T is a non-cv-qualified function template parameter. In your call operator:

void operator()(Args&&... args) {

Args isn't a template parameter of the function, it's a template parameter of the class. So for Signal<int*>, this operator() takes an rvalue reference to int*. Since six is an lvalue, that fails.

What you want is to provide the correct reference qualifications to Signal. Like so:

template<typename... Args>
class Signal
{
    using F = std::function<void(Args...)>; // NB: Just Args...
public:
    int operator+=(F func)  {
        int token = getNewToken();
        m_subscribers.emplace(token, std::move(func));
        return token;
    }

    void operator()(Args... args) {       // NB: just Args...
        for (auto& it : m_subscribers) {  // NB: auto&
            it.second(args...);
        }
    }

private:
    std::map<int, F> m_subscribers;
};

Note that forwarding Args... is questionable anyway. What if you had two subscribers? Once you forward the args once you can't really use them a second time.

The above will make Signal<int*> do what you expect. The operator() will just take an int*, which you can pass either an lvalue or an rvalue to.

Answer 2

Barry's answer is correct, but perhaps not as clearly explained as it could be.

&& is only given its special treatment as a forwarding (or "universal") reference when template parameter deduction occurs. But there is no deduction occurring here:

Signal<int*> e1;  // `Args...` is explicitly `int*`
...
e1(six);          // `Args...` *has already been specified*

When template classes are instantiated, they are essentially transformed into normal classes that just happen to be written by the compiler. See this answer for an example of what this might look like if written out in C++ code.

In C++14, there is no way to trigger template-parameter deduction of class templates without an auxiliary function (not a constructor):

template <typename Args...>
Signal<Args...> make_signal(Args&&...) { return Signal<Args...>; }

....But note that in your case, this makes no sense: you don't want to infer the types of your arguments when create the Signal, you want to specify them in advance.

(Note that in C++17, there will be support for template argument deduction of class templates. I assume this means that it will be possible to forward template-class arguments, though it's not immediately clear to me what the implications of doing such a thing would be.)

What you want to permit is for arguments to be forwarded at call time. This is actually reasonably simple:

template<typename... Args> class Signal
{
public:
    // .... skipping some code...

    template <typename... CallArgs>
    void operator()(CallArgs&&... args) {
        callback(std::forward<CallArgs>(args)...);
    }
};

....But, again, in your case this doesn't quite make sense, as noted in Barry's answer. You don't want to forward arguments if you have multiple callbacks, to prevent moving and re-using them.

It's possible to work around this by checking the size of m_subscribers and only using the forwarding code if it's 1, and just passing the arguments as-is otherwise. However, this might lead to confusing behavior, since the way callbacks are invoked shouldn't generally depend on the state of your Signal object. So you could perhaps write a separate class, SingletonSignal, for callbacks that must be invoked with forwarded arguments (e.g. in case a callback wants to transfer ownership of an uncopyable object such as unique_ptr).