Does PIMPL idiom actually work using std::unique_ptr?

Question

I've been trying to implement the PIMPL idiom by using a unique_ptr. I inspired myself from several articles that always highlight the same important point : ONLY DECLARE the destructor in the header of the class implementing PIMPL and then define it in your .cpp file. Otherwise, you'll get compilation error like "Incomplete type bla bla".

Alright, I did it on a little test which respects this, but I still have the "incomplete type" error. The code is just below, it's very short.

A.hpp:

#pragma once
#include <memory>

class A
{
public:
  A();
  ~A();
private:
  class B;
  std::unique_ptr<B> m_b = nullptr;
};

A.cpp:

#include "A.hpp"

class A::B
{

};

A::A()
{

}

A::~A() // could be also '= default'
{

}

main.cpp:

#include "A.hpp"

int main()
{
  A a1;

  return 0;
}

I built in 2 (quick and dirty) ways and the result is very surprising from my point of view.

First I built without linking A.cpp

g++ -c A.cpp

No error so far.

Then, I compiled A.cpp and main.cpp to create an executable

g++ A.cpp main.cpp -o test

That's where I am into troubles. Here I got the famous error about the incomplete type:

In file included from /usr/include/c++/9/memory:80,
                 from A.hpp:2,
                 from test.cpp:2:
/usr/include/c++/9/bits/unique_ptr.h: In instantiation of ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = A::B]’:
/usr/include/c++/9/bits/unique_ptr.h:292:17:   required from ‘std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = A::B; _Dp = std::default_delete<A::B>]’
A.hpp:11:28:   required from here
/usr/include/c++/9/bits/unique_ptr.h:79:16: error: invalid application of ‘sizeof’ to incomplete type ‘A::B’
   79 |  static_assert(sizeof(_Tp)>0,
      |                ^~~~~~~~~~~

I know what the constraints are when you intend to use unique_ptr as a part the PIMPL idiom and I tried to care about them. However, in this case, I have to admit that I'm out of idea (and hairs as it gets me on my nerves).

Do I do something wrong, or are we just not censed to use unique_ptr in such a case ?

Live demo

Delete copy ctor, declare/delete move ctor, same with assignment operator. — Marek R, Mar 08 '22 at 14:36
maybe duplicate: https://stackoverflow.com/questions/9020372/how-do-i-use-unique-ptr-for-pimpl. Its the not-accepted answer that is more complete — 463035818_is_not_an_ai, Mar 08 '22 at 14:38
@MarekR That's a good idea in general, but shouldn't change anything in this case I think. Copy operations are non-existent because of `unique_ptr`. Move operations are also non-existent because of a custom destructor. — HolyBlackCat, Mar 08 '22 at 14:39
This has something to do with `= nullptr` in class body. It starts working if you remove it. — HolyBlackCat, Mar 08 '22 at 14:47
@463035818_is_not_a_number I already checked this topic I did not see any solution. I got the point though — PlayerK, Mar 08 '22 at 14:47
As @HolyBlackCat predicted it, removing all the mentioned stuff does not actually change anything, I gave it a try. And you are right regarding the `nullptr`, removing it makes it compiles with no error. The question is why :/ — PlayerK, Mar 08 '22 at 15:15

463035818_is_not_an_ai · Accepted Answer · 2022-03-08T14:56:33.843

6

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.

edited Mar 08 '22 at 14:56

answered Mar 08 '22 at 14:51

463035818_is_not_an_ai

109,796
11
89
185

Yes it does solve the issue but I don't understand why either... I may mark this as an accepted answer. However, I would be glad if you could update if you found it out :) – PlayerK Mar 08 '22 at 14:59
2

@PlayerK i am still just experimenting, though `std::unique_ptr m_b{nullptr};` works as well. Seems like `std::unique_ptr m_b = nullptr;` for some reason needs the destructor already in A.hpp. Actually I am hoping this is a compiler bug, because during initialization no destructor should be needed – 463035818_is_not_an_ai Mar 08 '22 at 15:03
It's because of when construction happens. If the initializer is in the header then the member is constructed prior to calling the constructor body. Thus it will be destructed on an exception IIRC. The object must be "complete" at the location of destruction. So that means that there can't be a situation where partial destruction can happen. – Mgetz Mar 08 '22 at 15:04
@Mgetz thats some good pointers, but it doesnt explain why `std::unique_ptr m_b{nullptr};` is ok (unless I made a mistake while trying it) – 463035818_is_not_an_ai Mar 08 '22 at 15:07
@Mgetz https://wandbox.org/permlink/AepAJYkbRU4buDoJ – 463035818_is_not_an_ai Mar 08 '22 at 15:07
@463035818_is_not_a_number may be doing `std::unique_ptr m_b= nullptr` implicitely calls something this->reset(nullptr) which calls the destructor of B. That may ensure that previously handled resource (if there was one, that's not the case here) is correctly deallocated/freed – PlayerK Mar 08 '22 at 15:10
@PlayerK I wrote a follow up question, hopefully someone can explain https://stackoverflow.com/questions/71397495/why-does-default-member-initializer-request-instantiation-while-does-not – 463035818_is_not_an_ai Mar 08 '22 at 15:27
2

There is no need to explicitly initialize a `unique_ptr` to `nullptr` to begin with. Just use the `unique_ptr`'s default constructor and let it initialize itself to `nullptr` internally. – Remy Lebeau Mar 08 '22 at 18:17

Does PIMPL idiom actually work using std::unique_ptr?

1 Answers1

Linked