0

Wikipedia claims, in the article on opaque pointers, that

The d-pointer is the only private data member of the class and points to an instance of a struct (which must be a POD since its destructor is not visible)

This is not required in PIMPL and just Wikipedia being typically idiosyncratic isn't it?

I'm taking the lack of d-pointer tag as an answer to my question, but hoping someone might contribute to Wikipedia and/or clarify things. Or just say Wikipedia is awful, last-resort etc :)

The point of my question is, how visible are a nested class's methods when fully declared and defined in the cpp implementation file? Will its destructor get called as expected (the containing class will call delete on it in its destructor)?

_EDIT_ Fixed version, http://en.wikipedia.org/wiki/Opaque_pointer

John
  • 6,433
  • 7
  • 47
  • 82

2 Answers2

4

PIMPL is a disgusting idiom, and I've come to the conclusion that it's more of an anti-pattern.

However, should you insist on using it, you can trivially define the destructor as empty in an implementation file to make full use of any smart pointer you care to use to automatically destruct non-POD implementations.

class Impl;
class Object {
    std::unique_ptr<Impl> impl;
public:
    //...
    ~Object();
};

#include "Impl.h"
// in cpp
Object::~Object() {}

This quick code sample clearly defines Wikipedia as completely wrong- it's more than possible to simply define the destructor in the implementation file to use whatever destructor you want, even smart pointers which require the full definition.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • Some smart pointers require that their template argument is not an incomplete type, so "any smart pointer you care" may not work. – CB Bailey Aug 23 '11 at 12:15
  • @Charles: It will, because the destructor instantiation is deferred. `unique_ptr` requires complete type for the destructor- but the code sample still works. – Puppy Aug 23 '11 at 12:23
  • I'm not using smart pointers at all (but assigning locals to members ASAP so as to safely delete). The memory gets freed if the application crashes so unless I have resources other than memory to free I don't see the need (one less #include). Thank you for your opinion of the PIMPL idiom. What I notice in VC++ is that autocomplete has now stopped working since I went PIMPL, but restarting often fixes that. – John Aug 23 '11 at 12:47
  • @DeadMG: I'm not arguing about `unique_ptr`, I was just saying that some smart pointers require a complete type at the point of instantiation of the smart pointer specialization itself and not just when member functions that use the destructor of the template parameter are instantiated. – CB Bailey Aug 23 '11 at 14:01
  • @Charles: Those functions can have a delayed instantiation too. The destructor trick is generic and doesn't work just on the destructor. – Puppy Aug 23 '11 at 14:38
  • @DeadMG: I think you've misunderstood what I was trying to say. I wasn't talking about the instantiation any functions at all, I was saying that some smart pointers require a complete type where you instantiate the class specialization itself. – CB Bailey Aug 23 '11 at 15:03
  • @Charles Bailey: This bit of code is sufficient to prove Wikipedia wrong. Sure, there may be other smart pointers that cannot be used to disprove WP. But that's not really relevant to the question, is it? – MSalters Aug 23 '11 at 15:16
  • @MSalters: I was just pointing out a minor opportunity for improvement in this answer, re "any smart pointer you care to use". If you thought I was making some more significant point then I apologize for not making my intent clearer. – CB Bailey Aug 23 '11 at 15:24
3

Yup, it's plain wrong. The destructor of the PIMPL struct must be visible at the point where it's called, which is from the definition of the destructor of the class itself. Put both destructors in the same .cpp file, PIMPL dtor first, and visibility is assured.

There is no reason for the PIMPL dtor to be visible at any other point, since it's not called there.

MSalters
  • 173,980
  • 10
  • 155
  • 350
  • Brilliant, makes perfect sense, thank you. Still to single step into it but it is compiling ok (at least the last remaining bugs seem unrelated..). – John Aug 23 '11 at 12:46