Design question related to std::auto_ptr

[abhay.burli]

Hello Group,

Consider the following class design using forward declarations.

//start-snip
#include<memory>

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;
};

int main() {
MyConcrete obj;
return 0;
}
// end-snip

comeau gives the following warning(s) :
// statrt-snip
Comeau C/C++ 4.3.10.1 (Oct 6 2008 11:28:09) for
ONLINE_EVALUATION_BETA2
Copyright 1988-2008 Comeau Computing. All rights reserved.
MODE:strict errors C++ C++0x_extensions

"memory", line 76: warning: delete of pointer to incomplete class
~auto_ptr() { delete _M_ptr; }
^
detected during:
instantiation of
"std::auto_ptr<_Tp>::~auto_ptr() [with
_Tp=Details]" at
line 11 of "ComeauTest.c"
implicit generation of "MyConcrete::MyConcrete()" at line
11 of
"ComeauTest.c"
In strict mode, with -tused, Compile succeeded
// end-snip

As we see, std::auto_ptr requires complete definition of the class to
be used as a smart-pointer unlike if we compose using only a class-
pointer(Details*).
I have a lot of such similar forward declarations (recommended
physical design). Using std::auto_ptr would have made writing
exception-safe operator= (...) easier on these classes so...

Q-1. Does the above warning lead to only memory leaks or something
more ?
Q-2. Do boost smart-pointers also have the same semantics w.r.t to the
above situation?
Q-3. Any suggestions on redesign? Something like using PIMPL, and
implement the above concrete class in a .cpp file where i can #include
the composed class definition though;it is quite a lot of redesign!

Thanks in advance,
Abhay

 

[Vladimir Jovic]

Hello Group,

Consider the following class design using forward declarations.

//start-snip
#include<memory>

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;
};

int main() {
MyConcrete obj;
return 0;
}
// end-snip

comeau gives the following warning(s) :
// statrt-snip
Comeau C/C++ 4.3.10.1 (Oct 6 2008 11:28:09) for
ONLINE_EVALUATION_BETA2
Copyright 1988-2008 Comeau Computing. All rights reserved.
MODE:strict errors C++ C++0x_extensions

"memory", line 76: warning: delete of pointer to incomplete class
~auto_ptr() { delete _M_ptr; }
^
detected during:
instantiation of
"std::auto_ptr<_Tp>::~auto_ptr() [with
_Tp=Details]" at
line 11 of "ComeauTest.c"
implicit generation of "MyConcrete::MyConcrete()" at line
11 of
"ComeauTest.c"
In strict mode, with -tused, Compile succeeded
// end-snip

As we see, std::auto_ptr requires complete definition of the class to
be used as a smart-pointer unlike if we compose using only a class-
pointer(Details*).
I have a lot of such similar forward declarations (recommended
physical design). Using std::auto_ptr would have made writing
exception-safe operator= (...) easier on these classes so...

Q-1. Does the above warning lead to only memory leaks or something
more ?

This is the output from g++ 4.1.2:

g++ fr.cpp
/usr/lib/gcc/i386-redhat-linux/4.3.0/../../../../include/c++/4.3.0/backward/auto_ptr.h:
In destructor ‘std::auto_ptr<_Tp>::~auto_ptr() [with _Tp = Details]’:
fr.cpp:6: instantiated from here
/usr/lib/gcc/i386-redhat-linux/4.3.0/../../../../include/c++/4.3.0/backward/auto_ptr.h:173:
note: neither the destructor nor the class-specific operator delete will
be called, even if they are declared when the class is defined.

I would take it serious, and include the header of the class Details in
the cpp file.

Q-2. Do boost smart-pointers also have the same semantics w.r.t to the
above situation?

Off course

Q-3. Any suggestions on redesign? Something like using PIMPL, and
implement the above concrete class in a .cpp file where i can #include
the composed class definition though;it is quite a lot of redesign!

In the header forward declare, but include the headers (or forward
declared classes) in the source files

 

[Alf P. Steinbach]

* Vladimir Jovic:

Hello Group,

Consider the following class design using forward declarations.

//start-snip
#include<memory>

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;
};

int main() {
MyConcrete obj;
return 0;
}
// end-snip

[snip]

This is the output from g++ 4.1.2:

