Boost logo

Boost :

Subject: Re: [boost] [pimpl] Mini Review
From: Krzysztof Czainski (1czajnik_at_[hidden])
Date: 2011-05-26 17:30:33


2011/5/26 Sergiu Dotenco <sergiu.dotenco_at_[hidden]>

> Am 26.05.2011 21:23, schrieb Artyom Beilis:
> > ----- Original Message ----
> >> From: Sergiu Dotenco <sergiu.dotenco_at_[hidden]>
> >>
> >> Am 26.05.2011 18:28, schrieb Artyom Beilis:
> >>>> From: Sergiu Dotenco <sergiu.dotenco_at_[hidden]>
> >>>>
> >>>> Am 26.05.2011 11:22, schrieb Artyom Beilis:
> >>>>> The only advantage I can see in boost.pimpl over scoped_ptr or
> auto_ptr
> >>>>> is that it does not require explicit destructor.
> >>>>>
> >>>>> i.e.
> >>>>>
> >>>>>
> >>>>> class Foo : boost::noncopyable {
> >>>>> public:
> >>>>> Foo();
> >>>>> int x() const;
> >>>>> void x(int v);
> >>>>> private:
> >>>>> struct data;
> >>>>> std::auto_ptr<data> d;
> >>>>> };
> >>>>>
> >>>>> No good. You need to add
> >>>>>
> >>>>> ~Foo();
> >>>>>
> >>>>> So it would know to destroy Foo::Data correctly.
> >>>>
> >>>> You can't use std::auto_ptr for pimpl implementation in portable
> code.
> >>>> Doing so is undefined behavior. To quote ISO/IEC 14882:2003
> (§17.4.3.6,
> >>>> p. 329):
> >>>>
> >>>> "In particular, the effects are undefined in the following cases:
> >>>> [...]
> >>>> — if an incomplete type (3.9) is used as a template argument when
> >>>> instantiating a template component, unless specifically allowed
> >>>> for that component."
> >>>>
> >>>> which applies to most standard library types.
> >>>
> >>>
> >>> When you actually implement Foo::~Foo() {} in cpp file
> >>> the type of the data object is complete so the destructor
> >>> is installed correctly.
> >>
> >> That's not correct. std::auto_ptr<data> is an instantiation which uses
> >> an incomplete type as template argument. Providing an non-inline
> >> destructor doesn't magically make the data struct used in your example
> a
> >> complete type.
> >>
> >
> > The quote you provide is too general about the library requirements.
>
> That doesn't make the quoted requirement less valid.
>
> > Particularly, looking into auto_ptr. (20.4.5.1 C++/2003 standard)
> >
> > ~auto_ptr() throw();
> > Requires: The expression delete get() is well formed.
> > Effects: delete get().
> >
> > The expression delete get() is well formed when ~auto_ptr() is called
> > in the destructor of Foo::~Foo() defined in cpp file.
> >
> > So it fills auto_ptr requirements.
>
> I don't see how is that relevant.
>
> The non-inline destructor you referred to previously is needed for
> correct object deallocation (ISO/IEC 14882:2003, §5.3.5, p. 83, par. 5).
> Obviously, templates are not instantiated in the destructor, are they?
> The std::auto_ptr<> template used in your example is instantiated right
> below the struct data; line at which point data is still an incomplete
> type. Hence the effects are undefined and the code is not portable.
>
> > No problems I can see also I'm not aware of any compiler that
> > does not handle this correctly (so far Intel, GCC, MSVC, SunStudio, HP's
> ACC,
> > DECXX)
>
> Well, that's not the point. Consider what happens if a library vendor
> decides to perform a (fully legitimate) check, as in
>
> template<class T> class auto_ptr
> {
> // ...
> static_assert(__is_complete_type(T), "incomplete type used");
> };
>
> If this happens, your pimpl version will no longer work.

Looks like auto_ptr really isn't suitable for pimpl. And auto_ptr has many
more draw-backs, so it is deprecated.

But getting back to the mini-review, suppose we use scoped_ptr instead of
auto_ptr. I think Artyoms arguments still stand. And then you can't forget
to make a non-inline destructor, because scoped_ptr uses checked_delete.

Regards
Kris


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