2

I am failing to understand the following scenario. It is about using the pimpl idiom based on the std::unique_ptr in a derived class. Given a simple class hierarchy declared as follows:

class Foo
{
public:
  virtual ~Foo(); 
  //...
};

struct X;

class Bar : public Foo
{
public:
  ~Bar();
  //...

private:
  std::unique_ptr<X> _d;
};

I am showing only code relevant for my question.

Imagine class Foo being some interface and the class 'Bar' which implements it. I want to use the pimpl idiom in Bar. Destructors are virtual and defined in the corresponding cpp files. Also the full definition of the struct X, which is only forward declared, is accessible in the cpp, so that for the destructor of Bar destructor std::unique_ptr<X>::~unique_ptr() can be instantiated. When I try to create an instance of Bar, I would expect it to work

int main()
{
  Bar b;
}

Instead, I get the compilation error use of undefined type 'X' followed by the message can't delete an incomplete type (in Visual Studio 2013 Update 2). However, if I explicitly add the default constructor to Bar, main() compiles/builds correctly.

class Foo
{
public:
  virtual ~Foo(); 
};

struct X;

class Bar : public Foo
{
public:
  Bar();
  ~Bar();

private:
  std::unique_ptr<X> _d;
};

I am failing to see the correspondence between the presence of default constructors in this class hierarchy and the completeness of the struct X in the context of the std::unique_ptr<X> in Bar. Could someone explain or point to a possibly already existing explanation?

Marko
  • 21
  • 5
  • 2
    You actually need a destructor for Bar in a file where X is declared, only. –  Jul 06 '15 at 16:11
  • Side note, maybe see this: https://stackoverflow.com/questions/24635255/is-it-possible-to-write-an-agile-pimpl-in-c/24638702#24638702 – Galik Jul 06 '15 at 16:14
  • the "switch" is adding a constructor to `Bar`, adding one to `Foo` doesn't change a thing – WorldSEnder Jul 06 '15 at 16:19
  • I think your `std::unique_ptr` needs to point to your *interface* class (see my previous note). – Galik Jul 06 '15 at 16:24
  • @WorldSEnder Indeed, I have updated the post accordingly. – Marko Jul 07 '15 at 12:45

1 Answers1

6

The problem is that the default constructor added to Bar has to call std::unique_ptr<X>'s destructor in case Bar's constructor throws an exception.

But std::unique_ptr<X>::~unique_ptr() calls the unique_ptr's Deleter: std::default_delete<X> - which requires X to be a complete type. [unique.ptr.single.dtor]

When you declare the constructor yourself, the instantiation doesn't happen because the constructor is only declared and thus linked at a later point in time and the compiler happily eats your code.

WorldSEnder
  • 4,399
  • 1
  • 24
  • 53
  • "But when instantiating std::unique_ptr, the compiler also has to generate std::unique_ptr::~unique_ptr()" -- This is, at least, sloppily worded. An implicit instantiation of a templated class does not lead to an instantiation of all its methods. The problem is really that Bar's destructor is generated for when the scope in which b resides is left. This in turn leads to an instantiation of unique_ptr's destructor. If "Bar b;" was replaced with the intentionally leaky "Bar *b = new Bar();", it would compile. (Don't do that, though.) – Arne Vogel Jul 06 '15 at 17:33
  • @ArneVogel, `Bar *b = new Bar()` does not compile: http://ideone.com/yFbE8V. The real problem is that sizeof(X) is static asserted, then? The method doesn't have to be instantiated. – WorldSEnder Jul 06 '15 at 17:38
  • @ArneVogel I found the real reason why all this happens. See the updated answer. – WorldSEnder Jul 06 '15 at 18:38
  • I noticed this too, but didn't understand it. Now it finally dawns on me (at least a bit): The constructor needs to be able to destroy the members in case an exception occurs during construction. Therefore, it is the implicitly generated c'tor, not the d'tor, of Bar, that requires unique_ptr's d'tor to be instantiated. – Arne Vogel Jul 06 '15 at 18:55
  • 1
    Thank you for the explanation, that makes sense. – Marko Jul 07 '15 at 12:51