Re: [boost] [pimpl] Mini Review

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

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.

> 
> It is undefined if you have inline destructor that does not have
> defined "struct data" but it is defined for non-inline destructor in cpp
> that has fully defined "struct data"
> 
> So there is nothing wrong there neither in reality nor by the standard.

Your code produces undefined behavior and thus is not portable. See above.