AMDG On 03/17/2011 10:32 AM, Jens Auer wrote:

Well, that's true for exception_detail, but error_info has a public non-virtual destructor and might be used as a base class (I am sure it isn't, but when I just have a quick look at it when coding it doesn't say that it shouldn't).

The virtual functions are undocumented implementation details. You're flipping the usual rule here. A class should never be used as a base class unless it explicitly intended to be one and is documented as such. In Christ, Steven Watanabe

...

Well, that's true for exception_detail, but error_info has a public non-virtual destructor and might be used as a base class (I am sure it isn't, but when I just have a quick look at it when coding it doesn't say that it shouldn't).

error_info has a protected (not public) non-virtual destructor, by design.

Just checked the code provided in 1.46.1: template <class Tag,class T> class error_info: public exception_detail::error_info_base { public: typedef T value_type; error_info( value_type const & value ); ~error_info() throw(); value_type const & value() const { return value_; } value_type & value() { return value_; } private: std::string tag_typeid_name() const; std::string value_as_string() const; value_type value_; }; }

...

error_info has a protected (not public) non-virtual destructor, by design.

Just checked the code provided in 1.46.1: template <class Tag,class T> class error_info: public exception_detail::error_info_base { public: typedef T value_type; error_info( value_type const & value ); ~error_info() throw(); value_type const & value() const { return value_; } value_type & value() { return value_; } private: std::string tag_typeid_name() const; std::string value_as_string() const; value_type value_; }; }

On 3/17/2011 2:17 PM, Ted Byers wrote:

Hi Stevem,

...

-----Original Message----- From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: March-17-11 12:37 PM To: boost-users@lists.boost.org Subject: Re: [Boost-users] boost::exception_detail::error_info_base does not have virtual destructor

AMDG

On 03/17/2011 08:50 AM, Jens Auer wrote:

...

I am compiling a project with a more strict warning set and got the following reports: Error 29 error C4265: 'boost::exception_detail::error_info_base' : class has virtual functions, but destructor is not virtual c:\work\svn\sandbox\externals\boost\include\boost-1_45\boost\exception\detai l\error_info_impl.hpp 36

Error 30 error C4265: 'boost::error_info<Tag,T>' : class has virtual functions, but destructor is not virtual c:\work\svn\sandbox\externals\boost\include\boost-1_45\boost\exception\detai l\error_info_impl.hpp 69

IMHO these classes should have a virtual destructor as they are intended to be used as base classes.

No they shouldn't. A virtual destructor is only needed when deleting through a base class pointer. This is explicitly forbidden by making the destructor protected.

I must respectfully disagree.

There are two ways to look at this 'should'. First, there is what is required or permitted by the standard. Second, there is what is useful in managing a software development project. When I am trying to help my juniors develop good programming habits, one of the things I want them to do is always provide a virtual destructor whenever they make one or more member functions virtual. That lets a team comprised of many more junior programmers than really really old guys like me avoid a lot of subtle bugs. And I don't see a significant down side to making a destructor virtual. Hence, when I am mentoring junior programmers, I routinely advise them to make destructors virtual whenever they need one of the other member functions to be virtual, whenever the issue arises.

Are you aware of a significant downside to making destructors virtual as a matter of habit?

Cheers

Ted

Why not just encourage the habit of making *public* destructors virtual in the presence of one or more other virtual methods? That is, after all, when a virtual destructor can be necessary.

On Thu, Mar 17, 2011 at 12:27 PM, Ted Byers <r.ted.byers@gmail.com> wrote:

Because I tend to let junior programmeers work only on those classes that would have a public destructor anyway.  I find young, inexperienced programmers need to develop a good grounding on, or experience with, generally applicable patterns before being introduced to situations where one can usefully deviate from those patterns. <snip> Again, can you point to a significant downside to having all destructors virtual?

Virtual isn't something that you slap on functions for good measure; it is an invitation to call a function through a base pointer. I agree that it is best to teach basic patterns to novices. You could teach them to always make destructors in base types protected and non-virtual, and to use shared_ptr to manage the lifetime of (polymorphic) objects. From a different point of view, this simply means that delete (and the public virtual destructors it requires) is best treated as an advanced feature novices should avoid. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Frank Mori Hess

On Thursday, March 17, 2011, Ted Byers wrote:

...