g++ fr.cpp
/usr/lib/gcc/i386-redhat-linux/4.3.0/../../../../include/c++/4.3.0/backward/auto_ptr.h:
In destructor ‘std::auto_ptr<_Tp>::~auto_ptr() [with _Tp = Details]’:
fr.cpp:6: instantiated from here
/usr/lib/gcc/i386-redhat-linux/4.3.0/../../../../include/c++/4.3.0/backward/auto_ptr.h:173:
note: neither the destructor nor the class-specific operator delete will
be called, even if they are declared when the class is defined.

I would take it serious, and include the header of the class Details in
the cpp file.

Q-2. Do boost smart-pointers also have the same semantics w.r.t to the
above situation?

Off course

Happily that's incorrect.

std::auto_ptr's referent type is required to be complete, but that is not a
requirement of e.g. boost::shared_ptr.

And in particular, boost::shared_ptr provides custom deleter functionality that
probably can handle the OP's case with ease (weasel-words present because the
example is lacking in detail).

In the header forward declare, but include the headers (or forward
declared classes) in the source files

Generally good advice.

Sometimes the "problem" of incomplete type is only present due to premature
optimization of build times.


Cheers & hth.,

- Alf

 

[Vladimir Jovic]

* Vladimir Jovic:

[more sniping]

From this example, to me it looks like the OP wanted to implement the pimpl

Happily that's incorrect.

std::auto_ptr's referent type is required to be complete, but that is
not a requirement of e.g. boost::shared_ptr.

And in particular, boost::shared_ptr provides custom deleter
functionality that probably can handle the OP's case with ease
(weasel-words present because the example is lacking in detail).

I am bit confused. Don't you need the class declaration to use that
class? (assuming the class is defined somewhere )
Do you have an example where you would only forward declare a class, and
then do something useful with it?

 

[Alf P. Steinbach]

* Vladimir Jovic:

* Vladimir Jovic:

[more sniping]

From this example, to me it looks like the OP wanted to implement the
pimpl

Happily that's incorrect.

std::auto_ptr's referent type is required to be complete, but that is
not a requirement of e.g. boost::shared_ptr.

And in particular, boost::shared_ptr provides custom deleter
functionality that probably can handle the OP's case with ease
(weasel-words present because the example is lacking in detail).

I am bit confused. Don't you need the class declaration to use that
class? (assuming the class is defined somewhere )
Do you have an example where you would only forward declare a class, and
then do something useful with it?

Well, the PIMPL idiom is a/the prime example of taking advantage of this
property -- or lack of requirement -- of boost::shared_ptr.

For the PIMPL idiom it's not even necessary to explicitly use a custom deleter
because with PIMPL you can easily arrange to have a complete referent type at
the time of initialization of the shared_ptr. And shared_ptr then just does The
Right Thing. It generates a deleter function that it calls on destruction.

I.e., in the implementation file it goes like this:

class Foo::Impl {}; // Complete definition of Impl class.

Foo::Foo()
: myPImpl( new Impl ) // shared_ptr has all info it needs here.
{}

Some folks have argued that the same technique must in practice have to work
nicely also for std::auto_ptr, at least if also Foo::~Foo() is defined in the
implementation file, because how could any compiler manage to screw up things
then? But according to the standard's general rules it's just UB to do it with
std::auto_ptr. And one risks the compiler warning about that UB, as above.

Cheers & hth.,

- Alf

 

[abhay.burli]

Well, the PIMPL idiom is a/the prime example of taking advantage of this
property  --  or lack of requirement  --  of boost::shared_ptr.

[snip]
I.e., in the implementation file it goes like this:

   class Foo::Impl {};        // Complete definition of Impl class.

   Foo::Foo()
       : myPImpl( new Impl )  // shared_ptr has all info it needs here.
   {}

Thanks, this is exactly what i was thinking of.

Some folks have argued that the same technique must in practice have to work
nicely also for std::auto_ptr, at least if also Foo::~Foo() is defined in the
implementation file, because how could any compiler manage to screw up things
then? But according to the standard's general rules it's just UB to do it with
std::auto_ptr. And one risks the compiler warning about that UB, as above..

Can you kindly explain why this is the case? I thought the original
example produces a warning because the compiler tries to generate an
implicit dtor which may be orthogonal for polymorphic types like
virtual ~Dtor() {}.

Regards,
Abhay

 

[James Kanze]

