8

Consider these classes:

#include <iostream>
#include <string>

class A
{
    std::string test;
public:
    A (std::string t) : test(std::move(t)) {}
    A (const A & other) { *this = other; }
    A (A && other) { *this = std::move(other); }

    A & operator = (const A & other)
    {
        std::cerr<<"copying A"<<std::endl;
        test = other.test;
        return *this;
    }

    A & operator = (A && other)
    {
        std::cerr<<"move A"<<std::endl;
        test = other.test;
        return *this;
    }
};

class B
{
    A a;
public:   
    B (A && a) : a(std::move(a)) {}
    B (A const & a) : a(a) {}
};

When creating a B, I always have an optimal forward path for A, one move for rvalues or one copy for lvalues.

Is it possible to achieve the same result with one constructor? It's not a big problem in this case, but what about multiple parameters? I would need combinations of every possible occurrence of lvalues and rvalues in the parameter list.

This is not limited to constructors, but also applies to function parameters (e.g. setters).

Note: This question is strictly about class B; class A exists only to visualize how the copy/move calls gets executed.

Toby Speight
  • 27,591
  • 48
  • 66
  • 103
fscan
  • 433
  • 2
  • 9
  • You should read these: http://stackoverflow.com/questions/8472208/under-what-conditions-should-i-be-thinking-about-implementing-a-move-constructor and http://stackoverflow.com/questions/4782757/rule-of-three-becomes-rule-of-five-with-c11 – James Custer May 06 '12 at 16:41
  • @JamesCuster: I just wanted to test how many times the respective constructors/operators get called. – fscan May 06 '12 at 17:18

3 Answers3

9

The "by-value" approach is an option. It is not as optimal as what you have, but only requires one overload:

class B
{
    A a;
public:   
    B (A _a) : a(move(_a)) {}
};

The cost is 1 extra move construction for both lvalues and xvalues, but this is still optimal for prvalues (1 move). An "xvalue" is an lvalue that has been cast to rvalue using std::move.

You could also try a "perfect forwarding" solution:

class B
{
    A a;
public:   
    template <class T,
              class = typename std::enable_if
              <
                 std::is_constructible<A, T>::value
              >::type>
    B (T&& _a) : a(std::forward<T>(_a)) {}
};

This will get you back to the optimal number of copy/move constructions. But you should constrain the template constructor such that it is not overly generic. You might prefer to use is_convertible instead of is_constructible as I've done above. This is also a single constructor solution, but as you add parameters, your constraint gets increasingly complicated.

Note: The reason the constraint is necessary above is because without, clients of B will get the wrong answer when they query std::is_constructible<B, their_type>::value. It will mistakenly answer true without a proper constraint on B.