Again, can you point to a significant downside to having all destructors virtual?

The usual reason given is that if the class has no other virtual functions, then making the destructor virtual adds some size overhead to each object, since they have to keep a pointer to a virtual function table.

That's why I teach the kids to make the destructor virtual IF they need to make some other member function virtual. But I don't usually take them to the various optimization issues until they're at the intermediate level. Cheers Ted

From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Steven Watanabe

On 03/17/2011 11:17 AM, Ted Byers wrote:

...

I must respectfully disagree.

There are two ways to look at this 'should'. First, there is what is required or permitted by the standard. Second, there is what is useful in managing a software development project. When I am trying to help my juniors develop good programming habits, one of the things I want them to do is always provide a virtual destructor whenever they make one or more member functions virtual. That lets a team comprised of many more junior programmers than really really old guys like me avoid a lot of subtle bugs. And I don't see a significant down side to making a destructor virtual. Hence, when I am mentoring junior programmers, I routinely advise them to make destructors virtual whenever they need one of the other member functions to be virtual, whenever the issue arises.

a) This is in Boost. If a junior programmer were maintaining it, this would be the least of his worries.

b) Boost.Exception uses a different idiom which is even more safe. i.e. use shared_ptr. c) The virtual functions in question are an implementation detail used in a very restricted context. From a user's perspective, it's a concrete class,

d) Emil is strongly against changing the code to eliminate warnings that don't represent real problems, and prefers suppressing them explicitly. I have to say that I have a lot of sympathy for his view, since beyond a certain point, if you try to eliminate all warnings, >fixing problems turns into working around compiler quirks. In my experience, eliminating all warnings on all compilers, results in less readable and maintainable code as we dance around all the constructs that any compiler thinks /might/ be a

:-) I wouldn't have a junior programmer maintaining it. However, I would routinely encourage junior programmers to make use of boost libraries, so they don't run the risk of reinventing the wheel (e.g. in our code, you'll never find a naked pointer, but rather one of boost's smart pointers). But then I was responding specifically to the question of whether or not destructors should be virtual if there are other virtual member functions. plain and simple. problem under /some/ >circumstances.

I wouldn't presume to tell Emil what he should do with his library. It does, though, mean I have some explaining to do when the kids see a good library that occassionally does things a bit differently from what I try to teach them. I do have a little sympathy with your, and his, position, when dealing with extremely tight time constraints, but not a lot. If one of the design criteria is that the code being produced must be widely portable, then I pass it through the range of platforms and compilers that we have to support, and as far as it is possible, I try to treat all warnings as errors. And that is an attitude I try to encourage among those junior programmers I mentor. This is, without question, much easier when dealing with only one compiler on one platform, but it remains the ideal I encourage the kids to aim for. I don't worry so much about 'readability' as I encourage extensive comments within the code for those blocks that are a little less than obvious. Such comments have the effect of both making the code maintainable and flagging those potential querks that can be reviewed as new versions of compilers are released (to see if they are still an issue with the new version of the supported compiler(s)). Cheers Ted

From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Emil Dotchevski Sent: March-17-11 5:27 PM On Thu, Mar 17, 2011 at 1:58 PM, Ted Byers <r.ted.byers@gmail.com> wrote:

...

 I do have a little sympathy with your, and his, position, when dealing with extremely tight time constraints, but not a lot.  If one of the design criteria is that the code being produced must be widely portable, then I pass it through the range of platforms and compilers that we have to support, and as far as it is possible, I try to treat all warnings as errors.

The virtual destructor warning goes directly against a conscious design decision. In my opinion it also teaches programmers a bad habit. This doesn't make the warning any less annoying of course, so I'm doing my best to suppress it.

The fact that we're having this discussion is another upside: it shows the reader that at least some people think that it is an error to "fix" this

