-1

Wrote the following code in vs13: 

std::vector<std::string> myvector(1000);
std::fill(myvector.begin(), myvector.end(), "Hello World");
std::vector<std::string> pushto;
for (auto s: myvector)
    pushto.push_back(std::move(*s));

Works but didn't move, it called string copy ctor instead. myvector still had his "Hello World"s at the end. Using regular c++ 98 iteration like this:

 std::vector<std::string> myvector(1000);
 std::fill(myvector.begin(), myvector.end(), "Hello World");
 std::vector<std::string> pushto;
 for (auto s = myvector.begin(); s != myvector.end();s++)
    pushto.push_back(std::move(*s));

Actually worked, and move was called. myvector strings were empty. Why the first more modern notation doesnt work?

orenshochat
  • 5
  • 1
  • 4
    Does the first example really compile? I don't think there should be the `*` in `std::move(*s)`. – aschepler Jul 23 '16 at 22:20
  • Note that noone ever said that a move should erase the original. After moving, the original is in a *valid but unspecified* state. Even if you put the `&` in place, it's perfectly valid (and, imho, sometimes desired) for the original object to have the same value(s) as the new one (i.e., same as before till modified). – lorro Jul 23 '16 at 22:29

2 Answers2

5

In your first example you are using for (auto s: myvector), s in this case is a copy of the value in the current iteration. to accomplish what you want, you should do it by reference - for (auto& s: myvector).

Note that the string is not guaranteed to be emptied after std::move, this call just casts its argument to a rvalue reference (&&). other functions (such as std::vector::push_back) which have an overload for rvalue referece arguments might free their argumnet resources.

Xiobiq
  • 400
  • 2
  • 7
  • 16
  • 2
    .. and even then, you're not guaranteed to have the old string emptied. – lorro Jul 23 '16 at 22:22
  • Indeed, but this is the right way to use `std::move` (and it will probably be emptied in the OP's compiler since it got emptied in the second example) – Xiobiq Jul 23 '16 at 22:23
  • Right way: definitely, I agree with you. Excepting emptying: that's implementation detail of the class *and* of the compiler (both, not either). – lorro Jul 23 '16 at 22:40
  • @orenshochat your welcome :) If this answer helped you, you should mark it as accepted (using the green tick v) – Xiobiq Jul 24 '16 at 22:11
3

As @Polikdir said, the copying comes from the copy you made at (auto s: myvector). One way is to use the range-for loop, with && (forwarding reference) or & (normal reference):

for (auto & val : myvector)
    pushto.push_back(std::move(val));

Not well known, but there's a dedicated algorithm for moving objects between containers. It's actually also called std::move.

std::move(s.begin(), s.end(), std::back_inserter(pushto));

edit:

q: Since std::move just casts to rvalue reference is it really needed? Isn't std::move in this case just redundant? No, because a variable (such as val) can not be an r-value reference. That's why we need to call std::forward<T> on universal references.

Also note: What does `auto && e` do in range-based for-loops?

Community
  • 1
  • 1
Johan Lundberg
  • 26,184
  • 12
  • 71
  • 97