13

I am using Visual Studio 2012 Update 2 and am having trouble trying to understand why std::vector is trying to use the copy constructor of unique_ptr. I have looked at similar issues and most are related to not having an explicit move constructor and/or operator.

If I change the member variable to a string, I can verify that the move constructor is called; however, trying to use the unique_ptr results in the compilation error:

error C2248: 'std::unique_ptr<_Ty>::unique_ptr' : cannot access private member declared in class 'std::unique_ptr<_Ty>'.

I'm hoping someone can point me to what I am missing, thanks!

#include <vector>
#include <string>
#include <memory>

class MyObject
{
public:
    MyObject() : ptr(std::unique_ptr<int>(new int))
    {
    }

    MyObject(MyObject&& other) : ptr(std::move(other.ptr))
    {
    }

    MyObject& operator=(MyObject&& other)
    {
        ptr = std::move(other.ptr);
        return *this;
    }

private:
    std::unique_ptr<int> ptr;
};

int main(int argc, char* argv[])
{
    std::vector<MyObject> s;
    for (int i = 0; i < 5; ++i)
    {
        MyObject o;
        s.push_back(o);
    }

    return 0;
}
il_guru
  • 8,383
  • 2
  • 42
  • 51
zYzil
  • 208
  • 1
  • 2
  • 6
  • 1
    If you want to construct your objects directly in the vector, you can also just `emplace` them: `for (int i = 0; i < 5; ++i) s.emplace_back();` That should work with VC11 too. –  Apr 16 '13 at 08:51

2 Answers2

15

The push_back() function takes its argument by value. Therefore, an attempt is made to either copy-construct the argument of push_back() (if you are passing an lvalue), or to move-construct it (if you are passing an rvalue).

In this case, o is an lvalue - because named objects are lvalues - and rvalue references cannot bind to lvalues. Therefore, the compiler cannot invoke your move constructor.

In order to have your object moved, you have to write:

s.push_back(std::move(o));
//          ^^^^^^^^^

What surprises me in this case is that it seems VC11 generated a copy-constructor for MyObject implicitly without defining it as deleted (judging from the error you posted). This should not be the case, since your class declares a move constructor. Per Paragraph 12.8/7 of the C++11 Standard, in fact:

If the class definition does not explicitly declare a copy constructor, one is declared implicitly. If the class definition declares a move constructor or move assignment operator, the implicitly declared copy constructor is defined as deleted; otherwise, it is defined as defaulted (8.4)

I must conclude that while the error you are getting is correct - because you are not passing an rvalue to push_back() - VC11 is not fully compliant here.

Andy Prowl
  • 124,023
  • 23
  • 387
  • 451
  • Great! I was not aware of this. I am still a little confused as to why the move constructor gets called just by changing the unique_ptr to a string though. If VC11 is generating an implicit copy constructor, I would expect that to be used since I did not use std::move. – zYzil Apr 15 '13 at 08:36
  • 1
    @zYzil: The correct behavior would be not to invoke the copy constructor nor the move constructor of `std::string` (see [this live example](http://liveworkspace.org/code/3kW04t$377)), since `MyObject` is non-copyable and you are not moving from it. If your code looks *exactly* like my example (including the missing `std::move()`), then VC11 has a bug. – Andy Prowl Apr 15 '13 at 08:40
  • I definitely think there is a bug in VC11. [This example](http://liveworkspace.org/code/1edTJY$3) does not compile on GCC 4.8.0; however, on VC11 it compiles and outputs "Move ctor" due to the move constructor being called. Thanks for all your help! – zYzil Apr 15 '13 at 08:49
  • @zYzil: Sounds like a bug, indeed. Good luck with your project! – Andy Prowl Apr 15 '13 at 08:50
  • 1
    VC11 doesn't yet implement the rules about automatic creation of move constructors and inhibition of automatic creation of copy constructors. I think STL refers to these parts as rvalue reference 3.0, and VC11 only implements 2.1. – Sebastian Redl Apr 15 '13 at 10:40
  • @SebastianRedl: OK, that explains. Thank you for the information – Andy Prowl Apr 15 '13 at 10:41
4

MyObject o; defines o to be an object. Which means it's a l-value. Doing s.push_back(o); then invokes the l-value overload of push_back() (it has no other choice), which tries to create a copy.

Since your class is noncopyable, you have to move the object into the vector:

for (int i = 0; i < 5; ++i)
{
    MyObject o;
    s.push_back(std::move(o));
}
Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455