15

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.

Bartek Banachewicz
  • 38,596
  • 7
  • 91
  • 135
  • 2
    [Relevant](http://stackoverflow.com/questions/7592630/is-pass-by-value-a-reasonable-default-in-c11). – Luc Danton May 31 '12 at 16:29
  • The quoted code is a broken pattern that encompasses the worst of both worlds: it constrains the caller to be required to pass an rvalue to `X()`, but then it fails to forward the rvalueness to the constructor for `S`, which therefore can't move and must copy. So, it imposes a burden on users of the function, while preventing them getting any compensatory benefit in return. – underscore_d Jun 30 '17 at 21:57
  • @underscore_d I think five years later we've reached an agreement in the answers that my code wasn't perfect ;) – Bartek Banachewicz Jul 01 '17 at 09:49
  • @BartekBanachewicz Heh, sure, and Howard already made the same point; I just thought it was worth making here in case any reader doesn't realise the problem and doesn't scroll down. :P – underscore_d Jul 02 '17 at 10:01

4 Answers4

25
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.

David Rodríguez - dribeas
  • 204,818
  • 23
  • 294
  • 489
  • 3
    Why shouldn't he have a constructor receiving an rvalue-reference in this case? – noe May 31 '12 at 15:19
  • 2
    @ncasas: combinatorial explosion? – Kerrek SB May 31 '12 at 15:23
  • 6
    @ncasas: Then you would need to provide both a `X( std::string&& )` and `X( std::string const & )` (for the case where the argument used to construct the object is an *lvalue*). The alternatives are easily managed by the compiler in the *pass-by-value* constructor, the parameter will use the existing constructor choosing from `std::string( std::string const & )` or `std::string( std::string && )` to create the copy. That is, providing only the by-value constructor lets the compiler use the features of the library instead of having you rewrite it. – David Rodríguez - dribeas May 31 '12 at 15:29
  • 3
    ... creating constructors that take *rvalue-references* should be limited to the cases where you *really* need to differentiate when the argument is an *rvalue* or an *lvalue*, but in this case you don't care, you need to copy anyway to the member `std::string`. By moving the copy to the interface of the function you let the compiler choose the best way of copying, and by using `std::move` internally, you ensure that you incur no extra costs. – David Rodríguez - dribeas May 31 '12 at 15:31
  • 1
    "That is not a constructor taking an rvalue, but a constructor taking an rvalue-reference." - I think that the statement the questioner made is way more precise than yours. A function does not "take an rvalue reference" or "take an lvalue reference". It "takes an rvalue" or "takes an lvalue". His constructor can in fact only take rvalues, so his statement was perfectly precise. (And yours wasn't. `X && x = ...; foo(x)` passes an rvalue reference, but `foo` won't take it, because it passes it as an lvalue instead of as an rvalue). Other than this obvious mistake, I like the answer. – Johannes Schaub - litb Jun 15 '12 at 11:33
  • 4
    Although I think you should differentiate this a bit more. Types like std::array that cannot be moved efficiently or just don't yet have a move constructor still need the `T const&` approach to avoid effectively copying it twice in the interface *and* the implementation. – Johannes Schaub - litb Jun 15 '12 at 11:42
22

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.

Howard Hinnant
  • 206,506
  • 52
  • 449
  • 577
14

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.

Benjamin Lindley
  • 101,917
  • 9
  • 204
  • 274
  • I was precisely wondering if rvalue-references had other uses apart from move constructors (and obviously the `move` and `forward` functions provided by the standard library). Do you know some other cases where one should use rvalue-references? – Luc Touraille May 31 '12 at 15:29
  • @LucTouraille: In template functions to allow perfect-forwarding. e.g. `emplace_back`, `make_shared` – Benjamin Lindley May 31 '12 at 15:35
  • Oh right, of course! I evoked `forward` and did not even think about its purpose... – Luc Touraille May 31 '12 at 15:39
  • @BenjaminLindley Why is this better than taking a const lvalue reference and copy constructing `S` from it? – David Jun 01 '12 at 14:20
  • 1
    @Dave: Because if a function that takes a const l-value reference receives an r-value, you need to copy the value into the member, even though you probably want to just move it. My answer is based on the general principles that **1)** Moves are usually cheaper than copies, sometimes by a very significant margin. and **2)** You want to avoid writing multiple functions, as Howard elaborates on in his answer. If you're okay with writing multiple functions (and it gets a lot worse when you have multiple arguments), then go ahead and write both an l-value and an r-value version. -- *cont.* – Benjamin Lindley Jun 01 '12 at 15:03
  • 1
    *cont.* -- But if you only want to write one, then it's best to take any arguments you want to copy by value, then move them. This results in a copy and a move in case an l-value is received, and two moves in case an r-value is received. – Benjamin Lindley Jun 01 '12 at 15:04
5

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)) {}
};
Luc Touraille
  • 79,925
  • 15
  • 92
  • 137