* Vladimir Jovic:

[...]

Well, the PIMPL idiom is a/the prime example of taking
advantage of this property -- or lack of requirement -- of
boost::shared_ptr.

Except that you probably don't want the semantics of
boost::shared_ptr with compilation firewall idiom. Most of the
time, you want the semantics of boost::scoped_ptr, but I think
it also requires a complete object.

When all is said and done, using a smart pointer for the
compilation firewall idiom seems a bit of overkill; adding a lot
of excess machinery for no really good reason.

For the PIMPL idiom it's not even necessary to explicitly use
a custom deleter because with PIMPL you can easily arrange to
have a complete referent type at the time of initialization of
the shared_ptr.

In fact, you've have to take extreme pains NOT to have a
complete type; normally, you'll have just new'ed it (and new
requires a complete type).

 

[Vladimir Jovic]

* Vladimir Jovic:

* Vladimir Jovic:
(e-mail address removed) wrote:
Hello Group,

Consider the following class design using forward declarations.

//start-snip
#include<memory>

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;
};

int main() {
MyConcrete obj;
return 0;
}
// end-snip

[more sniping]

From this example, to me it looks like the OP wanted to implement the
pimpl

Q-2. Do boost smart-pointers also have the same semantics w.r.t to the
above situation?

Off course

Happily that's incorrect.

std::auto_ptr's referent type is required to be complete, but that is
not a requirement of e.g. boost::shared_ptr.

And in particular, boost::shared_ptr provides custom deleter
functionality that probably can handle the OP's case with ease
(weasel-words present because the example is lacking in detail).

I am bit confused. Don't you need the class declaration to use that
class? (assuming the class is defined somewhere )
Do you have an example where you would only forward declare a class,
and then do something useful with it?

Well, the PIMPL idiom is a/the prime example of taking advantage of this
property -- or lack of requirement -- of boost::shared_ptr.

For the PIMPL idiom it's not even necessary to explicitly use a custom
deleter because with PIMPL you can easily arrange to have a complete
referent type at the time of initialization of the shared_ptr. And
shared_ptr then just does The Right Thing. It generates a deleter
function that it calls on destruction.

I.e., in the implementation file it goes like this:

class Foo::Impl {}; // Complete definition of Impl class.

Foo::Foo()
: myPImpl( new Impl ) // shared_ptr has all info it needs here.
{}

But the Impl is declared (and defined) in the source file, and the
destructor has access to it. In original example, the Details class
wasn't defined.

Some folks have argued that the same technique must in practice have to
work nicely also for std::auto_ptr, at least if also Foo::~Foo() is
defined in the implementation file, because how could any compiler
manage to screw up things then? But according to the standard's general
rules it's just UB to do it with std::auto_ptr. And one risks the
compiler warning about that UB, as above.

On my compiler (g++ 4.1.2), modifying the example to this:

//start-snip
#include<memory>

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;
};

int main() {
MyConcrete obj;
return 0;
}

class Details
{
};
// end-snip

everything compiles fine (without warning). Am I causing UB in any way?

 

[abhay.burli]

When all is said and done, using a smart pointer for the
compilation firewall idiom seems a bit of overkill; adding a lot
of excess machinery for no really good reason.

But most large projects have phical design like layering. Say for
example you have the following layers (sample only)
App
Pres
GUI
Platform
Base
Fundamental

Normally one would allow a top-down access but not bottom-up access
for code maintainability.
PIMPL is used mostly in the GUI layers to hide the 3rd party library
specific implementation.

I agree that it is usually overkill, but it may be required sometimes
IMHO.

In fact, you've have to take extreme pains NOT to have a
complete type; normally, you'll have just new'ed it (and new
requires a complete type).

Yes its is painful but i guess we have no option under such
circumstances.

Regards,
Abhay

 

[James Kanze]

But most large projects have phical design like layering. Say
for example you have the following layers (sample only)
App
Pres
GUI
Platform
Base
Fundamental

Normally one would allow a top-down access but not bottom-up
access for code maintainability. PIMPL is used mostly in the
GUI layers to hide the 3rd party library specific
implementation.

