Boost logo

Boost :

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.


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