Should I write constructors using rvalues for std::string?

Question

I have a simple class:

class X
{
    std::string S;
    X (const std::string& s) : S(s) { }
};

I've read a bit about rvalues lately, and I've been wondering, if I should write constructor for X using rvalue, so I would be able do detect temporary objects of std::string type?

I think it should look something like:

X (std::string&& s) : S(s) { }

As to my knowledge, implementation of std::string in compilers supporting C++11 should use it's move constructor when available.

Answer 1

X (std::string&& s) : S(s) { }

That is not a constructor taking an rvalue, but a constructor taking an rvalue-reference. You should not take rvalue-references in this case. Rather pass by value and then move into the member:

X (std::string s) : S(std::move(s)) { }

The rule of thumb is that if you need to copy, do it in the interface.

Answer 2

In the interest of clarifying: The pass-by-value answers are not wrong. But neither is your first guess of adding a string&& overload, except for one detail:

Add:

X (std::string&& s) : S(std::move(s)) { }

I.e. you still need the move because although s has a declared type of rvalue reference to string, the expression s used to initialize S is an lvalue expression of type string.

Indeed, the solution you first proposed (with the move added) is slightly faster than the pass-by-value solution. But both are correct. The pass-by-value solution calls string's move constructor an extra time when passing lvalue and xvalue arguments to X's constructor.

In the case of lvalue arguments, a copy is made anyway, and string's copy constructor is likely to be much more expensive than string's move constructor (except for strings that fit within the short string buffer, in which case move and copy are approximately the same speed).

In the case of xvalue arguments (an xvalue is an lvalue that has been passed to std::move), the pass-by-value solution requires two move constructions instead of one. So it is twice as expensive as the pass by rvalue reference solution. But still very fast.

The point of this post is to make clear: pass-by-value is an acceptable solution. But it is not the only acceptable solution. Overloading with pass-by-rvalue-ref is faster but has the disadvantage that the number of overloads required scales as 2^N as the number of arguments N grows.

Answer 3

No, you should not. What you should do is replace your current constructor with one like this:

X (std::string s) :S(std::move(s)) {}

Now you can handle both l-values, which will be copied to the parameter then moved into your class' string, and r-values, which will be moved twice (your compiler hopefully can optimize this extra work away).

For the most part(there are exceptions which I will not go into here), you shouldn't be writing functions which take r-value references, except for the move constructors of the classes you write. Any time you need your own copy of a value, and this doesn't apply to just constructors, you should take it in by value, and move it where it needs to go. You let the class' own move constructor decide whether the value should be copied or moved based on whether it recieves an r-value, or an l-value. After-all, r-value references were introduced to make our lives easier, not harder.

Answer 4

Since you need a copy of the argument, take the parameter by value. Then, move it into your member data. It is the std::string constructor which is responsible for detecting whether the argument given is an rvalue or an lvalue, not you.

class X
{
    std::string s_;
    X(std::string s) : s_(std::move(s)) {}
};