Re: [boost] [pimpl] Mini Review

Subject: Re: [boost] [pimpl] Mini Review
From: Sean Chittenden (sean_at_[hidden])
Date: 2011-05-31 19:15:27

>>> Helping to implement copying and assignment is useful,
>>> provided the class is supposed to support those operations.
>> 
>> shared_ptr-based Pimpls copying and assignment are done by
>> shared_ptr.
> 
> That tells me that Pimpl provides copying and assignment for the derivate.

> As you've noted, this is only by virtue of using private inheritance.  I still don't think it is correct behavior for a library implementing the Pimpl Idiom to modify the interface class' (Book's) interface.  Consider a raw pointer Pimpl design:
> 
> class Book {
>   // state
> };
> 
> becomes:
> 
> class Book {
>   struct impl;
>   impl pimpl_;
> };
> 
> struct Book::impl {
>   // state
> };
> 
> Adding a pimpl did nothing to Book's interface.  Why then should Book's interface chance when using your library?  That seems to be an extension of the idiom into the territory of the Bridge Pattern or something else entirely.

I've been watching this discussion for a while (with vested interest), and want to use a few examples to shore up some of the discussion (and possibly move it off list so that things can proceed in a constructive manner). The concern is not that Pimpl is implementing an opaque pointer, it's the miscellaneous copy constructors, constructors, equality or whatever other misc "extras" that come along from using Pimpl, right?

To recap (using pointer semantics since that's my preferred implementation):

> // book.h

> class Book : public pimpl< Book >::pointer_semantics {
>   // interface
> };
> 
> // book_impl.cc

> template<> struct pimpl< org::example::String >::implementation : boost::noncopyable {
>   // impl state
> }

> Book::Book() : base() { }

> 

> // user.cpp

> Book b;
> std::cout << "sizeof(Book): " << sizeof(b) << std::endl;

From an ABI perspective, Book implemented via Pimpl maintains the same size (currently 16 bytes, regardless of Book::implementation), and thereby provides a stable ABI (if someone really was concerned about future-proofing their frontend ABI, they could pad Book in book.h). I think the concern is coming from:

> Book* b2 = new Book();
> Book* b1 = b2;

because there possibly isn't a clear understanding of how those mechanics would work with Pimpl (yes?). Or is it because it's unclear how you'd obtain the desired copy semantics - for example - with the proposed Pimpl library?

I'm trying to tease out what the actual feedback is or what the possible suggestions could be (vs outright criticism).

> Modifying the interface is the part I was referring to.
> 
> You've said that copying and assignment are only supported (for value semantics, anyway) if the derivate provides them.  That's a good approach.  If everything is of that sort, then all's well, but that's not what I see.

How is Book's interface different?

> // Is the Interface different because of the inheritance?

> class Book : public pimpl< Book >::pointer_semantics { };
> // Or is the interface different because the ABI is different?
> class Book {
>   class* impl_;
> };

Would a Pimpl implementation with preprocessor directives be more suitable because of the level of explicitness?

Also, Pimpl implements an opaque pointer and successfully breaks compilation dependencies. Pimpl is not a Bridge implementation.

> class Book {
>   PIMPL_PUBLIC_INTERFACE(Book, impl);
> };

? I use similar cpp(1) magic to implement NO_COPY_OR_ASSIGN() because I've found it to be more predictable in working (or failing) than inheriting from boost::noncopyable.

With as (in?)efficient compilers sucking up gigs of RAM and frequently crashing during compilation[1], having a friendly mechanism to hide linkage and implementations behind opaque pointers is useful beyond externally exposed APIs in libraries. The tedium of doing this frequently makes the Pimpl library an easy win for many developers (though possibly not all of them and not in all situations). As a Pimpl consumer for over a year, I'm trying to concretely demonstrate where the possible pitfalls could be lurking with this implementation. Thanks in advance. -sc

[1] A few years ago I came to terms with it being a Boost "Right Of Passage" to debug your first compiler crash (that wasn't the result of running out of RAM).

--
Sean Chittenden
sean_at_[hidden]