CODEWITHSUNDEEP
c++
user1544067
user1544067

Reputation: 1764

error in moving std::unique_ptr

I try to run this code to move the unique_ptr's from the first vector to another, in the copy constructor:

class Text
{
    struct paragraph
    {
        int index;
        string text;
    };

    vector<unique_ptr<paragraph>> paragraphs; 

public:

    Text()
    {
        paragraphs.push_back(unique_ptr<paragraph>(new paragraph));
    }

    Text(const Text & t)
    {
        for(int i = 0; i < (int)t.paragraphs.size(); i++)
        {
            paragraphs.push_back(move(t.paragraphs[i]));
        }
    }
};

and i got this error:

1>c:\program files\microsoft visual studio 10.0\vc\include\xmemory(208): error C2248: 'std::unique_ptr<_Ty>::unique_ptr' : cannot access private member declared in class 'std::unique_ptr<_Ty>'
1>          with
1>          [
1>              _Ty=Text::paragraph
1>          ]
// Etc.

Upvotes: 0

Views: 751

Answers (2)

Jonathan Wakely
Jonathan Wakely

Reputation: 171433

Text(const Text & t)
{
    for(int i = 0; i < (int)t.paragraphs.size(); i++)
    {
        paragraphs.push_back(move(t.paragraphs[i]));
    }
}

In this constructor t is const, so t.paragraphs[i] gives a const lvalue reference to a unique_ptr.

move(t.paragraphs[i]) turns that into a const rvalue reference, but it's still const. The unique_ptr move constructor needs a non-const rvalue reference, so isn't viable, so the deleted copy constructor is the best match. you shouldn't be trying to move the contents of t in a copy constructor, that's what move constructors are for.

You should also say std::move not just move, to prevent ADL finding the wrong move.

Upvotes: 0

Nicol Bolas
Nicol Bolas

Reputation: 474336

There are several things wrong with your code.

First, you cannot move from a const&, and this applies to members of a const& too. Movement is destructive; you should only move from a &&. Your copy constructor should copy; if you don't want a copy constructor, then = delete it or whatever your compiler allows.

Second, assuming you took a Test && for a proper move constructor, you should not move each element like that. Instead, move the vector into the new one like this:

Text(Text && t) : paragraphs(std::move(t.paragraphs)) {}

Third, you should only write this function assuming that your compiler of choice doesn't support automatically generated move constructors (ie: is Visual Studio). If it does support it, you shouldn't write one at all. Let the compiler do it's job.

Upvotes: 5

Related Questions