I would say that none of these solutions is always better than the others. There are engineering tradeoffs to be made here.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
  • regarding by-value: in my test i had an extra move for real rvalues too (vs2010). – fscan May 06 '12 at 16:53
  • Interesting. Thanks for that information. I don't have access to vs2010. – Howard Hinnant May 06 '12 at 16:54
  • Ok, this is nice, but kind of makes the code unreadable if applied to every constructor/setter. Is there a case i would not want this behavior from a constructor/setter. If not, can't the compiler do this by default if i declare by-value? (also automatically convert to xrvalue if lvalue is never again used, like in this constructor) – fscan May 06 '12 at 17:05
  • The three basic solutions have been laid out. They each have advantages and disadvantages. I'm not currently aware of any other solutions that compete with these three. – Howard Hinnant May 06 '12 at 17:20
  • The compiler could do the Right Thing by default if `B` was an aggregate, meaning (among other things) `B::a` was `public` and `B` had no user-declared constructors. Then you could just construct a `B` as `B b{ A() };` which would move construct `B::a` from the temporary, or you could do `B b{ a2 };` which would copy construct `B::a` from the lvalue `a2`. But once you have user-declared constructors you need to say what their parameter types are and be explicit about what you do with those parameters. – Jonathan Wakely May 06 '12 at 17:21
  • thanks for the input. so the only real solution is passing by value and hoping the compiler is good enough to eliminate unneeded moves. – fscan May 06 '12 at 17:42
  • @fscan: Your conclusion is the exact opposite of what I attempted to communicate. I'm tempted to delete my answer if that is the impression I've left. Because "always pass by value" is something I strongly disagree with. Fwiw I also strongly disagree with "never pass by value". What I attempted to communicate is that you have a toolbox with several good tools. And none of them is always the right tool. **You** have to design **your** code. No one here can do that better than you. – Howard Hinnant May 06 '12 at 17:58
  • @HowardHinnant: I'm sorry, what i meant is in the general case where i want a clean interface to some other code that i don't know how it's gonna be used. I think in performance critical sections (also depending on the datatype to move) it would make sense to use one of the optimized solutions. Again, i'm talking about setter type methods/constructors, eg: take a value, maybe operate on it and store it somewhere (or move it somewhere else). – fscan May 06 '12 at 19:17
  • Ok, in that situation (pass in, modify, return by value), if the compiler performs copy/move elision everywhere it can, the 2-overload solution (and the perfect forwarding solution) will cost lvalue(1 copy), xvalue(1 move), prvalue(1 move), and the pass-by-value solution will cost lvalue(1 copy + 1 move), xvalue(2 moves), prvalue(1 move). – Howard Hinnant May 06 '12 at 19:56
  • It's also important to constrain in the perfect forwarding case to avoid this converting constructor to be preferred over some special members (e.g. the copy constructor, if it does take `B const&`). – Luc Danton May 07 '12 at 10:23
  • @HowardHinnant: What about constraining using **`static_assert`** to cause a compile-time error on bad input arguments? `std::static_assert(std::is_convertible ::value, "T should be A.");` This seems to me simpler than the `std::enable_if` technique: `class B { A m_a; public: .... template B(T&& a) : m_a(std::forward(a)) { std::static_assert(std::is_convertible::value, "T should be A."); } }` – Mr.C64 Nov 21 '12 at 18:14
  • @Mr.C64: The use of `static_assert` will not keep `std::is_constructible::value` from mistakenly answering `true`. – Howard Hinnant Nov 21 '12 at 19:38
  • @HowardHinnant: Could you please give some example? – Mr.C64 Nov 21 '12 at 22:55
  • Perhaps Concepts will bring this style into fashion? `template B (T&& _a) requires std::is_constructible_v : a(std::forward(_a)) {}` is just about readable. – Toby Speight Aug 31 '17 at 16:34
  • That is how I tend to think of concepts: This technique with better syntax. – Howard Hinnant Aug 31 '17 at 16:36
2

Use a deduced parameter type for the constructor for B:

template <typename T> explicit B(T && x) : a(std::forward<T>(x) { }

This will work for any argument from which an A object is constructible.

If A has multiple constructors with a varying number of arguments, you can just make the whole thing variadic by adding ... everywhere.

As @Howard says, though, you should add a constraint so that the class doesn't appear to be constructible from arguments from which it really isn't.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
1

If the string in your sample is std::string, simply don't care: the default provided copy and move calls their respective in members. And std::string has copy and move both implemented, so that temporaries are moved, variables are copied.

There is no need to define specific copy and move ctor and assign. You can just leave with the constructor

A::A(string s) :test(std::move(s)) {}

In general a straightforward implementation of copy and move can be the following

class A
{
public:
    A() :p() {}

    A(const A& a) :p(new data(*a.p)) {} //copy
    A(A&& a) :p(a.p) { a.p=0; }         //move

    A& operator=(A a) //note: pass by value
    { clear(); swap(a); return *this; }
    ~A() { clear(); }

    void swap(A& a) { std::swap(p,a.p); }
    void clear() { delete p; p=0; }

private:

    data* p;
};

The operator= takes a value that is internally moved. If it comes from a temporary is moved, if it comes from a variable is copied. The difference between copy and move requires distinct constructors but, if we derive A as

class B: public A
{
...
};

there is no need to override anything, since the default copy-ctor for B calls the copy for A, and the default move for B calls the move for A, and all the default assign operators for B call the only one defined for A (that moves or copy depending what has been forwarded).

Emilio Garavaglia
  • 20,229
  • 2
  • 46
  • 63