0

I am writing an application that needs to store objects of a class that uses the PIMPL idiom in a std::vector. Because the class uses std::unique_ptr to store a pointer to it's implementation and std::unique_ptr is not copyable, the class itself is not copyable. std::vector should still work in this case because the class is still movable.

To avoid creating a copy I tried using emplace_back to construct the elements directly into the vector, but for some reason it still complains that it is trying to call the copy constructor!

I've written a simple example to demonstrate the problem.

test.h:

#pragma once

#include <memory>

// Simple test class implemented using the PIMPL (pointer to implementation) idiom
class Test
{
public:
    Test(const int value);
    ~Test();

    void DoSomething();
private:

    // Forward declare implementation struct
    struct Impl;

    // Pointer to the implementation
    std::unique_ptr<Impl> m_impl;
};

test.cpp

#include "test.h"

#include <iostream>

// Implementation struct definition
struct Test::Impl
{
    Impl(const int value)
        : m_value(value)
    {}

    void DoSomething()
    {
        std::cout << "value = " << m_value << std::endl;
    }

private:
    int m_value;
};

// Construct the class and create an instance of the implementation struct
Test::Test(const int value)
    : m_impl(std::make_unique<Impl>(value))
{}

Test::~Test() = default;

// Forward function calls to the implementation struct
void Test::DoSomething()
{
    m_impl->DoSomething();
}

main.cpp:

#include "test.h"

#include <vector>

int main()
{
    std::vector<Test> v;

    // Even though I'm using "emplace_back" it still seems to be invoking the copy constructor!
    v.emplace_back(42);

    return 0;
}

When I try to compile this code I get the following error:

error C2280: 'Test::Test(const Test &)': attempting to reference a deleted function

This leads to two questions...

  1. Why is it attempting to use the copy constructor even though I explicitly used emplace_back?

  2. How can I get this to compile without errors? The object is movable so according to the standard it should be able to be stored in a std::vector.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
tjwrona1992
  • 8,614
  • 8
  • 35
  • 98
  • 2
    `the class is still movable` - is it? does `Test b(std::move(a));` work? – dewaffled Jun 30 '21 at 12:53
  • You should be able to invoke the move constructor within `emplace_back`. Seems like this link might be what you're looking for: https://stackoverflow.com/questions/35404932/stdvectoremplace-back-and-stdmove – Guy Marino Jun 30 '21 at 12:56
  • Consider using `std::shared_ptr` to make your class copiable. – Igor R. Jun 30 '21 at 13:17
  • 1
    You declared a destructor, therefore the compiler won't even attempt to provide a move constructor. It does provide a copy constructor, but that's deprecated. You can `Test(Test&&) = default;` to re-enable the move constructor. Then, you might run into issues that you're using an incomplete type in `main.cpp` :) – dyp Jun 30 '21 at 13:19
  • Why do we run into issues with the incompleteness of `Impl` in main? Well, the move ctor is inline, so its code has to be generated in main. I believe the issue is that the compiler instantiates the destructor of `unique_ptr`. That would be reasonable if we had multiple data members, because on construction failure, we undo construction of already-constructed members. This cannot be the case here but maybe this special case isn't optimized for. – dyp Jun 30 '21 at 13:27
  • @dyp that was it! I had to declare the move constructor as `default`. I got around the "incomplete type" error by defining `Test::Test(Test&&) noexcept = default` in the `test.cpp` file *after* the `Test::Impl` struct was already defined. – tjwrona1992 Jun 30 '21 at 15:28
  • 1
    I swapped [tag:emplace] for [tag:C++]; there is a max of 5 tags, and having C++ there is useful. – Yakk - Adam Nevraumont Jun 30 '21 at 17:53

1 Answers1

3

Add Test(Test&&) noexcept; and Test& operator=(Test&&) noexcept; then =default them in your cpp file.

You should probably make Test(const int value) explicit while you are at it.

In modern gcc/clang at least, you can =default the move ctor in the header. You cannot do this to operator= because it can delete the left hand side pointer, nor the zero argument constructor (something to do with constructing the default deleter?) or destructor (which has to call the default deleter).

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • Do you know why the inline default move ctor cannot deal with incomplete `Impl`? – dyp Jun 30 '21 at 16:42
  • @dyp No. I do know that some C++ standard containers have over-strict requirements on completeness. I don't think of any fundamental reason why a unique ptr move ctor would actually need the definition of the object. – Yakk - Adam Nevraumont Jun 30 '21 at 17:46
  • @dyp So, I tried it and in at least modern gcc/clang [move ctor works](https://godbolt.org/z/fb89Mar1c) with a `=default` and an undefined object. Default ctor and dtor do not work. – Yakk - Adam Nevraumont Jun 30 '21 at 17:50
  • @dyp, as far as I know it has something to do with the move constructor needing to know the size of the object in order to compile. If the object is not a complete type its size is not yet defined. – tjwrona1992 Jun 30 '21 at 18:37
  • @tjwrona1992 A unique ptr to an incomplete type should not need to know the object's size to move the pointer. For assignment, destruction or construction yes. – Yakk - Adam Nevraumont Jun 30 '21 at 20:40
  • 1
    Here, in clang+libc++: https://godbolt.org/z/Mejv4q5nv it looks like we instantiate the dtor from the move ctor. – dyp Jul 01 '21 at 06:43
  • 1
    @dyp Looks like it happens when it checks if the deleter can be copied from the source unique ptr. https://godbolt.org/z/583GzzreK works around it. Don't know what standard says. – Yakk - Adam Nevraumont Jul 01 '21 at 16:55