How to Use Unique_Ptr for Pimpl

How do I use unique_ptr for pimpl?

I believe that your test_help.cpp actually sees the ~Help() destructor that you declared default. In that destructor, the compiler tries to generate the unique_ptr destructor, too, but it needs the Impl declaration for that.

So if you move the destructor definition to the Help.cpp, this problem should be gone.

-- EDIT --
You can define the destructor to be default in the cpp file, too:

Help::~Help() = default;

Unique_ptr usage for pimpl - doesn't compile even though destructor is declared

Any constructor definition (including the implicitly defined default constructor) potentially invokes the destructor of the types of all member objects of class type. The rationale for this is that if constructing a later member throws, the destructor of all previous members would need to be called. In the case of the last member, this is not strictly required, but the standard does not make this exception.

For example, if your class had been:

struct complete_type_with_throwing_constructor {
    complete_type_with_throwing_constructor() { throw 0; }
};

struct Wrapper
{
    class Impl;
    ~Wrapper();
    std::unique_ptr<Impl> _impl;
    complete_type_with_throwing_constructor x;
};

Then the implicit default constructor of Wrapper would construct _impl then destroy it after the default constructor of x throws.

This is the reason why your code works if you explicitly declare your default constructor. This moves the definition to a point where the destructor of std::unique_ptr<Impl> can be instantiated.

---

As for your second question, you are right that the code should still not work when adding the definition afterwards. It's just that compilers do not detect it, so they don't have an error.

Does PIMPL idiom actually work using std::unique_ptr?

I don't (yet) fully understand the issue, but the cause is the default member initializer of the m_ptr member. It compiles wihout errors if you use the member initializer list instead:

// A.hpp:
class A
{
public:
  A();
  ~A();
private:
  class B;
  std::unique_ptr<B> m_b; // no initializer here
};

// A.cpp:
A::A() : m_b(nullptr)     // initializer here
{

}

https://wandbox.org/permlink/R6SXqov0nl7okAW0

Note that clangs error message is better at pointing at the line that causes the error:

In file included from prog.cc:1:
In file included from ./A.hpp:3:
In file included from /opt/wandbox/clang-13.0.0/include/c++/v1/memory:682:
In file included from /opt/wandbox/clang-13.0.0/include/c++/v1/__memory/shared_ptr.h:25:
/opt/wandbox/clang-13.0.0/include/c++/v1/__memory/unique_ptr.h:53:19: error: invalid application of 'sizeof' to an incomplete type 'A::B'
    static_assert(sizeof(_Tp) > 0,
                  ^~~~~~~~~~~
/opt/wandbox/clang-13.0.0/include/c++/v1/__memory/unique_ptr.h:318:7: note: in instantiation of member function 'std::default_delete<A::B>::operator()' requested here
      __ptr_.second()(__tmp);
      ^
/opt/wandbox/clang-13.0.0/include/c++/v1/__memory/unique_ptr.h:272:19: note: in instantiation of member function 'std::unique_ptr<A::B>::reset' requested here
  ~unique_ptr() { reset(); }
                  ^
./A.hpp:12:28: note: in instantiation of member function 'std::unique_ptr<A::B>::~unique_ptr' requested here
  std::unique_ptr<B> m_b = nullptr;
                           ^
./A.hpp:11:9: note: forward declaration of 'A::B'
  class B;
        ^
1 error generated.

Pimpl with unique_ptr : Why do I have to move definition of constructor of interface to .cpp ?

The constructor needs to destroy the class members, in the case that it exits by exception.

I don't think that making the constructor noexcept would help, though maybe it should.

C++ Pimpl Idiom Incomplete Type using std::unique_ptr

You can't use defaulted constructors and assignment operators (such as SomeInt( SomeInt&& other ) = default;) declared in header file with Pimpl classes, because the default implementations are inline, and at the point of declaration SomeInt's declaration SomeInt::impl is incomplete, so unique_ptr complains. You have to declare and define out of line (that is, in implementation file) all special member functions yourself.

That is, change SomeInt and SomeComposite declarations as follows:

// SomeInt.h
SomeInt( SomeInt&& other ); // move
SomeInt& operator=( SomeInt&& other ); // move assign

// SomeInt.cpp
// after definition of SomeInt::impl
SomeInt::SomeInt( SomeInt&& other ) = default;
SomeInt& operator=( SomeInt&& other ) = default;

Another option is to create your own Pimpl pointer, as suggested in this answer.

Is pimpl idiom better than using always unique_ptr as member variables?

After a while I have a broader understanding of the problem and finally I can answer to my own question.

It turned out that what I was saying was not completely correct.

In fact in the code below only Bp class is pImpl. If we change Ap to be pImpl aswell we obtain that, if we change Bp.h we need to recompile only Ap.cpp, Bp.cpp, which is the same of the corresponding solution with unique_ptrs.

Said that, I think I can say that the solution with pImpl seems in general better than the solution with unique_ptrs (we just have to pImpl the correct classes!).

For this reason we decided to switch to pImpl idiom as default for our classes.

---

Related Topics