Boost logo

Boost :

Subject: Re: [boost] Review Request: impl_ptr (pimpl)
From: Vladimir Batov (Vladimir.Batov_at_[hidden])
Date: 2017-08-22 00:34:27


On 08/21/2017 10:38 PM, Andrzej Krzemienski via Boost wrote:
> 2017-08-15 2:51 GMT+02:00 Vladimir Batov via Boost
> <boost_at_[hidden]>:
>> After much procrastination I would like to request a formal review
>> for the pimpl library...
> I find it disturbing that the first example describes using
> shared_ptr-s and defensive null checks. Boost has always been
> promoting the STL-like value-semantic style of programming, and this
> usage of shared_ptr's looks counter to the trend. When I am defining
> type Book -- unless I have to hide implementation, I will just put the
> members directly in order to achieve value semantics, in particular, I
> want to guarantee the following property: Book b1 {"title", "author"};
> Book b2 = b1; Book b3 = b1; modify(b1); assert (b2 == b3); assert (b2
> != b1); The shared_ptr does no seem to guarantee the last assertion.
> Also, there are these additional members testing if pimpl is null,
> which appear suspicious to me. How can it be null? Do I have to
> implement the default constructor now? Does this also mean than in my
> types that I want to pimpl-ize I cannot use operator bool for my own
> business-logic-related purpose, because it is already taken by the
> implementation of boost::pimpl? There may be good use cases for
> flyweight-like implementation, but it should not necessarily be the
> first example we see in the docs.

Andrzej,

To be honest I am somewhat surprised... in part by the fact that you
find the first example no less than "disturbing". I do hear your view
and I find it is a perfectly valid view. Still, I am sure you've been
around the bush for long enough to know that there are as many
views/opinions/preferences as there are people. Then different projects
have different requirements. So, there should not be anything
"disturbing" if you see someone doing something his way rather than
yours to satisfy the requirements of their project rather than yours.
Don't you agree?

Then you go on to explain that the shared-properties pimpl does not suit
your requirements and to describe your potential confusion (and even
frustration) with such pimpls... Where all that is coming from? No one
is forcing you to use the shared-properties pimpl, right?
Exclusive-ownership pimpls are equal-rights' citizens in the
implementation. They are equally presented in the documentation. What is
there to be "disturbed" by? Indeed, in the documentation I do
mention/describe the shared-properties pimpl first... because it is
easier to introduce the Pimpl concept using std::shared_ptr. Different
people have different skill/knowledge levels, different expectations,
different priorities, different reading styles. There is no way to
satisfy them all. Please do not be "disturbed" if you do not see things
done exactly you'd do that.

> Also, there
> are these additional members testing if pimpl is null, which appear
> suspicious to me. How can it be null?

std::optional has relational operators and can be "null", invalid.
std::shared_ptr() has relational operators and can be "null", invalid.
unique_ptr has relational operators and can be "null", invalid. The
class I proposed -- impl_ptr -- has relational operators and can be
"null", invalid. What is so suspicious about it? The name itself --
impl_ptr -- highlights the fact that it is a smart pointer, a
handle/body construct. So, the handle can be associated with a body...
or the handle might not have a body, i.e. "invalid", "null".

> Do I have to implement the default constructor now?

No, you do not. Your frustration seems palpable. I fail to understand
how possibly I could cause that.

> Does this also mean than in my types that I want to pimpl-ize
> I cannot use operator bool for my own business-logic-related purpose,
> because it is already taken by the implementation of boost::pimpl?

No, it does not mean anything like you describe... and it not "taken".
It is provided. If you want your "own business-logic", you implement it
and you use it. There is nothing stopping you from doing that, right?
Unless I missed, mis-understood something.


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk