Re: [boost] Review Request: Generalization of the Pimpl idiom

From: Vladimir.Batov_at_[hidden]
Date: 2008-04-01 17:47:42

> >>     Base::Base(int k) : base(new BaseImpl(k)){}
> >>     Derived1::Derived1(int k, int l) : Base(new Derived1Impl(k, l))
> >
> > No, this initialization method is not allowed. One needs to use 
reset()
> > instead as you show below. I considered the syntax above and rejected 
it
> > on the grounds that it does not add new functionality (as reset() 
already
> > does that) and it is not generic enough as it won't work for "Derived2 
:
> > public Derived1"
> 
> Why this syntax could not work? Could you elaborate more?

For that syntax to work with a hierarchy every class needs to explicitly 
declare a constructor taking "implementation*". That is, in the following 
hierarchy

Base : pimpl<Base> {}
Derived1 : public Base {}
Derived2 : public Derived {}

pimpl, Base and Derived1 need to have those mentioned constructors so that 
the following could work

Derived2::Derived2(int) : Derived1(new Derived2Implementation())
{
}

That requirement to have 

Derived1::Derived1(implementation*)

is unreasonable as it is clearly not related to to the Derived1 *public* 
interface. On the other hand reset()-based approach works fine and scales 
fine. 

...

> > I cannot think of any particular implementation constraints
> 
> Imagine that after using the your pimpl class which will do an extra
> allocation, and due to performance I need to come back to a traditional
> class. If my class inherits publicaly from pimpl I will need to provide 
the
> same interface as when using the pimpl class.
> 
> Before
> class A : public pimpl<A>::value_semantic ...
> 
> After
> class A {
>     public:
>     // needs to implement publicaly the null function, bool_type 
operator,
>     // and the ! operator.
>     protected:
>     // needs to implement all the protected part inherithed from 
> pimpl<A>::value_semantic
> 
> }

I am not sure what that is supposed to demonstrate. You use infrastructure 
to help you to do your job. If you decide that the infrastructuere does 
not help you and discard that, you'll obviously have to re-implement 
whatever functionality from that infrastructure you managed to deploy. If 
one uses std::list and then for whatever reason goes ahead with his own 
list implementation or merely swaps it for std::vector, he'll likely to 
face the same issue. Trade-offs of the pimpl idiom are well-known -- 
another indirection and memory allocation.  In the end, one can deal with 
memory-allocation by writing a custom allocator for 
pimpl<Foo>::implementation.

> Do you really think that the pimpl idiom is helping me to hide my
> implementation. If I want to hide my implementation I need to restrict 
to
> private inheritance.

I cannot say about you but I do really think that the pimpl idiom is 
helping me to hide the implementation. Private inheritance restricts 
access but not hides the implementation. 

struct Base { int k; };
class Derived : Base {}

Here Base::k is not accessible in Derived but not hidden. More, if that 
stuff is a commercial library, I can easily circumvent that restriction by 
modifying Derived declaration. Pimpl really hides the implementation. See 
for more about the idiom

1. Guru of the Week #24. http://www.gotw.ca/gotw/024.htm
2. Herb Sutter. Exceptional C++ (Addison-Wesley, 1999)
3. J. Carolan. Constructing bullet-proof classes. In Proceedings C++ at 
Work'89 (SIGS Publications, 1989)
4. James O. Coplien. Advanced C++ Programming Styles and Idioms 
(Addison-Wesley, 1992)
5. Eric Gamma et al. Design Patterns (Addison-Wesley,1995) 

...

> Once we remove the public inheritance from pimpl, it is not eassy to
> implement all these extra functions and operators.

Then do not remove public inheritance. I am far from being convinced that 
insistence on private inheritance is justified.

...

> >> class A;
> >>
> >> class B : A, public pimpl<B>::value_semantics  {...}
> >>
> >> How the B implementation has access to A?
> >
> > I am not sure it is a good (from pimpl perspective) design. Pimpl is 
about
> > separation of interface and implementation and implementation hiding. 
By
> > inheriting from A we just threw all that out the window. So, doing 
that
> > for B implementation does not seem to have much value. Having said 
that,
> > if one indeed needs access to A inside B implementation, then keeping 
B*
> > inside B implementation seems to solve the problem. With regard to
> > handling polymorphic hierachies the Bridge description from GoF would 
be a
> > good start. That'll be fully applicable to the proposed pimpl.
> 
> Keeping a A* inside B implementation  will complicate a bit the B 
> implementation, isn't it?

