2

Context: I've a class DLXMatrix with some attribute which are vector of some local class called Header. Each Header holds some pointer on some other Header which refer to elements of the same vector (think of a structure which is both a vector and a doubly linked list). As a consequence, I cannot use the default copy constructor and assignment operators since they would point to the original elements and not the copy. Note: I'm making sure that the vector is never resized or moved in the memory.

#include <type_traits>
#include <vector>

class DLXMatrix {
  private:
    struct Header {
        Header *left, *right;
    };
    std::vector<Header> heads;
  public:
    DLXMatrix() = delete;
    explicit DLXMatrix(size_t nb_col);
    DLXMatrix(const DLXMatrix &);
    DLXMatrix &operator=(DLXMatrix other);
    DLXMatrix(DLXMatrix &&) = default;
    DLXMatrix &operator=(DLXMatrix &&other) = default;
    ~DLXMatrix() = default;
};

static_assert(std::is_move_constructible<DLXMatrix>::value);
static_assert(std::is_move_assignable<DLXMatrix>::value);

If I'm not mistaken, though I defined custom copy and assigment operator, the default destructor, move constructor and move assignment copy should work as expected without leaking. Now, I'd like to use std::swap but it refuses to compile because my class is not move assignable:

dlx_matrix.cpp:257:5: error: static_assert failed due to requirement
      'std::is_move_assignable_v<DLX_backtrack::DLXMatrix>'
    static_assert(std::is_move_assignable_v<DLXMatrix>);

So my questions are:

  • is this a reasonable way of doing things ?
  • why is DLXMatrix not move assignable ?

If it matters I'm compiling with g++ 7.5.0 and clang++ 6.0.0 with standard c++17.

hivert
  • 10,579
  • 3
  • 31
  • 56
  • 5
    Is there a reason as to why the copy assignment operator doesn't take a `const DLXMatrix &`? [Changing it resolves the issue](https://godbolt.org/z/5jTEd6) (I don't have a good explanation why). – IlCapitano Oct 30 '20 at 13:11
  • I noticed the same thing. Looks like something about that is getting in the way. Which is really weird, considering copy and swap is a known idiom. – NathanOliver Oct 30 '20 at 13:11
  • 2
    See e.g. why this messes things up: https://stackoverflow.com/questions/17961719/overload-resolution-between-object-rvalue-reference-const-reference. I don't know how `is_move_assignable` is implemented, but it probably doesn't even see your actual move assignment operator. – AVH Oct 30 '20 at 13:14
  • @IlCapitano : I needed a copy so I figured out that calling the copy constructor during the function call spared one line. The code is working now. Thanks for the catch ! I'll gladly accept it if you write a proper answer ! – hivert Oct 30 '20 at 13:43
  • It looks like the malformed copy constructor hides the move constructor. Results may vary from compiler to compiler. – Michaël Roy Oct 31 '20 at 20:02

1 Answers1

0

I'm posting an answer following the indication of @IlCapitano.

In the implementation of operator= I needed to do a copy and decided to perform it thanks to a call-by-value. So the prototype was

DLXMatrix &operator=(DLXMatrix other);

instead of the standard

DLXMatrix &operator=(const DLXMatrix &other);

This call-by-value form is not recognized as a proper copy assignment operator by the compiler/library.

hivert
  • 10,579
  • 3
  • 31
  • 56