CODEWITHSUNDEEP
c++gccc++11
user1662078
user1662078

Reputation: 23

GCC C++11 deleting copy assignment for move assignable classes prevents std::sort from compiling?

I'm trying to sort a vector of class that implements move assignment operator. This code works fine in Microsoft and Intel C++. In GCC 4.8.1, the copy constructor is deleted and seems causing problem.

c:\mingw\lib\gcc\mingw32\4.8.1\include\c++\bits\stl_algo.h:2164:11: error: use of deleted function 'constexpr MoveOnly::MoveOnly(const MoveOnly&)'
__val = _GLIBCXX_MOVE(*__i);
        ^
test.cpp:6:11: note: 'constexpr MoveOnly::MoveOnly(const MoveOnly&)' is implicitly declared as deleted because 'MoveOnly' declares a move constructor or move assignment operator

And with help from Matthieu M., this page explained why the copy constructor is deleted.

#include <vector>
#include <algorithm>
#include <iostream>
#include <type_traits>

class MoveOnly {

public:
    int data;
    MoveOnly& operator = (const MoveOnly && rhs) {
        data = rhs.data;
        return *this;
    }
    MoveOnly& operator = (const MoveOnly & rhs) {
        data = rhs.data;
        return *this;
    }
    bool operator < (const MoveOnly& j) const {
        return data<j.data;
    }
};

int main() {

    std::cout<<"Is move_assignable:"<<std::is_move_assignable<MoveOnly>::value<<std::endl;
    std::cout<<"Is copy_assignable:"<<std::is_copy_assignable<MoveOnly>::value<<std::endl;
    std::vector<MoveOnly> vMoveOnly;
    //std::sort(vMoveOnly.begin(), vMoveOnly.end());
    return 0;
}

Upvotes: 2

Views: 1349

Answers (2)

masoud
masoud

Reputation: 56499

Declaring a move constructor or move assignment operator deletes the default copy/move constructors (IMO due to encourage us to obey rule of five!), on the other hand std::sort needs one of move-construct or copy-construct having this code:

    template<typename _RandomAccessIterator>
    void
    __insertion_sort(_RandomAccessIterator __first,
             _RandomAccessIterator __last)
    {
      if (__first == __last)
    return;

    for (_RandomAccessIterator __i = __first + 1; __i != __last; ++__i)
    {
      if (*__i < *__first)
        {
          typename iterator_traits<_RandomAccessIterator>::value_type
        __val = _GLIBCXX_MOVE(*__i);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

To solve the issue:

You need a class which is move-constructable and you can test it by is_move_constructible, std::sort needs first one. Put a move-constructor to make it move-constructable.

You need at least one of move-assignment or copy-assignment for assignment, and at least one of copy-constructor or move-constructor for construction.

While you class name is MoveOnly, I think it's reasonable to pick move-assignment and move-constructor. So, this code is enough to compile:

class MoveOnly {

    ...

    MoveOnly(MoveOnly &&m) : data(m.data) {}

    MoveOnly& operator = (const MoveOnly && rhs) { ... }
};

Upvotes: 5

Davidbrcz
Davidbrcz

Reputation: 2397

I have add a few things to your code and this works fine : 

#include <vector>
#include <algorithm>
#include <iostream>
#include <type_traits>

class MoveOnly {

public:
    int data;
        MoveOnly (int i): 
        data (i)
        {}

    MoveOnly (MoveOnly && rhs): 
        data (std::move(rhs.data))
        {}
    MoveOnly (const MoveOnly & rhs)=delete;

    MoveOnly& operator = (MoveOnly && rhs) {
        data = std::move(rhs.data);
        return *this;
    }
    MoveOnly& operator = (const MoveOnly & rhs) {
        data = rhs.data;
        return *this;
    }
    bool operator < (const MoveOnly& j) const {
        return data<j.data;
    }
};

int main() {

    std::cout<<"Is move_assignable:"<<std::is_move_assignable<MoveOnly>::value<<std::endl;
    std::cout<<"Is copy_assignable:"<<std::is_copy_assignable<MoveOnly>::value<<std::endl;
    std::vector<MoveOnly> vMoveOnly;
    vMoveOnly.push_back(MoveOnly(10));
    vMoveOnly.push_back(MoveOnly(666));
    vMoveOnly.push_back(MoveOnly(-100));
    std::sort(vMoveOnly.begin(), vMoveOnly.end());
    for(std::size_t i=0;i<vMoveOnly.size();++i){std::cout<<vMoveOnly[i].data<<std::endl;}
    return 0;
}

PS : using a const r-value reference is kind of nonsense because the main purpose of rvalue references is to to move objects instead of copying them. And moving the state of an object implies modification. So no const

Upvotes: -3

Related Questions