I wasn't saying that the compilation firewall idiom is overkill;
in fact, I find it should be more or less an automatism as soon
as the class stops being trivial, and starts containing other
classes. What I consider "overkill" is trying to find a smart
pointer to implement it. Basically, unless the smart pointer
contains some pretty complex machinery (like boost::shared_ptr),
you're going to have to declare a destructor anyway, and
definite it in a location where the full definition of the
implementation class is visible. Given that, you might as well
just throw in the delete yourself---it's not as if you need a
lot of reflection to determine where the implementation should
be deleted.

I agree that it is usually overkill, but it may be required
sometimes IMHO.

Yes its is painful but i guess we have no option under such
circumstances.

There's nothing really painful about it. Typically, you'll have
the exported header file, with just:

class Something
{
public:
Something( ... ) ;
~Something() ;
// ...
private:
class Impl ;
Impl* myImpl ;
} ;

The initialization of Something will take place in a source file
(not exported), where you'd naturally have the full definition
of Something::Impl anyway.

My point was basically that in order to avoid even declaring the
destructor, you need a lot of complex and very heavy baggage
under the hood, with support for a lot of things you don't need
(e.g. reference counting, etc.), and with a name which is at the
least misleading in this case---you don't share the
implementation object. If you declare and define the
destructor, you can, of course, use auto_ptr, or with semantics
even closer to what is needed (and practically no excess
baggage), boost::scoped_ptr. But once you've declared and
defined a destructor, why bother. It's just as easy, if not
easier, to simply slip in the delete where it is needed.

 

[James Kanze]

Alf P. Steinbach wrote:

[...]

On my compiler (g++ 4.1.2), modifying the example to this:

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;

int main() {
MyConcrete obj;
return 0;
}

class Details
{};

// end-snip

everything compiles fine (without warning). Am I causing UB in
any way?

Formally, yes. Put the definition of Details in another
translation unit, and g++ complains. It probably doesn't here
due to an artifact of the way it instantiates templates---I
think it parses the complete file before instantiating any
templates. And having seen the complete file, it knows the
structure of Details.

 

[Alf P. Steinbach]

* James Kanze:

* Vladimir Jovic:
[...]

std::auto_ptr's referent type is required to be complete,
but that is not a requirement of e.g. boost::shared_ptr.
And in particular, boost::shared_ptr provides custom
deleter functionality that probably can handle the OP's
case with ease (weasel-words present because the example is
lacking in detail).
I am bit confused. Don't you need the class declaration to
use that class? (assuming the class is defined somewhere
) Do you have an example where you would only forward
declare a class, and then do something useful with it?

Well, the PIMPL idiom is a/the prime example of taking
advantage of this property -- or lack of requirement -- of
boost::shared_ptr.

Except that you probably don't want the semantics of
boost::shared_ptr with compilation firewall idiom. Most of the
time, you want the semantics of boost::scoped_ptr, but I think
it also requires a complete object.

When all is said and done, using a smart pointer for the
compilation firewall idiom seems a bit of overkill; adding a lot
of excess machinery for no really good reason.

As a very relevant example, Herb Sutter, long time chair of the C++
standardization committee and moderator of comp.lang.c++.moderated, now lead
architect of Microsoft's C++/CLI, once wrote about using auto_ptr for PIMPL
idiom: "In fact, if there's no other reason for explicitly writing a destructor,
we don't need to bother with a custom destructor at all any more. Clearly, this
is easier than managing the pointer manually, and it follows the good practice
of wrapping resource ownership in objects--a job that auto_ptr is well suited to
do. We'll revisit this example again at the end."

<url: http://www.gotw.ca/publications/using_auto_ptr_effectively.htm>

Of course, Herb at the time couldn't be aware of the formal limitations of
auto_ptr in this regard... But anyway, it really seems that at the time he
disagreed very much with your stance, that he thought there was a good reason
and didn't think there was a lot of excess machinery.

Of course I'm using this authority argument because your argument essentially is
just that vague sort, so I'm just providing a counter-weight of the same sort.
One unsubstantiated opinion by well-known person (mod, committee member) pitted
against the opposing unstubstantiated opinion of another well-known person (mod,
former committee chair). And, well, my opinion also.

That one shouldn't use auto_ptr for PIMPL is discussed elsewhere on the GOTW site.

Another PIMPL gotcha, which is probably what you referred to by mentioning
scoped_ptr, but I don't know if Herb discusses it at all, is that with an
ownership transfer or shared ownership smart pointer the outer class should not
be copyable, unless e.g. a shared implementation is intentional. If it is
desired to have it copyable then a cloning smart point should be employed. ;-)

In fact, you've have to take extreme pains NOT to have a
complete type; normally, you'll have just new'ed it (and new
requires a complete type).

In some cases a PIMPL wraps some vendor's class Boo where a Boo instance is
obtained by a factory function createBoo and destroyed by destroyBoo. In this
case Boo can very easily be an incomplete type, or be a dummy class without any
relationship to what Boo pointers actually point to. Anyway you then need to
have destroyBoo called at destruction, and a shared_ptr custom deleter works
fine for that.


Cheers,

- Alf

 

[Vladimir Jovic]

Alf P. Steinbach wrote:
[...]

Some folks have argued that the same technique must in
practice have to work nicely also for std::auto_ptr, at
least if also Foo::~Foo() is defined in the implementation
file, because how could any compiler manage to screw up
things then? But according to the standard's general rules
it's just UB to do it with std::auto_ptr. And one risks the
compiler warning about that UB, as above.

On my compiler (g++ 4.1.2), modifying the example to this:

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;

int main() {
MyConcrete obj;
return 0;
}

class Details
{};

// end-snip

everything compiles fine (without warning). Am I causing UB in
any way?

Formally, yes. Put the definition of Details in another
translation unit, and g++ complains. It probably doesn't here
due to an artifact of the way it instantiates templates---I
think it parses the complete file before instantiating any
templates. And having seen the complete file, it knows the
structure of Details.

Sorry to be PITA, but I really don't understand this. I changed the
example to this:

// file fr.cpp
#include<memory>
#include "hy1.hpp"

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;
};

int main() {
MyConcrete obj;
return 0;
}

// file hy1.hpp
class Details
{
public:
Details(int a );
~Details();
void inc();
int *b;
};


// file hy1.cpp
#include "hy1.hpp"
Details:etails(int a ) :
b(new int(a))
{
}
Details::~Details()
{
delete(b);
}
void Details::inc()
{
*b += 3;
}


As you can see, Details is in another translation unit, and the compiler
doesn't complain.
What do I have to do to make it complains?


PS Ignore missing guards and style

 

[Alf P. Steinbach]

* Vladimir Jovic:

Alf P. Steinbach wrote:
[...]
Some folks have argued that the same technique must in
practice have to work nicely also for std::auto_ptr, at
least if also Foo::~Foo() is defined in the implementation
file, because how could any compiler manage to screw up
things then? But according to the standard's general rules
it's just UB to do it with std::auto_ptr. And one risks the
compiler warning about that UB, as above.

On my compiler (g++ 4.1.2), modifying the example to this:

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;

int main() {
MyConcrete obj;
return 0;
}

class Details
{};

// end-snip

everything compiles fine (without warning). Am I causing UB in
any way?

Formally, yes. Put the definition of Details in another
translation unit, and g++ complains. It probably doesn't here
due to an artifact of the way it instantiates templates---I
think it parses the complete file before instantiating any
templates. And having seen the complete file, it knows the
structure of Details.

Sorry to be PITA, but I really don't understand this. I changed the
example to this:

// file fr.cpp
#include<memory>
#include "hy1.hpp"

class Details;

class MyConcrete {
private:
std::auto_ptr<Details> details_;
};

int main() {
MyConcrete obj;
return 0;
}

// file hy1.hpp
class Details
{
public:
Details(int a );
~Details();
void inc();
int *b;
};


// file hy1.cpp
#include "hy1.hpp"
Details:etails(int a ) :
b(new int(a))
{
}
Details::~Details()
{
delete(b);
}
void Details::inc()
{
*b += 3;
}


As you can see, Details is in another translation unit, and the compiler
doesn't complain.
What do I have to do to make it complains?


PS Ignore missing guards and style

Not sure whether you can make the compiler complain (there's no guarantee,
that's the general nature of UB).

But above, by virtue of including [hy1.hpp] in the main file [fr.cpp] you have
the definition of Details in both translation units. The definition of a class
is not the collection of member definitions. It is the 'class' thing with
declarations -- and sometimes inline definitions -- of the members, and it
tells the compiler among other things the size of an instance and whether there
is a user defined constructor and/or a user defined destructor.

Remove the #include in [fr.cpp], perhaps the compiler will complain then; anyway
you then have formal UB, even if it's difficult to think of a way the compiler
could realize that and then underhandedly making the program do silly things,
especially considering that you do not instantiate Details.

Cheers & hth.,

- Alf

 

[James Kanze]

* James Kanze:

* Vladimir Jovic:

[...]

std::auto_ptr's referent type is required to be complete,
but that is not a requirement of e.g. boost::shared_ptr.
And in particular, boost::shared_ptr provides custom
deleter functionality that probably can handle the OP's
case with ease (weasel-words present because the example is
lacking in detail).
I am bit confused. Don't you need the class declaration to
use that class? (assuming the class is defined somewhere
) Do you have an example where you would only forward
declare a class, and then do something useful with it?
Well, the PIMPL idiom is a/the prime example of taking
advantage of this property -- or lack of requirement -- of
boost::shared_ptr.

Except that you probably don't want the semantics of
boost::shared_ptr with compilation firewall idiom. Most of the
time, you want the semantics of boost::scoped_ptr, but I think
it also requires a complete object.
When all is said and done, using a smart pointer for the
compilation firewall idiom seems a bit of overkill; adding a lot
of excess machinery for no really good reason.

As a very relevant example, Herb Sutter, long time chair of
the C++ standardization committee and moderator of
comp.lang.c++.moderated, now lead architect of Microsoft's
C++/CLI, once wrote about using auto_ptr for PIMPL idiom: "In
fact, if there's no other reason for explicitly writing a
destructor, we don't need to bother with a custom destructor
at all any more. Clearly, this is easier than managing the
pointer manually, and it follows the good practice of wrapping
resource ownership in objects--a job that auto_ptr is well
suited to do. We'll revisit this example again at the end."

Except that there is a very good reason for not using
auto_ptr in this case---it doesn't work. Nor do most other
smart pointers. To make it work, the smart pointer needs a lot
of extra baggage. A real lot.

As for the "good practice of wrapping resource ownership in
objects", the class itself is an object; I fail to see where
using a smart pointer really adds anything here. (And I know
who Herb Sutter is.)

<url:http://www.gotw.ca/publications/using_auto_ptr_effectively.htm>

Of course, Herb at the time couldn't be aware of the formal
limitations of auto_ptr in this regard...

Or rather, he overlooked them. The limitations aren't just
formal; making auto_ptr work on an incomplete object would
increase its complexity significantly. And they were well known
from the moment auto_ptr was presented. And if Herb had
actually tried any significant examples with auto_ptr, he would
have realized that it didn't work.

But anyway, it really seems that at the time he disagreed very
much with your stance, that he thought there was a good reason
and didn't think there was a lot of excess machinery.

And he proposed a solution which didn't work. Herb's not
perfect. If there were a simple solution which would allow you
to omit the destructor entirely, without creating other
problems, fine. As far as I know, there's not---shared_ptr is
not simple, and gives the wrong message, and auto_ptr and
scoped_ptr don't work.

Software engineering is concerned with trade offs. If the
memory management pattern was more complicated, or the proposed
*working* solutions simpler, then the answer might be different.

Of course I'm using this authority argument because your
argument essentially is just that vague sort, so I'm just
providing a counter-weight of the same sort.

What's vague about the fact that auto_ptr doesn't work. Or the
fact that shared_ptr is a lot of excess baggage (additional
allocations, virtual functions or pointers to functions, etc.),
and that it gives the wrong message (the pointer isn't "shared"
with anything).

If auto_ptr (or scoped_ptr, which IMHO is even more appropriate)
would work, and allow you to omit the destructor, I'd argue in
favor of them. As it is, all they by you is one trivial and
obvious line in the destructor. Weighed against one additional
include required in the header (more source for the client code
to compile) and a more complicated declaration in the class
itself.

One unsubstantiated opinion by well-known person (mod,
committee member) pitted against the opposing unstubstantiated
opinion of another well-known person (mod, former committee
chair). And, well, my opinion also.

Except that the "unstubstantiated opinion" in one case is based
on a false premise: that you could use auto_ptr and not declare
and define a destructor.

That one shouldn't use auto_ptr for PIMPL is discussed
elsewhere on the GOTW site.

Another PIMPL gotcha, which is probably what you referred to
by mentioning scoped_ptr, but I don't know if Herb discusses
it at all, is that with an ownership transfer or shared
ownership smart pointer the outer class should not be
copyable, unless e.g. a shared implementation is intentional.
If it is desired to have it copyable then a cloning smart
point should be employed. ;-)

