tower120
tower120

Reputation: 5265

c++11 constructor with variadic universal references and copy constructor

How to declare copy constructor, if we have constructor with universal reference arguments, also?

http://coliru.stacked-crooked.com/a/4e0355d60297db57

struct Record{
    template<class ...Refs>
    explicit Record(Refs&&... refs){
        cout << "param ctr" << endl;
    }

    Record(const Record& other){     // never called
        cout << "copy ctr" << endl;
    }

    Record(Record&& other){         // never called
        cout << "move ctr" << endl;
    }    
};

int main() {
    Record rec("Hello");    
    Record rec2(rec);  // do "param ctr"

    return 0;
}

According to this constructor list of std::tuple http://en.cppreference.com/w/cpp/utility/tuple/tuple [look case 3 and 8] this problem somehow solved in standard library... But I can't get through stl's code.


P.S. Question somewhat related to C++ universal reference in constructor and return value optimization (rvo)

P.P.S. For now, I just added additional first param Record(call_constructor, Refs&&... refs) for really EXPLICIT call. And I can manually detect if we have only one param and if it is Record, and than redirect call to copy ctr/param ctr, but.... I can't believe there is no standard way for this...

Upvotes: 9

Views: 1409

Answers (3)

Barry
Barry

Reputation: 303337

The Problem

When you call Record rec2(rec);, you have two viable constructors: your copy constructor Record(Record const&) and the variadic constructor with Refs = {Record&} which works out to Record(Record&). The latter is a better candidate since it's a less cv-qualified reference, so it wins even if that's not what you want.

The Solution

You want to remove anything that should call the move or copy constructors from being a viable candidate for the variadic constructor. In plain English, if Refs... consists of a single type that is either a reference to, or just a plain value, of a type that derives from Record - we do NOT want to use the variadic constructor. It's important to include the derived case as well, since you would certainly expect SpecialRecord sr; Record r(sr); to call the copy constructor...

Since this comes up, it's useful to have as a type trait. The base case is that it's neither a copy or move:

template <typename T, typename... Ts>
struct is_copy_or_move : std::false_type { };

We only have to specialize on a single type:

template <typename T, typename U>
struct is_copy_or_move<T, U>
: std::is_base_of<T, std::decay_t<U>>
{ }

And then we just have to replace our variadic constructor with this SFINAE'd alternative:

template <typename... Refs,
          typename = std::enable_if_t<!is_copy_or_move<Record, Refs...>::value>
          >
Record(Refs&&...);

Now if the arguments are such that this should be a call to a copy or move constructor, the variadic constructor will no longer be viable.

Upvotes: 3

Nikos Athanasiou
Nikos Athanasiou

Reputation: 31549

It's a bad practice to overload on forwarding references (see Effective modern C++, Item 26). They tend to devour everything you pass to them due to overload resolution rules.

In your example, you're constructing a Record object out of a non-const Record object and that's why your copy ctor doesn't get executed. If you call it like this

Record rec2(const_cast<Record const&>(rec));

then it works as expected.

A solution is to do SFINAE on the constructor with the forwarding references and disable the case a copy ctor should be called; it becomes somewhat ugly to write though in the variadic case :

template <
    class Ref1, class ...Refs, 
    typename = typename std::enable_if <
        !std::is_same<Ref1, Record&>::value || sizeof...(Refs)
    >::type
>
explicit Record(Ref1&& ref, Refs&&... refs)
{
    cout << "param ctr" << endl;
}

Now calling

 Record rec2(rec); // calls copy ctor 

dispatches to the copy constructor since the template can't be instantiated for Record&

Demo


If your find yourself doing this a lot (not recommended) you could remove some clutter by defining a type trait to do the SFINAE

template<class T1, class T2, class... Refs>
using no_copy_ctor = typename std::enable_if <
    !std::is_same<T1, T2>::value || sizeof...(Refs)>::type;

thus writing the above as

template<class Ref1, class ...Refs, typename = no_copy_ctor<Record&, Ref1, Refs...>>
explicit Record(Ref1&& ref, Refs&&... refs)
{ /*...*/ }

Upvotes: 2

Jarod42
Jarod42

Reputation: 217810

In your example, the forwarding reference is used with Record&.

So you may add an extra overload for Record& (to forward to copy constructor):

Record(Record& other) : Record(static_cast<const Record&>(other)) {}

or use sfinae on the one with forwarding reference.

Upvotes: 4

Related Questions