Re: [boost] Pimpl Again?

Subject: Re: [boost] Pimpl Again?
From: Vladimir Batov (Vladimir.Batov_at_[hidden])
Date: 2016-06-05 21:23:08

Domagoj,

Apologies for top posting. I started writing a point-by-point reply just 
to realize what it was unlikely to have any impact. The way we view the 
pimpl idiom, its deployment, applicability scope, acceptable design and 
overhead are drastically different. You do not seem to like the proposed 
implementation of "policies", you do not like allocators, you do not 
like pointer/value semantics, you do not like deployment of 
std::shared_ptr. And that's fine.

I had another look at your implementation and indeed it does seem to 
hide the implementation. Apologies. It does not seem to provide or to be 
extendible to support pointer-semantics or polymorphic hierarchies or 
variable-size structures/records which you seem to dismiss but which are 
important to me. Therefore, in order to move forward, one possibility is 
that In addition to the existing managers you write an additional 
policy/manager as you see it with no allocators, shared_ptr, etc. as 
part of the proposed policy-based extendible framework. That way people 
will have a choice of policies which is a good thing IMO as the end 
product will be useful for us all.

Alternatively, you might like to explore the possibility of proposing 
your design, say, on this list. Your design might well be better and 
more successful. IMO any outcome is good as I think it'd be useful to 
have a pimpl entry in Boost.

Does it sound reasonable?

