Re: [boost] [pimpl] Mini Review

Subject: Re: [boost] [pimpl] Mini Review
From: Sergiu Dotenco (sergiu.dotenco_at_[hidden])
Date: 2011-05-26 16:31:07

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.