std::auto_ptr : strange warning with forward declared class

Hi all,
I am exploring VC++ 8. I have a code which compiles well under VC++ 7.1,
but VC++ 8 warns me:

class Pimpl;

class Test
{
private:
std::auto_ptr<Pimpl> pimpl_;
};

(...)\vc\include\memory(604) : warning C4150:
deletion of pointer to incomplete type 'Pimpl'; no destructor called
see declaration of 'Pimpl'
(...)\vc\include\memory(603) :
while compiling class template member function
'std::auto_ptr<_Ty>::~auto_ptr(void)'
with
[
_Ty=Pimpl
]
see reference to class template instantiation 'std::auto_ptr<_Ty>' being
compiled
with
[
_Ty=Pimpl
]


Pimpl.h is included in Test.cpp, and it compiles well. But I get warned
when I include Test.h from other files. I don't understand why.
If I print something in ~Pimpl(), I can check that it is correctly
destroyed.
Any idea ?

Thanks.

--
Aurélien Regat-Barrel

Aurelien Regat-Barrel wrote:
> Hi all,
> I am exploring VC++ 8. I have a code which compiles well under VC++ 7.1,
> but VC++ 8 warns me:
>
>     class Pimpl;
>
>     class Test
>     {
>     private:
>         std::auto_ptr<Pimpl> pimpl_;
>     };
>
> (...)\vc\include\memory(604) : warning C4150:
> deletion of pointer to incomplete type 'Pimpl'; no destructor called
> see declaration of 'Pimpl'
> (...)\vc\include\memory(603) :
> while compiling class template member function
> 'std::auto_ptr<_Ty>::~auto_ptr(void)'
> with
> [
>     _Ty=Pimpl
> ]
> see reference to class template instantiation 'std::auto_ptr<_Ty>' being
> compiled
> with
> [
>     _Ty=Pimpl
> ]
>
>
> Pimpl.h is included in Test.cpp, and it compiles well. But I get warned
> when I include Test.h from other files. I don't understand why.
> If I print something in ~Pimpl(), I can check that it is correctly
> destroyed.
> Any idea ?

The warning is correct. The problem is this:

~auto_ptr<Pimpl> will attempt to delete your Pimpl object. Because Test
doesn't have a user defined destructor, ~auto_ptr<Pimpl> will be
implicitly instantiated in any translation unit that creates a Test
object. However, in every translation unit except for Test.cpp's, Pimpl
is incomplete, and therefore deleting a pointer to it has undefined
behaviour (unless the destructor of Pimpl is "trivial").

Furthermore, the C++ standard explicitly forbids using std::auto_ptr (or
any other standard library template) with an incomplete type:
In particular, the effects are undefined in the following cases:
[snip]
— if an incomplete type (3.9) is used as a template argument when
instantiating a template component.

You have a number of solutions to workaround the first problem:
1: Explicitly instantiate auto_ptr<Pimpl>::~auto_ptr,
auto_ptr<Pimpl>::reset, and any other operation that can lead to a
delete, at the bottom of Test.cpp, where Pimpl is complete.
2: Explicitly declare a destructor for Test. The (empty) definition will
implicitly instantiate auto_ptr<Pimpl>::~auto_ptr, and this is the only
place where it will be implicitly instantiated.

However, this doesn't get around the general illegality of using an
incomplete type. The solution is to use a class that is appropriate for
use as a pimpl pointer, such as boost's shared_ptr (see www.boost.org).
It uses a special trick to ensure that delete on the held object is only
instantiated at places where you call the shared_ptr constructor with an
object pointer.

Tom

Tom Widmer [VC++ MVP] a écrit : 
>
>
> The warning is correct. The problem is this:
>
> ~auto_ptr<Pimpl> will attempt to delete your Pimpl object. Because Test
> doesn't have a user defined destructor, ~auto_ptr<Pimpl> will be
> implicitly instantiated in any translation unit that creates a Test
> object. However, in every translation unit except for Test.cpp's, Pimpl
> is incomplete, and therefore deleting a pointer to it has undefined
> behaviour (unless the destructor of Pimpl is "trivial").