On 06/06/2016 07:12 AM, Domagoj Saric wrote:
> On 1.6.2016. 14:05, Vladimir Batov wrote:
> >> * -1 for 'breaking namespace usage' (forcing implementation in the
> >> global namespace) ...
> >
> > I believe that's been addressed... From the user perspectives anyway.
> > Your mileage may vary.
>
> Could you please point me to the post(s) that address this? AFAICT 
> you've mostly just dismissed the criticisms as subjective (which I 
> described as an extreme position WRT what can be named a matter of 
> style, especially when more conventional 'styles', that do not pollute 
> the global namespace, have been presented)...
>
>
> >> * -1 for forcing heap usage...
> >
> > This is incorrect. The proposed design is policy-based, i.e.
> > configurable/customizeable. Indeed, I personally only implemented
> > heap-based policies that I care about. Care to provide a stack-based
> > policy based on your implementation? That already was one the requests
> > on the thread.
>
> OK, I'm sorry, I somehow missed this possibility. I didn't see it 
> offered and the code seemed a bit convoluted to deduce how 
> configurable it actually is - a major culprit being the fact that 
> you've made the individual policies/'managers' members of the master 
> class template. These should be separate, decoupled classes that can 
> live in separate headers and be included only when desired...
>
>
> >> And no, allocators won't ('fully') do it. You still have extra
> >> indirection and even more code complexity (allocators which use stack
> >> in the current function? ugh...)...
> >
> > I would not be so rush dismissing allocators. There must be reasons why
> > we have them in the Standard.
>
> Whatever the reasons why we have them, the points raised still hold...
>
>
> >> * -1 for the 'pointer/value semantics' 'kludge'
> >
> > Not sure what you mean by that. Anyway, I'll provide some history. As
> > you are aware that pimpl stands for "pointer to implementation". Stress
> > is on "pointer". Kludge or not from the set-go very early
> > implementations of the idiom had pointer-semantics. In fact, they 
> *were*
> > pointers (opaque pointers throughout, say, X Window System). Then, when
> > we had a pimpl-related discussion on the list many moons ago "value
> > semantics" were requested and been provided. Now it's implemented as a
> > manager. If you do not need/like it, you simply do not use it. You
> > provide your own stack-based manager (incidentally with 
> value-semantics)
> > and use it. Does it sound acceptable?
>
> No :)
>
> The opaque pointers were/are used as pointers, not through adapters 
> that made them look like references.
>
> I'm aware of the history and how, once you already had an API used by 
> other code, the 'easiest' way to hide the implementation was to 
> 'store' it in a pointer and forward all the calls (requiring little or 
> no change in 'client' code). However, just because that's the 
> 'historical evolutionary path' or that it maps to 'pimpl', a 'cool 
> tongue-in-cheek name' isn't an argument that that's the way it should 
> be (the 'is-ought' distinction):
>
> * Supporting automatic-storage allocation automatically 'already' 
> doesn't square with the pimpl moniker. I'd rather lose or 'misuse' the 
> established idiom name than efficiency (and simpler user code).
>
> * 'Pointer-semantics'/'shallow-copy' types are..um...'unusual' (like 
> having a pointer with the dot syntax).
>
> * If all of a type's implementation is hidden in a pointer-to-impl it 
> means it is always dynamically allocated so just make it so. Instead of
> class interface
> {
> public:
>   void foo();
> private:
>   impl * pimpl;
> };
>
> just do
>
> class interface
> {
> public:
>   void foo();
>
>   static smart_ptr<interface> create();
>
> protected:
>    interface() = default;
>   ~interface();
> };
>
> // in .cpp
>
> class impl : interface
> { ... };
>
> void interface::foo()
> {
>    static_cast<impl &>( *this )....
> }
>
> ...this is also slightly more efficient as you eliminate one pointer 
> indirection...
>
>
> >> IMO this is completely misplaced, I can't see it serving as anything
> >> else but replacing/wrapping the standard arrow/-> (smart)pointer
> >> syntax with the dot syntax. If the particular class objects are always
> >> to be heap allocated simply use them through a pointer...
> >
> > I am under impression you've disposed of the pivotal pimpl property --
> > implementation hiding. Looking at your implementation seems to confirm
> > that view... I could be wrong. If my understanding and reading of your
> > code are correct, then, yes, indeed with no implementation-hiding goal
> > all that code is "completely misplaced" and "serving as anything else
> > but" and unnecessary. I agree.
>
> Even if my code is convoluted it still a priori cannot be the way you 
> understood it - what would it be for then (a pimpl that doesn't pimpl 
> the impl)? :)
>
> My 'pimpl impl' does hide the implementation, in fact it hides even 
> the pimpl's private/implementation side parts from the 
> interface/client side (that's why it has two headers, pimpl.hpp 
> included on the interface side and impl.hpp included on the 
> implementation side) unlike you implementation that has everything, 
> and all the policies/managers, in one header...
>
>
> >> * -1 for paying for shared_ptr for inherently shared classes of 
> objects
> >> Use boost::intrusive_ptr for those (in fact make the 'actual' factory
> >> function, that is implemented on the .cpp side, private and have it
> >> return a naked pointer, so that it can return it through a register
> >> across the ABI, and then wrap on the interface side in an inline
> >> member that returns a smart_ptr)...
> >
> > You might be right and boost::intrusive_ptr is far superior to
> > std::shared_ptr.
>
> It is different, not necessarily superior.
>
>
> > I personally never liked the intrusiveness of
> > boost::intrusive_ptr.
>
> That's kind of like saying I never liked the sharedness of shared_ptr :)
>
>
> > And I suspect that that requirement of
> > boost::intrusive_ptr won't sit well with at least some users as well.
>
> a) It is/would be an implementation detail?
> b) Why?
>
>
> > As the intrusive_ptr docs state: "As a general rule, if it isn't 
> obvious
> > whether intrusive_ptr better fits your needs than shared_ptr, try a
> > shared_ptr-based design first".
>
> That just follows from the 'managed' ('configurability discourages 
> use') mindset behind the whole smart_ptr library.
>
>
> > And it is not obvious to me at the moment.
>
> If a type is always allocated on the heap and inherently has shared 
> ownership semantics (e.g. any type deriving from 
> enable_shared_from_this) then 'obviously' reference counting has 
> already 'intruded' it 'by definition'...
>
>
> > I believe shared_ptr is as efficient as boost::intrusive_ptr when
> > created with make_shared or allocate_shared (used in the proposal).
>
> Instead of believing (WRT 'things of this world') you may be better 
> served by checking the evidence (e.g. codegen, binary size etc.).
> intrusive_ptr won't force an indirect deleter, RTTI, aditional pointer 
> and a 'control object' 'on you'...
>
>
> > Well, apart from the 2-pointers' foot-print... which IMO is the proper
> > way to go.
>
> Why is that the way to go?
>
>