The Things You Spot During Code Reviews

I was doing an informal code review the other day, and I ran into some code that I thought would make a fascinating blog post. It had to do with a pure virtual destructor declaration, followed by the same destructor’s definition.

The code in question looked something like this (with other details elided):

class Test {
public:
	virtual ~Test() = 0;
};
inline Test::~Test() {}

What is this code doing? Probably not what the developer thought it was doing. Unfortunately, I’ve not yet been able to get in touch with the developer who wrote the code. But I think I have a fairly good idea of what they were trying to accomplish.

The declaration of the destructor being pure virtual was likely done for two reasons. The first is that the developer didn’t want anyone to attempt to declare an instance of this base class, and the second is that the developer wanted to ensure subclasses define a virtual destructor for their object. I am more uncertain of what the developer thought would happen for the inline definition of the destructor, but I’m guessing they thought it was required in some way.

So did the developer accomplish their goals? Not really, no. Looking at the C++ specification:

A destructor can be declared virtual (10.3) or pure virtual (10.4); if any objects of that class or any derived class are created in the program, the destructor shall be defined. If a class has a base class with a virtual destructor, its destructor (whether user- or implicitly- declared) is virtual.

(I pulled this quote from the C++0x draft specification, but the C++03 and C++98 specifications read almost identically.)

What this says, in effect, is that pure virtual destructors do not actually make the class abstract. And if you run some example code through Lint’s online static analyzer, you’ll see that the code is not flagged as an error. However, for Visual Studio 2010, it will be flagged as an error because Test is considered an abstract class. I believe this to be a bug with the VC++ compiler, but not a particularly egregious one. I am thinking the developer who wrote this code was using Visual Studio and thought this was the proper way to ensure no one creates an instance of Test. Of course, a better way would be to make the default constructor non-accessible, like this:

// Pre C++0x:
class Test {
private:
	Test();
};

// C++0x way to do it:
class Test {
public:
	Test() = delete;
};

As for solving the second requirement (that subclasses define a destructor), the pure virtual approach does not work there either. Because the destructor will be implicitly defined by the class as per the specification, the subclass has no hard requirement to implement a destructor explicitly. There is no way to force a class to define a destructor, nor should there be. A base class cannot predict the resource requirements of its subclasses, so it should have no way to dictate how subclasses free those resources.

This brings up the question of what happens with the inline definition of the destructor which followed the class declaration. Also from the specification (10.4.2 in the C++0x spec):

[ Note: a function declaration cannot provide both a pure-specifier and a definition —end note ]

I don’t believe this note applies here though, because the declaration does not also provide the definition as part of the statement. That would be more like: virtual ~Test() = 0 { }. However, the specification makes no further mention of definitions which are not a part of the declaration, which leads me to believe the behavior is up to the compiler writer and not to be relied upon.

So I tried it out in Visual Studio 2010 to see what happens, using the following code:

class Test {
public:
	virtual void Foo() = 0;
};

void Test::Foo()
{
	::printf( "In foo\n" );
}

int main(int argc, char* argv[])
{
	return 0;
}

If you declare an instance of Test within main, the compiler will give an error that Test is an abstract class. However, it seems as though the compiler does generate code for Test::Foo, even though the code is logically unreachable. The following disassembly shows there’s definitely a definition of Foo:

Definition of a pure virtual function

So the compiler doesn’t believe the definition to be a problem and generates for code it, but trying to create an instance of the class fails. This lead me to the next logical step — let’s make the code compile and see what it does!

class Test {
public:
	virtual void Foo() = 0;
};

class Test2 : public Test {
public:
	virtual void Foo() {
		Test::Foo();
	}
};

void Test::Foo()
{
	::printf( "In foo\n" );
}

int main(int argc, char* argv[])
{
	Test2 t2;
	Test *t = &t2;
	t->Foo();
	return 0;
}

Using Visual Studio 2010, this compiles and runs to completion while printing “In foo” to the console. I did not test to see what other compilers do, but I certainly wouldn’t rely on the behavior!

tl;dr: pure virtual destructors do not do anything particularly useful, and creating definitions for pure virtual functions is unspecified.

This entry was posted in C/C++ and tagged , . Bookmark the permalink.

Leave a Reply

Your email address will not be published. Required fields are marked *