It depends. Even if it does, it'll be truly just a bit.

> >> maybe we need something like that
> >>
> >> class B : public pimpl<B, A>::value_semantics {...}
> >>
> >> making the implementation of B inherit from A.
> >
> > "making the implementation of B inherit from A" is
> >
> > struct pimpl<B>::implementation : public A
> > {
> > }
> >
> > That certainly makes sense (to me anyway).
> 
> How do you ensure this way that B  inherits from A?

It feels like you want two separate hierarchies -- for interface and 
implementation. The application of the Pimpl idiom to polymorphic class 
hierarchies is well described in the "Bridge" section of GoF. 

> >> Could we inherit publicly from A and privatly from pimpl<B,
> >> A>::value_semantics?
> >
> > Cannot comment on this one as I am not convinced we need that (see 
above).
> Imagine A3ppClass is a 3pp class ( we can not modify it), and before 
using 
> the
> your pimpl class we had:
> 
> class MyClass : public A3ppClass {};
> 
> How can we use pimpl without changing the interface of MyClass, neither 
> adding nor removing any function?
> 
> May be we can resume: do not use the pimpl library is you can not master 
the
> complete inheritance hierarchy.

I am not sure how relevant it is to the technical side of things. We are 
not in a legal profession where for legal reasons they write "may contain 
traces of nuts" on a packet of nuts. I see infrastructure as providing a 
service. Decision about applicability of that service for a particular 
situation is application developer's responsibility. I dot thin the time 
of the infrastructure developer will be well spent trying to think of 
every situation where his code *cannot* be used. 

> >> Can we manage with multiple inheritance?
> >> class A : public pimpl<A>::value_semantics {...};
> >> class B : public pimpl<B>::value_semantics {...};
> >> class C : public A, public B {...};
> >
> > Sure... with all the relevant multiple-inheritance perks/drawbacks.
> 
> Imagine that I have up to now
> class A;
> class B;
> class C : public A, public B {};
> 
> and that I have no problems with multiple-inheritance because I have no
> common class on the inheritance hierarchy (even if I don't like it very 
> much).
> Now I'd like to use your pimpl classes and now a found a common class, 
or
> atleast some common functions or operators. Does it means that again I 
need 
> to
> use private inheritance.

Yes, I understand what you describe and 'no' I do not think it'll be an 
issue. And, no, again, in my view private inheritance does not help you 
one bit. (Just admit you like "her", don't you. :-) )

> 
> class A : private pimpl<A>::value_semantics {...};
> class B : private pimpl<B>::value_semantics {...};
> class C : public A, public B {...};
> 
> May be we can resume: do not use the pimpl library if you need multiple 
> inheritance.

I cannot see anything drastically wrong with the code above even though I 
probably would not do it that way. So, I'll have to disagree with your 
conclusion.

> >> How pimpl interacts with STL containers?
> >>
> >> stl::list<A> al;
> >> B b;
> >> al.push_back(b);
> >> // ...somewhere else
> >> A a =al.pop_back();
> >>
> >> How 'a' should behaves? like a A or like a B?
> >
> > I presume
> >
> > struct A : pimpl<A>::... {}
> > struct B : public A {}
> >
> > Then the result depends if A is of value_semantics or 
pointer_semantics.
> > With pointer_semantics it behaves as, say, boost::shared_ptr would 
behave.
> > Otherwise, during copying B part will be chopped off. So, "A a" will
> > behave as A.
> 
> OK, I have missed that. Does it means that only pimpl<A>::value_semantic
> is sustitable for a class which was already used with STL containers?

No, it does not. I am overwhelmingly using pimpl::pointer_semantics and 
those are happily in various containers. And those objects behave 
polymorphically because they have pointer semantics. If it is not the 
desired behavior, then value_semantics should be used instead. It is up to 
the class designer to decide.

> >> The STL container user that
> >> shouldn't knowns nothing about A implementation, expect that it will
> > behaves
> >> as a A, but it think that 'a' will behaves like a B. How can we 
protect
> > from
> >> this situation?
> >
> > I probably do not understand something as I still do not see what it 
is to
> > protect from. A pimpl-based class behaves as any ordinary class (as it 
is
> > in fact an ordinary class).
> >
> 
> May be we can resume: do not use the pimpl pointer semantics if your 
class 
> has value semantics.

Well, I probably can agree with this conclusion... although I am temped to 
argue that in that case we'll have to put similar "warnings" everywhere. 
Like "do not use knife when you actually need a spoon", etc.

> 
> In order to understand better your purpose:
> * Could you give an example in which the object has value semantics and 
the
> implementation pointer semantics?
> * Could you give an example in which the object has pointer semantics 
and
> the implementation value semantics?

Private implementation does not have any particula semantics. It is the 
interface (pimpl-derived) class who has pointer_ or value_semantics... 
meaning how it treats the implementation -- aloows to share it or copies 
it.

> 
> >> Could we store a non polymorph pimpl object in shared memory?
> >> Could we store non polymorph pimpl objects in a container in shared
> > memory?
> >
> > I am not convinced these special cases require anything from the 
pimpl.
> > Although these certainly need more effort from the developer. Issues 
and
> > means to deal with those seem to be the same as if one tried to store 
a
> > pointer or std::auto_ptr or a boost::shared_ptr in shared memory. 
Given
> > it's all in the developer's hands (implementation can be allocated
> > explicitly and then associated via reset()) it'll be the developer's 
pain.
> > This seems to be merely part of overall painful experience of dealing 
with
> > shared memory. :-)
> 
> You are right, auto_ptr and shared_ptr do not work on shared_memory. The 