I would like to understand why making destructors virtual when there are other virtual member functions might be seen as a bad habit. I would also appreciate it if you can educate me (while I am so old I can recall fighting off T.Rex so I could enjoy my bronto-burgers, I'm not too old to learn something new. :-) What, precisely, was that conscious design decision, and what is the rationale for it? It is my experience that when two experienced developers disagree about a practice, it is born of differences in the nature of the problems they have faced in the past, and the information they have at their disposal. If you were to look at the applications I develop, you'd find very few objects created on the stack. Almost everything goes on the heap, managed by the most appropriate of the boost smart pointers. In my environmental modelling software, for example, the application starts off with almost nothing in the heap, but as the user builds the model, he may end up producing hundreds or even thousands of instances of sometimes complex UDTs, and these UDTs are often drawn from complex inheritance trees (but almost never involving multiple inheritance ;-). Connections between these instances can often be quite complex, so there is, in the base class, a function that breaks all connections among the objects before any attempt is made to delete anything. Because the number of UDTs is quite large, and there is a common modelling interface for which there are virtual functions (as pure virtual functions in the base class), all these objects are managed in a single std::vector containing smart pointers having pointers to the base class. About 99% of the effort in these applications is focussed on these UDTs, and the containers managing them, and managing the resources they require (which varies wildly among UDTs). particular warning. :) Agreed. Cheers Ted

From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Emil Dotchevski Sent: March-18-11 3:11 AM On Thu, Mar 17, 2011 at 3:22 PM, Ted Byers <r.ted.byers@gmail.com> wrote:

...

...

From: boost-users-bounces@lists.boost.org Sent: March-17-11 5:27 PM On Thu, Mar 17, 2011 at 1:58 PM, Ted Byers <r.ted.byers@gmail.com> wrote:

The virtual destructor warning goes directly against a conscious

...

design decision. In my opinion it also teaches programmers a bad habit. This doesn't make the warning any less annoying of course, so I'm doing my best to suppress it.

I would like to understand why making destructors virtual when there are other virtual member functions might be seen as a bad habit.

OK I'm exaggerating, obviously there are worse habits one can be taught. :)

A public virtual destructor lets anyone call delete as they please. In any non-trivial program this isn't a good thing.

...

What, precisely, was that conscious design decision, and what is the rationale for it?

"The error_info_base class does not support deleting objects of derived types polymorphically" (and virtual is used to indicate that a particular operation is polymorphic, which would be misleading in this case.)

...

It is my experience that when two experienced developers disagree about a practice, it is born of differences in the nature of the problems they have faced in the past, and the information they have at their disposal.  If you were to look at the applications I develop, you'd find very few objects created on the stack.  Almost everything goes on the heap, managed by the most appropriate of the boost smart pointers.  In my environmental modelling software, for example, the application starts off with almost nothing in the heap, but as the user builds the model, he may end up producing hundreds or even thousands of instances of sometimes complex UDTs, and these UDTs are often drawn from complex inheritance trees (but almost never involving multiple inheritance ;-).  Connections between these instances can often be quite complex, so there is, in the base class, a function that breaks all connections among the objects before any attempt is made to delete anything. Because the number of UDTs is quite large, and there is a common modelling interface for which there are virtual functions (as pure virtual functions in the base class), all these objects are managed in a single std::vector containing smart pointers having pointers to the

Why? You create classes to manage certain resources as you need them, and get rid of them when you don't. And, actually, when the pointer to new acquired instance of a class is placed within a hared pointer, you don't have to explicitly call delete. Instead, you let the smart pointer do it. base class.

What is the reason for not storing shared_ptrs in that std::vector?

Actually, paraphrasing myself, what I had written was that the pointers to these objects are cast to pointers to the base class and placed in smart pointers (usually boost::shared_ptr), and these smart pointers are placed in the std::vector.

Wouldn't a protected and non-virtual destructor be more appropriate in that case?

How could a protected, non-virtual destructor be appropriate here at all? You have a single container of smart pointers to a base class. But the actually instances there represent possibly hundreds of different derived classes, generally with very different storage requirements. These are all objects on the heap, so all were created initially using operator new. Operator delete has thus to be called on each of them, and if the destructor is not virtual, you'd have to cast back to the real UDT in order to delete them. Since they are pointers to the base class, you'd have to try a dynamic cast each to every known UDT in order to get a pointer to the right class in order to properly delete it. That would be an unmaintainable nightmare that is completely avoidable simply by using a virtual destructor and the combination of pointers to the base class living in shared pointers living in turn in a std::vector. The number of UDTs is huge and continually growing (for the biota portion of the model, there are typically UDTs for genera and species, and since there are so many species on the planet, I will probably not live long enough to see UDTs made for all of them). Cheers, Ted

From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Steven Watanabe Sent: March-18-11 11:27 AM On 03/18/2011 05:37 AM, Ted Byers wrote:

