3

Trying to understand why having a parameter pack templated constructor for a class apparently causes both the copy constructor and the copy assignment operator to be optimized out. (Actually I can see how the compiler wouldn't be able to discern the copy constructor signature as different from the templated constructor, but seems like it should be obvious when the copy assignment operator is used)

Example code"

#include <array>
#include <iostream>


struct A
{
    std::array<int,3> a;

    template<typename ... Args>
    A(Args&& ... args)
        :   a{args ...}
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }
    
    A(const A& other)
        :   a{other.a}
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }

    A& operator=(const A& other)
    {
        a = other.a;
        std::cout << __PRETTY_FUNCTION__ << std::endl;
        return *this;
    }
};

int main()
{
    A a(1,2.f,3.);
    A b = a; // fails (see compiler output)
    //A c(a); // also fails (templated constructor is better match)
}

Compile output:

templated_constructror.cpp: In instantiation of ‘A::A(Args&& ...) [with Args = {A&}]’:
templated_constructror.cpp:35:8:   required from here
templated_constructror.cpp:11:15: error: cannot convert ‘A’ to ‘int’ in initialization
   11 |   : a{args ...}
fabian
  • 80,457
  • 12
  • 86
  • 114
  • Why `A(Args&&... args)` instead of `A(Args... args)`? – Eljay May 08 '23 at 17:36
  • 1
    ```A(Args&& ... args)``` is a better match than ```A(const A& other)```. If I understand well, its because you have a A in input that can be deduced by the first constructor whereas the copy constructor implies a "conversion" from non-const to const. In order to fix this, you may have a look to item 27 in Effective Modern C++ by Scott Meyers. – Oersted May 08 '23 at 17:42
  • 1
    @Eljay not sure about the OPs intentions, but this could make a difference, if a type has a copy constructor and a non-explicit conversion operator to `int` that's useable on a const-qualified object: If you use `Args...`, a potentially expensive copy would be made, but with `Args&&...` the argument would be passed by reference... – fabian May 08 '23 at 17:48

3 Answers3

2

Since you're doing a declaration and assignment in the same expression,

A b = a

uses a constructor.

Since a is non-const qualified, the constructor taking an lvalue reference to non-const (A&) is a better match given the argument passed to the constructor.

If you actually pass a lvalue reference to a const A, the code compiles:

A b = static_cast<A const&>(a);

If you want this to work as expected, add a constructor overload with a parameter of type A&:


struct A
{
    std::array<int, 3> a;

    template<typename ... Args>
    A(Args&& ... args)
        : a{ static_cast<int>(args) ... }
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }

    A(const A& other)
        : a{ other.a }
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }

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

    A& operator=(const A& other)
    {
        a = other.a;
        std::cout << __PRETTY_FUNCTION__ << std::endl;
        return *this;
    }
};

int main()
{
    A a(1, 2.f, 3.);
    A b = a;
    A c(a);
}

Alternatively you could change the set of constructors as follows and the constructor template being a match, if a single constructor argument is provided. (The 2nd and 3rd signature could be implemented in one go using a constructor with a default argument.)

A(A const&);
A();
A(int);

template<class Arg0, class Arg1, class...Args>
A(Arg0&&, Arg1&&, Args&&...);
struct A
{
    std::array<int, 3> a {};

    A(A const&) = default;

    A(int a0 = 0)
        : a{a0}
    {
    }

    template<class Arg0, class Arg1, class...Args>
    A(Arg0&& arg0, Arg1&& arg1, Args&&... args)
        : a{ static_cast<int>(arg0), static_cast<int>(arg1), static_cast<int>(args)...}
    {
    }
};
fabian
  • 80,457
  • 12
  • 86
  • 114
2

Refer to item 26 of "Effective modern C++" by Scott Meyers: Avoid overloading on universal references. When the compiler does overload resolution, the constructor based on universal reference (also called forwarding reference) will be the function that will be preferred by the compiler. Also, the constructor based on the universal reference being variadic in nature is not relevant to this discussion.

As such, A b = a; will lead the compiler to call a constructor that will take a copy of A and it will not call the copy assignment operator. This is because the variable b is being created in this statement. Creating a temporary and then assigning to it is too much work.

Given the presence of the templated constructor, the compiler is obliged to go through the process of overload resolution to find the best possible function. It will instantiate the templated constructor to take a non-const lvalue A&. Then between a constructor taking A& (the instantiated templated constructor) and const A& (the already present copy constructor), it will choose the instantiated constructor because it is simpler (no need to add constness to A&).

The constructor taking the universal reference can be constrained, so that it will be called only if a list of values to be assigned to the underlying array is passed. It can be achieved via enable_if (again, refer to item 27 of the above Scott Meyer's book) or via concepts in C++20. The answer by Dharmesh946 shows a way based on enable_if. A way based on concepts (C++20) is

#include <concepts>

...

    template<typename ... Args>
    requires (std::is_convertible_v<Args, int> && ...) 
    A(Args&& ... args)
        :   a{args ...}
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }

In the above code, a fold expression has been used to check whether each of the arguments in the pack could be converted to an integral type. If the pack has one or more arguments that could all be converted to an int, then this constructor is instantiated and called.

Hari
  • 1,561
  • 4
  • 17
  • 26
2

A possible fix:

#include <array>
#include <iostream>
#include <type_traits>

struct A {
    std::array<int, 3> a;

    template <typename... Args,
              std::enable_if_t<sizeof...(Args) != 1, bool> = true>
    A(Args&&... args) : a{args...} {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }
    template <typename Arg,
              std::enable_if_t<!std::is_same<std::decay_t<Arg>, A>::value,
                               bool> = true>
    A(Arg&& arg) : a{arg} {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }

    A(const A& other) : a{other.a} {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }

    A& operator=(const A& other) {
        a = other.a;
        std::cout << __PRETTY_FUNCTION__ << std::endl;
        return *this;
    }
};

int main() {
    A a(1, 2.f, 3.);
    A b = a;  // now OK
    A c(a); // now OK
}

It can certainly be improved and I'm not sure it is what you want to achieve. Basically first constructor catches only initialisation with 0 or 2+ arguments. The second one is used with only one argument if it's not a A. The third one is the normal copy constructor. live Demo

Oersted
  • 769
  • 16