I don't understand why ~auto_ptr<Pimpl> will be implicitly instantiated
in any translation unit, because when a .cpp wants to destroy a Test,
afaik it wants to call ~Test(), and not ~auto_ptr<Pimpl>. In fact I
don't understand why VC++ complains about auto_ptr, I believed it would
look for ~Test(), not for ~auto_ptr<Pimpl>. It should do that only when
it compiles / generates ~Test() (well, that's what I used to believe).

> Furthermore, the C++ standard explicitly forbids using std::auto_ptr (or
> any other standard library template) with an incomplete type:
> In particular, the effects are undefined in the following cases:
> [snip]
> — if an incomplete type (3.9) is used as a template argument when
> instantiating a template component.

Again, I don't understand where is the problem, because I don't use
auto_ptr in the other translation units. It is simply declared, but
never used (private). It is used only in Test.cpp, and in this unit,
Pimpl is complete.

The following code :

class Test;

int main()
{
std::auto_ptr<Test> ptr;
}

generates the same warning, under VC++ 7.1 too. And I perfectly
undestand why. But, in this case:

class Pimpl;

class Test
{
private:
std::auto_ptr<Pimpl> pimpl_;
};

int main()
{
Test t;
}

I don't undestand the problem. And VC++ 7.1 seems to agree with me.

> You have a number of solutions to workaround the first problem:
> 1: Explicitly instantiate auto_ptr<Pimpl>::~auto_ptr,
> auto_ptr<Pimpl>::reset, and any other operation that can lead to a
> delete, at the bottom of Test.cpp, where Pimpl is complete.

But the warning occurs when compiling units which include Test.h. How
modifying Test.cpp can solve it ?

> 2: Explicitly declare a destructor for Test. The (empty) definition will
> implicitly instantiate auto_ptr<Pimpl>::~auto_ptr, and this is the only
> place where it will be implicitly instantiated.

I did it, and it doesn't have any effect.
I wanted to use auto_ptr to save the writing of a dtor. If I write one,
I will simply remove auto_ptr from my class.

Thanks for your help.

--
Aurélien Regat-Barrel

Aurelien Regat-Barrel wrote:
> Tom Widmer [VC++ MVP] a écrit :
> 
>
>
> I don't understand why ~auto_ptr<Pimpl> will be implicitly instantiated
> in any translation unit, because when a .cpp wants to destroy a Test,
> afaik it wants to call ~Test(), and not ~auto_ptr<Pimpl>. In fact I
> don't understand why VC++ complains about auto_ptr, I believed it would
> look for ~Test(), not for ~auto_ptr<Pimpl>. It should do that only when
> it compiles / generates ~Test() (well, that's what I used to believe).

Right, but since you haven't declared/defined ~Test, it is implicitly
defined in *every* translation unit that destroys a Test, and that
implicit definition includes an implicit instantiation of
~auto_ptr<Pimpl> (since an implicit destructor obviously destroys all
member variables).
 
>
> Again, I don't understand where is the problem, because I don't use
> auto_ptr in the other translation units.

But you do instantiate auto_ptr<Pimpl>. The definition of Test (in
Test.h) instantiates it, since member variables have to have complete
object type.

It is simply declared, but
> never used (private). It is used only in Test.cpp, and in this unit,
> Pimpl is complete.
>
> The following code :
>
>     class Test;
>
>     int main()
>     {
>         std::auto_ptr<Test> ptr;
>     }
>
> generates the same warning, under VC++ 7.1 too. And I perfectly
> undestand why.

Good.

But, in this case:
>

#include <memory>

>     class Pimpl;
>
>     class Test
>     {
>     private:
>         std::auto_ptr<Pimpl> pimpl_;
>     };
>
>     int main()
>     {
>         Test t;
>     }
>
> I don't undestand the problem.

Why not?
The implicitly generated Test::~Test implicitly calls
auto_ptr<Pimpl>::~auto_ptr.

> And VC++ 7.1 seems to agree with me.

You you have the warning level turned down? I get warning C4150 under
VC7.1.
 
>
>
> But the warning occurs when compiling units which include Test.h. How
> modifying Test.cpp can solve it ?

The warning might not be eliminated, but the delete of an incomplete
object will be eliminated, due to the explicit instantiation (which
doesn't try to delete an incomplete object) being chosen instead of the
implicit instantiations (which do). Remember, when linking .objs with
matching implicit and explicit template instantiations (in this case of
auto_ptr<Pimpl>::~auto_ptr), they are merged and the explicit
instantiation is the one that is linked. You can investigate this using
a breakpoint in the destructor of Pimpl to see the cases in which the
destructor is correctly called.
 
>
> I did it, and it doesn't have any effect.

It gets rid of the warning under VC7.1, so I don't believe you did it
correctly. Also, after the change, Pimpl::~Pimpl is correctly called
where required.

> I wanted to use auto_ptr to save the writing of a dtor. If I write one,
> I will simply remove auto_ptr from my class.

I'd use boost::shared_ptr instead, but it's up to you. There are
alternatives - google for grin_ptr.

Tom

>> I don't understand why ~auto_ptr<Pimpl> will be implicitly 
>
>
> Right, but since you haven't declared/defined ~Test, it is implicitly
> defined in *every* translation unit that destroys a Test, and that
> implicit definition includes an implicit instantiation of
> ~auto_ptr<Pimpl> (since an implicit destructor obviously destroys all
> member variables).

Ok, I missed the implicit definition principle.

>  But, in this case:
> 
>
> #include <memory>
> 
>
>
> Why not?
> The implicitly generated Test::~Test implicitly calls
> auto_ptr<Pimpl>::~auto_ptr.
> 
>
> You you have the warning level turned down? I get warning C4150 under
> VC7.1.

I did something wrong. I have done a new test from scratch and I now get
the warning.
 
>
>
> The warning might not be eliminated, but the delete of an incomplete
> object will be eliminated, due to the explicit instantiation (which
> doesn't try to delete an incomplete object) being chosen instead of the
> implicit instantiations (which do). Remember, when linking .objs with
> matching implicit and explicit template instantiations (in this case of
> auto_ptr<Pimpl>::~auto_ptr), they are merged and the explicit
> instantiation is the one that is linked. You can investigate this using
> a breakpoint in the destructor of Pimpl to see the cases in which the
> destructor is correctly called.
> 
>
>
> It gets rid of the warning under VC7.1, so I don't believe you did it
> correctly. Also, after the change, Pimpl::~Pimpl is correctly called
> where required.

The destructor alone doesn't rid of the warning. I didn't wrote the
constructor too. So I did, and now all is ok, both under VC++ 7.1 and
VC++ 8.
 
>
>
> I'd use boost::shared_ptr instead, but it's up to you. There are
> alternatives - google for grin_ptr.

I also use boost::shared_ptr in my project, but it is more costly than
auto_ptr, and I don't want a sharing semantics. I googled for grin_ptr,
and it is exactly what I need.
http://www.octopull.demon.co.uk/arglib/TheGrin.html
Thanks a lot!

--
Aurélien Regat-Barrel