If the outer class has reference semantics, or uses copy on
write, then the issues are different. If the outer class uses
deep copy, and you have a cloning smart pointer in your tool
kit, it's certainly worth considering; if the cloning smart
pointer can clone in the absense of a definition of the class,
then it's definitely worth considering. (The implementation of
such a cloning smart pointer is left to the reader. It's far
from trivial.) IMHO, it's not worth developing a cloning smart
pointer exclusively for the compilation firewall idiom, however,
if you don't already have it for other things. It's just that
much extra work, for a very, very limited real gain.

 

[James Kanze]

* Vladimir Jovic:

Alf P. Steinbach wrote:
[...]

// file fr.cpp
#include<memory>
#include "hy1.hpp"
class Details;
class MyConcrete {
private:
std::auto_ptr<Details> details_;
};
int main() {
MyConcrete obj;
return 0;
}
// file hy1.hpp
class Details
{
public:
Details(int a );
~Details();
void inc();
int *b;
};
// file hy1.cpp
#include "hy1.hpp"
Details:etails(int a ) :
b(new int(a))
{
}
Details::~Details()
{
delete(b);
}
void Details::inc()
{
*b += 3;
}
As you can see, Details is in another translation unit, and
the compiler doesn't complain. What do I have to do to make
it complains?
PS Ignore missing guards and style

Not sure whether you can make the compiler complain (there's
no guarantee, that's the general nature of UB).

But above, by virtue of including [hy1.hpp] in the main file
[fr.cpp] you have the definition of Details in both
translation units. The definition of a class is not the
collection of member definitions. It is the 'class' thing with
declarations -- and sometimes inline definitions -- of the
members, and it tells the compiler among other things the size
of an instance and whether there is a user defined constructor
and/or a user defined destructor.

Formally, the issue is whether the type is complete or not. The
class definition makes the type complete.

Remove the #include in [fr.cpp], perhaps the compiler will
complain then;

G++ does complain in this case. (It's basically what I tried.)

anyway you then have formal UB, even if it's difficult to
think of a way the compiler could realize that and then
underhandedly making the program do silly things, especially
considering that you do not instantiate Details.

However, it's not difficult to imagine that the compiler doesn't
realize that there is undefined behavior, and generate code
which does the wrong thing. Most compilers are totally unaware
of most of the standard library, and treat std::auto_ptr just as
they would any other template definition. Without concepts or
such, there would no problem in the specialization of auto_ptr
itself---the class probably only contains a pointer to the type,
and the language allows pointers to incomplete types. If the
destructor is instantiated over an incomplete type, however, the
issue is different. The destructor almost certainly contains a
delete expression, and "If the object being deleted has
incomplete class type at the point of deletion and the complete
class has a non-trivial destructor or a deallocation function,
the behavior is undefined." If the type is incomplete, of
course, the compiler can't know whether it has a non-trivial
destructor or a deallocation function, of course---the compiler
can warn about a potential error, or it can assume that you know
what you are doing, and that the class doesn't have a
non-trivial destructor or a deallocation function. In practice,
what will doubtlessly happen (warning or not) is that the memory
will be freed, but the class' destructor won't be called.

Note that in the case of g++, the warning is not a result of
constraints. (G++ verifies a lot of the constraints in the
standard library, but constraint violations result in an error,
not a warning.) You get exactly the same messages with:

class Details {} ;

int
main()
{
Details* p = NULL ;
delete p ;
return 0 ;
}

And the message is exceptionally clear, ending with:

note: neither the destructor nor the class-specific operator
delete will be called, even if they are declared when the
class is defined.

If you ignore the message, or compile the code with a compiler
which doesn't warn, the destructor will not be called, even if
it is not trivial.

In the case of auto_ptr, of course, the formal rule is the one
that the type must be complete for all instantiations. This is
a global rule in the standard; I'm pretty sure it was a case of
no one having the time, or wanting to take it, to specify the
completion requirements on a class by class or function by
function basis. In practice, I think a "typical" implementation
of auto_ptr would work with an incomplete type, except for the
destructor, the assignment operator and the reset function; i.e.
those functions which are required to delete the pointed to
object.