14

I tested the following code using Visual Studio 2017 version 15.3.1.

v.push_back(std::move(str1)) works as expected. It moves the contents of str1 into the vector.

str2 is a constant string. Since a constant string cannot be modified after it is created, I was expecting that the v.push_back(std::move(str2)) statement would result in a compiler warning. However, to my surprise there was no compiler warning. After stepped into it, I found that the overload of push_back(const T&) was actually called. The std::move in std::move(str2) seems has no effect.

My question: Should a compiler warning be given for trying to move a constant object?

// Compiled with Visual Studio 2017 version 15.3.1
std::vector<std::string> v;

std::string str1 = "string 1";
v.push_back(std::move(str1));
// Call push_back(T&&). The contents of str1 is moved into the vector.
// This is less expensive, but str1 is now valid but unspecified.

const std::string str2 = "string 2";
v.push_back(std::move(str2));
// Call push_back(const T&). A copy of str2 is added into the vector.
// str2 itself is unchanged.
Garland
  • 911
  • 7
  • 22
  • Closely related: [How to make sure an object will really be moved from?](https://stackoverflow.com/q/35792881/673852) – Ruslan Aug 22 '17 at 06:15

2 Answers2

21

No. Remember, std::move doesn't move anything, it is a glorified cast through remove_reference_t. Since in your case it is casted to const rvalue reference (as std::remove_reference_t<const T> is const T), it doesn't bind to rvalue reference overload push_back(T&& ), and instead binds to const lvalue reference one - push_back(const T& ).

SergeyA
  • 61,605
  • 5
  • 78
  • 137
  • I think there is still an argument that the compiler should warn in this case. Because the programmer (incorrectly) assumes that his object will be moved and may be surprised to find out that it is copied instead. – 0x5453 Aug 21 '17 at 19:17
  • 3
    @0x5453, the compiler has no insight into developers brain, for better or worse. Nothing in the code as written gives compiler any indication that something fishy is going on. – SergeyA Aug 21 '17 at 19:19
  • @SergeyA how would one write the code to get a warning or an error? –  Aug 21 '17 at 19:28
  • 7
    @SergeyA Is there ever a case where calling std::move on a const object is correct from a performance perspective? A similar case that comes to mind for me is how clang warns on pessimizing moves (i.e. `return std::move(foo);`) because they prevent copy elision. In both cases I think the end result will be the same, but the path taken may not be the most efficient. I think that "this code may not be doing what it looks like it is doing" is a *perfect* use case for compiler warnings. – 0x5453 Aug 21 '17 at 19:36
  • @SergeyA Calling std::move on a const object was done by accident in our production code. The code here was created to confirm that no warning was given. – Garland Aug 21 '17 at 23:18
  • 2
    You're free to call `move()` on almost any T -- but whether it *actually* gets moved or copied or frobnicated is completely up to whatever you then pass it to. Calls to the copy constructor are actually fairly common, if you apply it naively. Essentially, `move` is always treated as a suggestion, not a requirement. Remember, a move is a possible optimisation of a copy, it is not a replacement for passing by const-reference. – Miral Aug 22 '17 at 04:55
5

There is an easy way to prevent silent move-from-const: just delete it.

#include <type_traits>
#include <string>

//Copy this line into your global header
template<class T> void move (const T& arg) = delete; //ADL

int main()
{
    {
        std::string str;
        std::string str2 = move(str);
    }

    {
        //error
        //const std::string str;
        //std::string str2 = move(str);
    }
}
TheWisp
  • 306
  • 1
  • 6
  • 7
    Considering your ADL comment, each use of `move` would now need a `using std::move;` in order to be consistent. Otherwise, you'll have to change the call between `move()` and `std::move()` depending on what you're moving, which kind of defeats the purpose. Sure standard types play nicely, but standard types are certainly not the only types you'll be moving. – chris Aug 21 '17 at 20:21
  • 5
    I also do not like the idea of changing semantics of standard functions. Someone might spend a good deal of time trying to understand why move doesn't work the way standard prescribes. – SergeyA Aug 21 '17 at 21:13