Explicitly Asking for Default Move Constructor Does not Work?

Question

Here is my code:

#include <cstdint>
#include <vector>

class Bar {
    uint32_t m_value;
public:
    Bar(const uint32_t value) : m_value(value) {
    }
};

class Foo {
    Bar* m_p_bar;
    uint32_t m_value;
    Foo(const Foo&) = delete;
    Foo& operator=(const Foo&) = delete;
    Foo& operator=(Foo&&) = default;
    Foo(Foo&&) = default;
public:
    /*
    Foo(Foo&& from) {
        m_p_bar = from.m_p_bar;
        m_value = from.m_value;
        from.m_p_bar = nullptr;
    }
    */
    Foo(const uint32_t value) : m_value(value) {
        m_p_bar = new Bar(value);
    }
};

int main() {
    std::vector<Foo> foos;
    foos.emplace_back(8);
}

Compiler complains:

In file included from /opt/rh/devtoolset-9/root/usr/include/c++/9/vector:66,
                 from test_implicit_func.cpp:2:
/opt/rh/devtoolset-9/root/usr/include/c++/9/bits/stl_uninitialized.h: In instantiation of ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::move_iterator<Foo*>; _ForwardIterator = Foo*]’:
/opt/rh/devtoolset-9/root/usr/include/c++/9/bits/stl_uninitialized.h:307:37:   required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = std::move_iterator<Foo*>; _ForwardIterator = Foo*; _Tp = Foo]’
/opt/rh/devtoolset-9/root/usr/include/c++/9/bits/stl_uninitialized.h:329:2:   required from ‘_ForwardIterator std::__uninitialized_move_if_noexcept_a(_InputIterator, _InputIterator, _ForwardIterator, _Allocator&) [with _InputIterator = Foo*; _ForwardIterator = Foo*; _Allocator = std::allocator<Foo>]’
/opt/rh/devtoolset-9/root/usr/include/c++/9/bits/vector.tcc:474:3:   required from ‘void std::vector<_Tp, _Alloc>::_M_realloc_insert(std::vector<_Tp, _Alloc>::iterator, _Args&& ...) [with _Args = {int}; _Tp = Foo; _Alloc = std::allocator<Foo>; std::vector<_Tp, _Alloc>::iterator = __gnu_cxx::__normal_iterator<Foo*, std::vector<Foo> >; typename std::_Vector_base<_Tp, _Alloc>::pointer = Foo*]’
/opt/rh/devtoolset-9/root/usr/include/c++/9/bits/vector.tcc:121:4:   required from ‘std::vector<_Tp, _Alloc>::reference std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {int}; _Tp = Foo; _Alloc = std::allocator<Foo>; std::vector<_Tp, _Alloc>::reference = Foo&]’
test_implicit_func.cpp:34:21:   required from here
/opt/rh/devtoolset-9/root/usr/include/c++/9/bits/stl_uninitialized.h:127:72: error: static assertion failed: result type must be constructible from value type of input range
  127 |       static_assert(is_constructible<_ValueType2, decltype(*__first)>::value,
      |                                                                        ^~~~~

I noticed that for some reason, I need to define my own move constructor for Foo. By explicitly asking the compiler to use the default one (i.e. Foo(Foo&&) = default;), it doesn't work. However, if I ask the compiler to use all the implicit ones (i.e. removing Foo(const Foo&) = delete; Foo& operator=(const Foo&) = delete; Foo& operator=(Foo&&) = default; Foo(Foo&&) = default;), then compiler doesn't complain.

My question is why explicitly asking compiler to use the default move constructor here doesn't work but it works in this case. And this answer suggests I could ask for a default version if it is implicitly removed.

Answer 1

If you let the compiler use the implicitly defined "big 5" you will have problems with leaks and double free:s.

You need to make the move constructor public and you need to delete the pointer in the destructor. You also need to make sure not to leave the pointer in the moved-from object.

#include <utility> // exchange

class Foo {
    Bar* m_p_bar;
    uint32_t m_value;

public:
    Foo(const uint32_t value) : 
        m_p_bar(new Bar(value)),
        m_value(value) 
    {}

    Foo(const Foo&) = delete;
    Foo& operator=(const Foo&) = delete;

    Foo(Foo&& rhs) :
        m_p_bar(std::exchange(rhs.m_p_bar, nullptr)),  // put a nullptr in rhs
        m_value(rhs.m_value)
    {}
    Foo& operator=(Foo&& rhs) {
        std::swap(m_p_bar, rhs.m_p_bar);     // let rhs destroy our current Bar
        m_value = rhs.m_value;
        return *this;
    }

    ~Foo() { delete m_p_bar; }               // destroy it
};

A better idea would be to use a std::unique_ptr<Bar> - or to not use a pointer at all.