...

...

From: boost-users-bounces@lists.boost.org [mailto:boost-users-bounces@lists.boost.org] On Behalf Of Emil Dotchevski Wouldn't a protected and non-virtual destructor be more appropriate in that case?

How could a protected, non-virtual destructor be appropriate here at all?

You have a single container of smart pointers to a base class. But the actually instances there represent possibly hundreds of different derived classes, generally with very different storage requirements. These are all objects on the heap, so all were created initially using operator new. Operator delete has thus to be called on each of them, and if the destructor is not virtual, you'd have to cast back to the real UDT in order to delete them. Since they are pointers to the base class, you'd have to try a dynamic cast each to every known UDT in order to get a pointer to the right class in order to properly delete it. Nope. shared_ptr remembers the actual type it was constructed with. It doesn't need a virtual destructor, unless you cast to the base before you create the shared_ptr.

Right. The type for the container would be: std::vector<boost::shared_ptr<base_type> > boost::shared_ptr<base_type> and boost::shared_ptr<derived_type> are two different types. You can't cast from the one to the other without writing code to do it. So, to make one vector containing (smart) pointers to instances of any or all the derived types, you would have to cast the pointers to instances the derived types to the base before you create the shared_ptr. Or is it the case that extra intelligence has been adde to boost::shared_ptr, et al, so that if you create a boost::shared_ptr<base_type>, and pass it a pointer to derived type, it remembers the type of the original pointer (and maintains that memory across copy/assignment), and does the right thing when the object is to be freed? If so, that capability was added after I started using them extensively. But I'd be curious as to why and how that would have been added, and when, when virtual functions acomplish the same thing with no extra effort or pain.

...

That would be an unmaintainable nightmare that is completely avoidable simply by using a virtual destructor and the combination of pointers to the base class living in shared pointers living in turn in a std::vector. The number of UDTs is huge and continually growing (for the biota portion of the model, there are typically UDTs for genera and species, and since there are so many species on the planet, I will probably not live long enough to see UDTs made for all of them).

Cheers Ted

On Fri, Mar 18, 2011 at 8:55 AM, Ted Byers <r.ted.byers@gmail.com> wrote:

The type for the container would be: std::vector<boost::shared_ptr<base_type> >

boost::shared_ptr<base_type> and boost::shared_ptr<derived_type> are two different types.  You can't cast from the one to the other without writing code to do it.  So, to make one vector containing (smart) pointers to instances of any or all the derived types, you would have to cast the pointers to instances the derived types to the base before you create the shared_ptr.

Put the return value from new directly into a shared_ptr -- it'll just work! When the object expires, shared_ptr will not call the destructor through a base pointer, so the base destructor should be protected (and non-virtual) to protect from someone attempting to call delete on a base pointer. Emil Dotchevski Reverge Studios, Inc. http://www.revergestudios.com/reblog/index.php?n=ReCode

AMDG On 03/18/2011 12:33 PM, Ted Byers wrote:

OK. Good. But how? I am not one to settle for 'it just works'. Rather, I like to dig into code to see precisely how it works. Though, when dealing with business admin types, whose eyes glaze over in incomprehension the moment the discussion begins to get technical, I too use the sentence "Trust me, it just works." ;-)

I actually had the code for boost:shared_ptr open, as a result of Frank Hess' message, looking for where and how this capability was implemented.

I don't suppose you can speed my examinatin of this code by giving me some line numbers ... ?

The code for managing destruction is in boost/smart_ptr/detail/sp_counted_base.hpp and boost/smart_ptr/detail/sp_counted_impl.hpp In Christ, Steven Watanabe

AMDG On 03/18/2011 03:47 PM, Emil Dotchevski wrote:

On Fri, Mar 18, 2011 at 12:33 PM, Ted Byers<r.ted.byers@gmail.com> wrote:

...

The most useful power of runtime polymorphism is based on invokving the right virtual functions through base pointers, so I don't see why it is bad to use polymorphism when it comes to destruction also. The way shared_ptr destroys objects is also polymorphic, it just doesn't use virtual functions. It's actually more powerful than virtual functions because it doesn't need a virtual function table; as illustrated, even a shared_ptr<void> can destroy the object correctly.

It does use virtual functions internally, just not virtual functions of the pointee. In Christ, Steven Watanabe