mr-edd.co.uk :: horsing around with the C++ programming language

Avoid: empty inline destructors

[13rd July 2008]

Recently the Google C++ style guide was making the rounds on reddit.

There's quite a lot in there that I actually disagree with, because I'm a software developer, and hey, software developers love to argue about largely pointless stuff such as this. But one thing caught my eye that I hadn't really considered before.

Here's the advice given for inline functions:

Define functions inline only when they are small, say, 10 lines or less.

Google C++ style guide

I think that's entirely sensible. The interesting bit is in the rationale, which goes on to say:

A decent rule of thumb is to not inline a function if it is more than 10 lines long. Beware of destructors, which are often longer than they appear because of implicit member- and base-destructor calls!

Google C++ style guide

Now I quite commonly write code like this:

class abstract_base
{
    public:
        virtual ~abstract_base() { } // <-- empty inline definition, in the header
        // ...
    private:
        std::vector<std::string> data_;
};

But, I really shouldn't. Even though I've written absolutely no code inside my destructor, it doesn't mean that it does nothing.

If asbtract_base happens to have a large number of member objects[1], the destructor will do quite a lot of work in destroying those objects. In the example code, the destructor of a vector<string> will be invoked and inlined inside this supposedly empty definition if the compiler sees fit.

Now, vector<string>'s destructor will probably be an inline function too because it's a template[2]. So that destructor could itself get inlined everywhere an abstract_base is destroyed.

Do we really want all this code inlined by the compiler? In many cases, probably not.

Lesson learned.

Footnotes
  1. I think a large number of member objects is an indicator of a dodgy design and possible violates the one responsibility-per-code-entity principle, but we'll ignore that for now []
  2. without support for the export keyword, all member functions of template classes must be defined inline, including the destructor []

Comments

Pierre Phaneuf

[13/07/2008 at 22:57:00]

Also, when using a forward declaration and a scoped_ptr (a common pattern, to reduce dependencies), it will simply not be possible to generate the destructor! This can even happen when the destructor is not even virtual, or not even specified at all.

Dave Johansen

[15/07/2008 at 23:11:00]

Is the the compiler actually required by the standard to inline the function if it has an inline definition? Because it just doesn't seem to me that that would be the case.

Edd

[16/07/2008 at 17:19:00]

Pierre: good point. Though I've often wondered why a scoped_ptr doesn't create a function pointer to do the deletion. This would allow easier "pimpl-ing" because the scoped_ptr's destructor wouldn't have to know about the pointee's destructor (but its constructor would) e.g:

template<typename T>
class my_scoped_ptr : boost::noncopyable
{
    public:
        my_scoped_ptr(T *p) : p_(p), del_(&my_scoped_ptr<T>::destroy) { }
        ~my_scoped_ptr() { del_(p_); }

        // similar interface to boost::scoped_ptr ...

    private:
        static void destroy(T *p) { delete p; }

        T *p_;
        void (*del_)(T *);
};

Perhaps some would object to the overhead of an extra member and a function-pointer call?

Dave: the compiler isn't required to inline the function, but by defining it inline you make it much easier for it to do so.

The point was that, out of laziness, I sometimes define empty inline virtual destructors right in the header. On the face of it, it seems harmless enough because the body is empty and so it doesn't appear to do anything. But appearances can be deceiving; it still does a non-trivial amount of work in destroying a vector of strings. That's probably enough such that if I were to do the work explicitly, I would do so inside a function not declared inline.

Dave Johansen

[17/07/2008 at 05:11:00]

Ya, but isn't that the whole point of a compiler? (allow the programmer to be lazy when it comes to code generation so he/she can focus on the algorithm/design) Of course, a human could always come up with a slightly better way to right most loops or optimize some small segment of code, but that shouldn't be anything that a programmer spends any time on (at least in the design/development phases).

Edd

[17/07/2008 at 19:57:00]

You're right Dave, but in most cases you're at least aware of when you're leaving it up to the compiler. In this case I wasn't aware of possible consequences of the choice I had made; I know you're not suggesting that we define every function inline and let the compiler loose!

Perhaps I should have called this post something like "be aware of the fact that empty (inline) constructors aren’t often empty"?!

Dave Johansen

[20/07/2008 at 23:33:00]

I agree that there's definitely a balance to be had, because we can't just write up horrible code and expect the compiler to make it magically work like a charm.

And to be honest, I had never even thought about the potential pitfalls of this (or even realized that it may be happening). And knowing what is going on under the hood (and acting accordingly) is never a bad thing.

(optional)
(optional)
(required, hint)

Links can be added like [this one -> http://www.mr-edd.co.uk], to my homepage.
Phrases and blocks of code can be enclosed in {{{triple braces}}}.
Any HTML markup will be escaped.