Is extending a base class with non-virtual destructor dangerous in C++

Question

Take the following code

class A {
};
class B : public A {
};
class C : public A {
   int x;
};

int main (int argc, char** argv) {
   A* b = new B();
   A* c = new C();

   //in both cases, only ~A() is called, not ~B() or ~C()
   delete b; //is this ok?
   delete c; //does this line leak memory?

   return 0;
}

when calling delete on a class with a non-virtual destructor with member functions (like class C), can the memory allocator tell what the proper size of the object is? If not, is memory leaked?

Secondly, if the class has no member functions, and no explicit destructor behaviour (like class B), is everything ok?

I ask this because I wanted to create a class to extend std::string, (which I know is not recommended, but for the sake of the discussion just bear with it), and overload the +=,+ operator. -Weffc++ gives me a warning because std::string has a non virtual destructor, but does it matter if the sub-class has no members and does not need to do anything in its destructor?

FYI the += overload was to do proper file path formatting, so the path class could be used like

class path : public std::string {
    //... overload, +=, +
    //... add last_path_component, remove_path_component, ext, etc...
};

path foo = "/some/file/path";
foo = foo + "filename.txt";
std::string s = foo; //easy assignment to std::string
some_function_taking_std_string (foo); //easy implicit conversion
//and so on...

I just wanted to make sure someone doing this

path* foo = new path();
std::string* bar = foo;
delete bar;

would not cause any problems with memory allocation

--edit: Thanks to GMan for explaining in detail why this should not be done, and honorable mention Matthieu M. for explaining how to do it safely if one must do it.

Answer 1

Problem can occur if you store the memory address of a derived type inside the base type and then call delete on the base type:

B* b = new C();
delete b;

If B had a virtual destructor then C's destructor would be called and then B's. But without a virtual destructor you have undefined behavior.

the following 2 deletes cause no problem:

B* b = new B();
delete b;
C* c = new C()
delete c;

Answer 2

No, it's not safe to publically inherit from classes without virtual destructors, because if you delete the derived through a base you enter undefined behavior. The definition of the derived class is irrelevant (data members or not, etc.):

§5.3.5/3: In the first alternative (delete object), if the static type of the operand is different from its dynamic type, the static type shall be a base class of the operand’s dynamic type and the static type shall have a virtual destructor or the behavior is undefined. _{(Emphasis mine.)}

Both of those examples in your code lead to undefined behavior. You can inherit non-publicly, but that obviously defeats the purpose of using that class then extending it. (Since it's not longer possible to delete it through a base pointer.)

This is (one reason*) why you shouldn't inherit from standard library classes. The best solution is to extend it with free-functions. In fact, even if you could you should prefer free-functions anyway.

---

_{*Another being: Do you really want to replace all your string usage with a new string class, just to get some functionality? That's a lot of unnecessary work.}

Answer 3

It is only safe to *private*ly inherit from base classes without a virtual destructor. Public inheritance would make it possible for a derived class to be deleted through a base class pointer, which is undefined behavior in C++.

This is one of the only reasonable uses of private inheritance in C++.

Answer 4

So everyone has been saying you cant do it - it leads to undefined behaviour. However there are some cases where it is safe. If you are never creating instances of your class dynamically then you should be OK. (i.e. no new calls)

That said, it is generally considered a bad thing to do as someone might try to create one polymorphically at some later date. ( You may be able to protect against this by having a private unimplemented operator new, but I'm not sure. )

I have two examples where I don't hate deriving from classes with non-virtual destructors. The first is creating syntactic sugar using temporaries ... here's a contrived example.

class MyList : public std::vector<int>
{
   public:
     MyList operator<<(int i) const
     {
       MyList retval(*this);
       retval.push_back(i);
       return retval;
     }
   private: 
     // Prevent heap allocation
     void * operator new   (size_t);
     void * operator new[] (size_t);
     void   operator delete   (void *);
     void   operator delete[] (void*);
};

void do_somthing_with_a_vec( std::vector<int> v );
void do_somthing_with_a_const_vec_ref( const std::vector<int> &v );

int main()
{
   // I think this slices correctly .. 
   // if it doesn't compile you might need to add a 
   // conversion operator to MyList
   std::vector<int> v = MyList()<<1<<2<<3<<4;

  // This will slice to a vector correctly.
   do_something_with_a_vec( MyList()<<1<<2<<3<<4 );

  // This will pass a const ref - which will be OK too.
   do_something_with_a_const_vec_ref( MyList()<<1<<2<<3<<4 );

  //This will not compile as MyList::operator new is private
  MyList * ptr = new MyList();
}

The other valid usage I can think of comes from the lack of template typedefs in C++. Here's how you might use it.

// Assume this is in code we cant control
template<typename T1, typename T2 >
class ComplicatedClass
{
  ...
};

// Now in our code we want TrivialClass = ComplicatedClass<int,int>
// Normal typedef is OK
typedef ComplicatedClass<int,int> TrivialClass;

// Next we want to be able to do SimpleClass<T> = ComplicatedClass<T,T> 
// But this doesn't compile
template<typename T>
typedef CompilicatedClass<T,T> SimpleClass;

// So instead we can do this - 
// so long as it is not used polymorphically if 
// ComplicatedClass doesn't have a virtual destructor we are OK.
template<typename T>
class SimpleClass : public ComplicatedClass<T,T>
{
  // Need to add the constructors we want here :(
  // ...
   private: 
     // Prevent heap allocation
     void * operator new   (size_t);
     void * operator new[] (size_t);
     void   operator delete   (void *);
     void   operator delete[] (void*);
}

Heres a more concrete example of this. You want to use std::map with a custom allocator for many different types, but you dont want the unmaintainable

std::map<K,V, std::less<K>, MyAlloc<K,V> >

littered through your code.

template<typename K, typename V>
class CustomAllocMap : public std::map< K,V, std::less<K>, MyAlloc<K,V> >
{
  ...
   private: 
     // Prevent heap allocation
     void * operator new   (size_t);
     void * operator new[] (size_t);
     void   operator delete   (void *);
     void   operator delete[] (void*);
}; 

MyCustomAllocMap<K,V> map;

Answer 5

This is not an answer to your question, but to the problem you were trying to solve (path formatting). Take a look at boost::filesystem, which has a better way to concatenate paths:

boost::filesystem::path p = "/some/file/path";
p /= "filename.txt";

You can then retrieve the path as a string in both platform-neutral and platform-specific formats.

The best part is that it has been accepted into TR2, which means it will become part of the C++ standard in the future.

Answer 6

To add to what's been said.

• You are actually considering adding methods to a class that's already considered bloated.
• You want your path class to be considered as a string even though a number of operations would make little sense

I would propose that you use Composition over Inheritance. This way you will only reimplement (forward) those operations which really make sense for your class (I don't think you really need the subscript operator, for example).

Also, you might consider using a std::wstring instead, or perhaps a ICU string, to be able to handle more than ASCII characters (I am a nitpick, but then I am French and lower ASCII is insufficient for the French language).

It is, really, a matter of encapsulation. If you decide to handle UTF-8 characters properly one day and change your base class... chances are your clients will hang you by the feet. On the other hand, if you have used composition, they'll never have any issue as long as you tread carefully with the interface.

Finally, as has been suggested, free functions shall lead the way. Once again because they provide better encapsulation (as long as they are not friend...).