0

I want to define a move constructor on a class that will be instantiated in a std::vector. However, the move constructor seems to interfere with the initialization of the vector.

#include <iostream>
#include <vector>
class cell
{
private:
int m_value;
public:
  void clear()  {m_value = 0;}
  cell(int i = 0): m_value(i) {}
  cell(const cell&& move): m_value(move.m_value) {} //move constructor
  cell& operator= (const cell& copy)
  {
    if (&copy == this) return *this;
    clear();
    m_value = copy.m_value;
    return *this;
  }
  int getValue() const {return m_value;}
};

int main()
{
cell mycell {3}; // initializes correctly
std::vector<cell> myVec {1, 2, 3, 4}; // compile error.
return 0;
}

I have done quite a bit of research but haven't been able to find the solution to this problem. Quite new to C++ programming.

edit: my class will eventually have a lot more than m_value in it, including some non-fundamental types, hence I don't want to use the default copy constructor.

Liam Goodacre
  • 381
  • 1
  • 10
  • 3
    The error says you misses `cell::cell(const cell&)`: https://ideone.com/cKOhC1 Also `cell(const cell&& move)` looks odd, are you sure about the `const`? – mch Aug 08 '18 at 07:01
  • 2
    It should be `cell(cell && other): m_value{::std::move(move.m_value)} ` – user7860670 Aug 08 '18 at 07:03
  • @VTT `std::move` has no effect for fundamental types (`m_value` is an `int`). They are copied anyway. – Henri Menke Aug 08 '18 at 07:04
  • 1
    @HenriMenke It also has no effect on non-fundamental types that don't implement move constructor either, but it is a good idea to use it anyway to state the move attempt. – user7860670 Aug 08 '18 at 07:07

2 Answers2

6

The problem is that in std::vector constructor with parametr of type std::initializer_list elements are copy-initialized, therefore, copy constructor is requried for your cell class (see, e.g., Why copy constructor is called in std::vector's initializer list? for some relevant discussion). Otherwise, you would be fine with move constructor:

std::vector<cell> myVec;
myVec.push_back(1);
...
Daniel Langr
  • 22,196
  • 3
  • 50
  • 93
  • Would it work if I didn't initialize the vector at all, but instead copied the values in with a loop? – Liam Goodacre Aug 08 '18 at 13:09
  • @LiamGoodacre First, you always need to initialize vector by calling any of its constructors. Second, it depends, since if you copy the elements within the loop, then you still need copy constructor for them. If you only move them (or construct them in place - that is to _emplace_ them), then you won't need copy constructor. – Daniel Langr Aug 08 '18 at 13:26
2
cell(const cell&& move): m_value(move.m_value) {}

Is wrong in several ways. First, the const means that it applies to a move from const objects. This is rarely what you want.

edit: The const move-constructor does not overcome the constness of the std::initializer_list objects, since the vector constructor never moves elements from the initializer_list regardless of the avialability of a const move constructor.

Second, it should have a noexcept specifier, otherwise the vector will prefer not to use the move constructor when it increases capacity. To have it work, you should replace the code with:

cell(cell&& move) noexcept: m_value(move.m_value) {}

Third, it is better to use the default, when possible:

cell(cell&& move) noexcept = default;
Michael Veksler
  • 8,217
  • 1
  • 20
  • 33
  • In this particular case, there is no problem with `const`. `cell(const cell&& move)` is a valid move consturctor and `const` says it does not change the argument, which is true. However, I agree that the defaulted version is much better here. – Daniel Langr Aug 08 '18 at 07:28
  • Of course `const cell&&` is valid, but it is a bad smell. It allows things like `const cell x; cell y(std::move(x));`. This is OK in this specific case, but it may cause trouble if/when somebody will add actual resources to `cell`. – Michael Veksler Aug 08 '18 at 07:35