Re: [boost] Pimpl Again?

Subject: Re: [boost] Pimpl Again?
From: Domagoj Saric (dsaritz_at_[hidden])
Date: 2016-06-05 17:12:38

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?

-- 
"What Huxley teaches is that in the age of advanced technology, 
spiritual devastation is more likely to come from an enemy with a 
smiling face than from one whose countenance exudes suspicion and hate."
Neil Postman