Vladimir.Batov@wrsa.com.au wrote:

The code is in the Boost Vault called Pimpl.zip

I haven't looked at your library, but one thing you need to do to get buy-in for this library is provide a comparison with the pimpl_ptr library that was proposed and rejected. My perception of the primary reason the library failed is that, despite being a basically sound library, it didn't work in the ways people wanted it to. It would be useful if you could summarize the vital details of your library, along with a comparison to the previous proposal, and other similar libraries, in your announcement message.

Hello, From: <Vladimir.Batov@wrsa.com.au> Subject: [boost] Review Request: Generalization of the Pimpl idiom

I hope I am following the submission procedure correctly. I read the guidelines and I seem to do what is needed. Apologies if I missed something.

The code is in the Boost Vault called Pimpl.zip It compiles with GCC 4.1.2 (I use it on Ubuntu 7.04) and MS Visual Studio 2005. Documentation is at DDJ site: http://www.ddj.com/cpp/205918714 Any questions/suggestions/critique - vladimir.batov@wrsa.com.au

Thanks, Vladimir

First of all, I think that a pimpl library should have a place in boost, but I think that the pimpl idiom do not scale very well and the library should present the advantages as well as the liabilities (see below). Are you thinking in writing the library documentation before the review? Even if there is a tutorial in DDJ, IMHO, you should spend some time on the documentation of the library before the review. Your library is usable in simple cases, I would like to see how it can manage more complex cases. If no satisfactory solution to these more complex cases is possible, the library documentation should express it explicitly. Could you clarify if the base implementation pointer can or can not be initialized with a pointer to the implementation of the derived class? There is something wrong in Base::Base(int k) : base(new BaseImpl(k)){} Derived1::Derived1(int k, int l) : Base(new Derived1Impl(k, l)) respect to Base::Base(int k) : base(null()) { reset(new BaseImpl(k)); } Derived1::Derived1(int k, int l) : Base(null<Base>()) { reset(new Derived1Impl(k, l)); } Could a class inherit privately from pimpl? class A : private pimpl<A>::value_semantics {...} There is something that seams extrange to me. If the pimpl is there to mask the implementation, doesn't adds the public inheritance of pimpl<A>::value_semantics some implemtation constraints? Note that public inheritance adds to the class interface the null() function, bool type conversion and ! operator, and adds also all the protected functions to the derevied classes. Can we found always a null object for every class when pimpl is not used? Can a class that inherits from a non pimpl class has its own private impl? class A; class B : A, public pimpl<B>::value_semantics {...} How the B implementation has access to A? maybe we need something like that class B : public pimpl<B, A>::value_semantics {...} making the implementation of B inherit from A. Could we inherit publicly from A and privatly from pimpl<B, A>::value_semantics? 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 {...}; 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? 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? 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 hope that you will find good solutions to these problems. In any case, I will apreciate that you include in the library documentation the limitations of the approach, if there are any. Regards _____________________ Vicente Juan Botet Escriba