CODEWITHSUNDEEP
c++c++11iteratormove-semantics
David
David

Reputation: 28178

Why is my swap method interfering with make_move_iterator?

This one has me thoroughly confused. In the below sample I get the error:

error C2664: 'void std::unique_ptr<_Ty>::swap(std::unique_ptr<_Ty> &&)' : cannot convert parameter 1 from 'const std::unique_ptr<_Ty>' to 'std::unique_ptr<_Ty> &&'

I have no idea whatsoever how it's ending up at my swap function, or why it's a problem. Interestingly if I change the signature of void swap(const one& other) and remove the const to void swap(one& other) everything works. if I change the signature of void swap(const one& other) and remove the const to void swap(one& other) it compiles in VS2010 but is still broken in GCC. If there are no swap overloads there is no problem.

//-----------------------------------------------------------------------------
class one
{
public:
    one(){}
    one(one&& other) : x(std::move(other.x)) {}
    one& operator=(one&& other){ x = std::move(other.x); return *this; }

    void swap(const one& other){ x.swap(other.x); }
    void swap(one&& other){ x.swap(std::move(other.x)); }

private:
    one(const one&);
    one& operator=(const one&);

    std::unique_ptr<int> x;
};

//-----------------------------------------------------------------------------
void swap(one& left, one& right)
{
    left.swap(right);
}

//-----------------------------------------------------------------------------
void swap(one&& left, one& right)
{
    right.swap(std::move(left));
}

//-----------------------------------------------------------------------------
void swap(one& left, one&& right)
{
    left.swap(std::move(right));
}

//-----------------------------------------------------------------------------
class two
{
public:
    two(){}
    two(two&&){}
    two& operator=(two&&){ return *this; }

    operator one(){return one();}

private:
    two(const two&);
    two& operator=(const two&);
};

//-----------------------------------------------------------------------------
int main()
{
    std::vector<two> twos(10);
    std::vector<one> ones(std::make_move_iterator(twos.begin()), std::make_move_iterator(twos.end()));
}

Edit: The non-constness requirement makes sense. Entirely an oversight on my part. Why is it calling swap in the first place though?

(For reference, I'm using VS2010)

Demo of it broken

Demo of it still broken, but this 'fix' works in VS2010

Upvotes: 1

Views: 370

Answers (3)

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361472

It seems pretty much straightforward to me. When you swap two objects, none of them could be read-only. They both should be modifiable, only then swap can be done.

So your swap function must take non-const reference.

Note that std::unique_ptr<T> doesn't have swap function which takes rvalue reference. In fact, it doesn't even make much sense to have one. VS2010 is non-standard in that regard. The Standard (§20.7.1.2) requires just one with this signature:

void swap(std::unique_ptr<T> &) noexcept;

There is no swap for rvalue-reference. I would also suggest you to remove your swap that takes rvalue-reference. It adds unnecessary complexity to your code.

Upvotes: 1

James McNellis
James McNellis

Reputation: 355069

The full error message is as follows (I've introduced some newlines to prevent horizontal scrolling):

error C2664: 'void std::unique_ptr<_Ty>::swap(std::unique_ptr<_Ty> &&)'
    : cannot convert parameter 1
        from 'const std::unique_ptr<_Ty>'
        to 'std::unique_ptr<_Ty> &&'
with
[
    _Ty=int
]
Conversion loses qualifiers

The error occurs on the following line of your code:

void swap(const one& other){ x.swap(other.x); }

other is const-qualified, but std::unique_ptr::swap requires a non-const argument. Thus, the "conversion loses qualifiers" (specifically, the const qualifier).

Upvotes: 2

ipc
ipc

Reputation: 8143

// other can't be const since swap modifies it
void swap(one& other){ x.swap(other.x); }

// Why swapping? (swap on rvalues don't work either since it's pointless.)
//Just move the other
void swap(one&& other){ x = std::move(other.x); }

Upvotes: 2

Related Questions