> difference is that as you state a pimpl is not a pointer, and so we 
should 
> not suffer from the pointer liabilities. When a user use std::auto_ptr 
or 
> boost::shared_ptr he knows that there are some pointers not too far. In 
the 
> case of the pimpl the user has nothing to be with the pimpl classes and 
its 
> pointers.

I have to disagee as I find that view over-simplified and in fact 
incorrect. Pimpl or no pimpl in general no objects beyond value-semantics 
PODs can be safely (or without additional effort) put into shared memory. 

> Imagine that  you are using a class A in shared memory and even a 
container 
> of A in shared memory and you want now to use your pimpl class to hide 
the 
> implementation of A. What I can do?

Shared memory is a delicate area. It's like embedded programming where not 
all language resources are readily available. If your A class can live in 
shared memory, then it is essentially a POD. Say, consisting of a few 
integers, no pointers, no inheritance. Otherwise, two processes cannot 
access/manipulate that same object in shared memory (not easily anyway). 
Still, those processes must have access synchronization issues resolved. 
More so, you mention a container in shared memory. Is it a standard 
container? Then, you'll have to provide custom allocators. The issue just 
keeps more and more complicated. Do you actually have that setting or we 
are talking hypothetically? If the former, then pimpl will not be your 
primary concern. :-) If it is the latter, then I find it too ephemeral to 
discuss effectively. 

If you do not have several processes accessing the same data and, 
therefore, do not care for inter-process data-access synchronization, 
inter-process vtbl pointer correction, etc., then I'd be wondering why you 
use shared memory in the first place. 

> Do not forget that you have two pointer member?
> 
> template<class Interface>
> template<class Implementation>
> class pimpl<Interface>::impl_ptr
> {
> ...
>     traits const* traits_;
>     Implementation*    p_;
> };
> 
> May be you need to add some policies to your class in order to manage 
with 
> this (see for example void_pointer in Boost.Intrusive)?
> 
> May be we can resume: pimpl library can not be used for classes that can 
be 
> stored in shared memory (atleast in his current state).

Well, again I am not sure we should rush with conclusions based purely on 
hypothetical assumptions without trying. I admit I did not try using pimpl 
with shared memory as I do not use shared memory these days. It's almost 
not portable, limiting, hard to handle, and most (all?) of the time is 
better implemented as a multi-threaded app anyway. 

> 
> >> I hope that you will find good solutions to these problems.
> >
> > So, do you find my answers to your satisfaction?
> >
> 
> Not completly, but I hope that this discusion will serv to construct a 
> better
> pimpl library or atleast to show the limits where this library is not 
really
> adequated.

Well, I certainly biased :-) but I am trying to keep an open mind and so 
far I do not see anything that I'd consider as a limitation. Call me a 
stubborn nut if you like.

...

> The question is, can we subtitute a class A by a class A : public
> pimpl<A>::xxx_semantics? I think that it will be better to restrict the 
null
> object and the bool type operator to the pointer semantics.
> 
> What do you think?

I consider value- or pointer-semantics to be the behavior when null() is 
clearly a functionality. They are very much orthogonal to each other. 
Foo::null() clearly declares an invalid (unusable) object regardles of its 
semantics. I see value in it. If one does not need that functionality, he 
probably won't use it. As simple as that. I did not hear compellling 
arguments *not" to provide that functionality for value-semantics classes.

Thanks,
Vladimir.