Why is the (virtual) destructor of the derived class not called when deleting an array through a pointer to the base class?

Question

I have an Animal class with a virtual destructor, and a derived class Cat.

#include <iostream>

struct Animal
{
    Animal() { std::cout << "Animal constructor" << std::endl; }
    virtual ~Animal() { std::cout << "Animal destructor" << std::endl; }
};

struct Cat : public Animal
{
    Cat() { std::cout << "Cat constructor" << std::endl; }
    ~Cat() override { std::cout << "Cat destructor" << std::endl; }
};

int main()
{
    const Animal *j = new Cat[1];
    delete[] j;
}

This gives the output:

Animal constructor
Cat constructor
Animal destructor

I don't understand why is the Cat's destructor not called, when my base class destructor is virtual?

Anthropomorphize

cannot reproduce godbolt.org/z/asahqsvxM. What compiler are you using? – Giacinta

which standard version are you using? Please only tag that and remove the other – Giacinta

Totally OT, but if a function doesn't have any arguments then you don't need to write anything. Instead of e.g. Cat(void) only Cat() will work. – Lucillelucina

Also OT, but the member should not be a raw pointer, and when it is you need to consider the rule of 3 / 5. – Giacinta

@463035818_is_not_a_number clang 14.0.3 on macOS reproduces OP's output for me. – Orian

@Orian I can only reproduce the output with gcc when Animal::~Animal is not virtual (godbolt.org/z/eKndcKGbM) – Giacinta

@463035818_is_not_a_number I changed your first compiler explorer link to x86-64 clang 16.0.0 and got OP's output. – Orian

silly bugs require silly thinking; maybe a #define virtual somewhere? Check with #ifdef virtual \n #error "virtual is defined" \n #endif – Kalb

Minimal repro: godbolt.org/z/EaaYnKPMP Note the const isn't needed, but array new/delete is. – Alansen

@Kalb no silly thinking required. We already found out that nothing but posted code is needed to reproduce the output with clang godbolt.org/z/aKxMnozbW. Only OP needs to clarify what they are using – Giacinta

(I've answered this, but interesting to note that GCC gets just as confused later if you change the number of cats allocated to 2) – Alansen

You almost never want an array of polymorphic types. You want an array of (smart) pointers to such types instead. – Herzog

Just to follow up on the Cat(void) vs. Cat() thing—in C, you should use the former (the void distinguishes between unspecified and empty arity), but in C++ it's an oxymoron (this cruft has been sensibly removed) and you should use the latter. Highly recommended reading: softwareengineering.stackexchange.com/a/287002 – Slightly

Since Cat and Animal are not the same size, you are going to have an issue if you try to index more than one Cat in the Animal array. This is also probably related to why some compilers are not immediately getting confused on this specific example. – Contrived

Deleting through the converted pointer is UB, as one (correct) answer says. There is no guarantee in general that the converted pointer points to the beginning of the array. (in case of multiple base classes, for one) Storing the result of new[] in a variable declared with const auto should keep the type and value of j appropriate for use with delete[], and the code would work as expected. It's (almost) no use trying to find a sensible reason for a specific behaviour in presence of UB. It resorts to compiler psychology, not logic. – Cleavers

Answer 1

Note that whilst a Cat is an Animal, an array of Cats is not an array of Animals. In other words, arrays are invariant in C++, not covariant like they are in some other languages.

So you are up-casting this array and this later confuses the compiler. You must do array delete[] in this case on the correct, original, type - Cat*.

Note that you would have similar issues for the same reason if you allocated an array of 2 or more Cats, cast this to an Animal* and then tried to use the second or subsequent Animal.

Alansen

Typo: down casting -> Upcasting (Downcasting would be Animal -> Cat) – Orian

std::vector to the rescue – Giacinta

Does a tool like CPPCheck see the issue? Can someone point a precise wording, perhaps in cppreference, about why, while storing a derived * as base * is legal, the unexpected delete [] is called? – Duffie

@Dharmesh946 I added such answer. – Ink

Could you elaborate on the terms invariant and covariant? I tried looking them up on a web search, but there was too much (off-diagonal) covariance with the math/data term. – Spectroscope

@AndrewHolmgren en.wikipedia.org/wiki/… – Herzog

This would become even clearer if you had a data member in Animal and added a second data member to Cat. Since then sizeof(Animal)!=sizeof(Cat), array indexing would be completely off. An array of smart pointers to Animal would work as expected since the destructor would get called for each pointer, and pointers are genuinely polymorphic. – Seay

@463035818_is_not_a_number It's not even an array thing. In C++, a Cat is not an Animal, full stop. C++ simply does not fully implement (I want to say: "believe in") the OOP is-a relationship, not in the way e.g. Java does. We can use the is-a model to design/plan class hierarchies. But the correct mental model for actually writing C++ is that a Cat only contains a (special) Animal object, as a base-class subobject. std::vector doesn't help much: a std::vector<Animal> cannot hold Cats. Assigning a std::vector<Cat> to one makes a copy of only the Animal part of each Cat. – Daughterinlaw

@Daughterinlaw what you say all makes sense, but frankly, has nothing to do with my confusion. A std::vector<Animal*> would be fine with a cat. Its obvious that a std::vector<Animal> is not a vector of pointers to Animal, but as soons as c-arrays are involved I get confused – Giacinta

Answer 2

It's Undefined Behaviour in my understanding because of (in 7.6.2.9 Delete,p2, Emphasis mine):

In a single-object delete expression, the value of the operand of delete may be a null pointer value, a pointer value that resulted from a previous non-array new-expression, or a pointer to a base class subobject of an object created by such a new-expression. If not, the behavior is undefined. In an array delete expression, the value of the operand of delete may be a null pointer value or a pointer value that resulted from a previous array new-expression...

Which basically means that for delete[] the type must be the exact one from new[] (no base class sub-objects allowed like delete).

So for the reason that is like this - in my opinion this time is obvious - the implementation needs to know how much is the full object size so it can iterate to the next array element.

I wrote counter arguments for why this could be different - but after some thought (and reading the comments) - I realised that such a solution is solving X-Y problem.

Ink

This encourages dangerous behaviour though. If you allow this then you are basically saying Animal* a = new Cat[2]; delete[] a; is ok. This would implicitly imply that a is useful on its own when it very much is not. a[1] is undefined behaviour at this point as array index uses the type (a[i] is defined as (a+i)) which moves on a by sizeof(Animal) bytes and not sizeof(Cat) bytes which it must do. So just avoiding this whole issue by not allowing a to be useful across the board is probably the right thing to do. – Alansen

Re "the implementation needs to store the number of array elements anyway": The number of elements may be found dynamically by an end marker. The destructor iterates the array anyway if element destructors are called, so that's not a time loss. The allocated block will probably store a block size, but that may not be the minimum required for the array size (it could always be a power of 2, for example). If the element type does not have a destructor, then the end marker is irrelevant and no iteration at all will take place -- the block can be released right away wholesale. – Babel

In addition, the first assignment in main does not guarantee in general that the Animal*, pointing to the first element of the array as an Animal would have the same value as the pointer returned by new Cat[], especially if Animal is not the first base class of Cat and EBO did not kick in on the previous ones. So yes, in general, deleting through the converted pointer ought to be UB. Using const auto instead should fix it :-P – Cleavers

Answer 3

I answer to my own comment: https://en.cppreference.com/w/cpp/language/delete (emphasis mine)

For the second (array) form, expression must be a null pointer value or a pointer value previously obtained by an array form of new-expression whose allocation function was not a non-allocating form (i.e. overload (10)). The pointed-to type of expression must be similar to the element type of the array object. If expression is anything else, including if it's a pointer obtained by the non-array form of new-expression, the behavior is undefined.

https://en.cppreference.com/w/cpp/language/reinterpret_cast#Type_aliasing

Informally, two types are similar if, ignoring top-level cv-qualification:

• they are the same type; or
• they are both pointers, and the pointed-to types are similar;or
• they are both pointers to member of the same class, and the types of the pointed-to members are similar; or
• they are both arrays of the same size or both arrays of unknown bound, and the array element types are similar. (until C++20)
• they are both arrays of the same size or at least one of them is array of unknown bound, and the array element types are similar.

So far as I understand, inheritance is not similarity...

Duffie

Inheritance cannot be similarity indeed. Note that the array contains values, not references, and a Cat values is larger than an Animal value, so it won't fit there. That makes the cast from Cat * to Animal * invalid when pointing to an array, but since the type does not indicate it's pointing to an array and for single instance it is valid, the programmer has to mind this. – Marlyn

ah yes - note that if you allocate more than one cat then for example j[1] calculates a wrong address for the second cat, because it uses sizeof(Animal) instead of sizeof(Cat) – Chromoprotein

Where do they get the "similarity" idea from? The standard only seems to say "pointer value that resulted from a previous array new-expression". Perhaps there is a constraint or additional remark somewhere which is not cross-referenced from 7.6.2.9? By the way, I must assume that "value" in the standard means "typed value" (and not simply numerical value, or the OP's code would work). But the standard does not define its use of "value" afaics, which may be a defect. – Babel

@Peter-ReinstateMonica it came from wg21.link/cwg2474 – Escalera

@Escalera Interesting, thanks. As an aside, the definition of "similar" is unintelligible to me. (I assume they mean roughly that cv-qualifications don't matter and that one can omit array bounds, but that's only a qualified guess. Examples would really help here.) – Babel

Answer 4

My only criticism of the other answers is that they are too tame. Arrays work because the entries all have the same size, so the starting address of the entry with index 5 can be computed easily. Casting array of Derived to array of Base (which is what you are doing here) could have much worse consequences than you have shown. (If the array happens to have only one entry or Derived has no new data members you might be OK.) A[1].method() will probably do something strange because (Base*)(A)+1 is not even the starting address of any object. The method will read the wrong data, and the effect of any change will be highly unpredictable. Any call to a virtual function will use an entirely meaningless vtable pointer, so who knows what 'code' will be executed. Even if that call is not an immediate disaster, you might override a vtable pointer. Destruction of the array will almost certainly delete some meaningless 'pointers'.

Pointer to single object (allocated & deleted without '[]') is fine. You can cast to Base*, dynamic_cast back, and call virtual functions. That's why Ayxan Haqverdili's suggestion works. Array of X should never have anything except objects of run-time type X, and new[] (even new[1]) creates arrays.

Villainage

"they are two tame": did you mean "too tame"? – Thorlie

Answer 5

Others have explained the issue, but if you want an array of polymorphic objects, you want an array of pointers. Here's one way:

vector<unique_ptr<Animal>> animals;
animals.push_back(std::make_unique<Cat>());
animals.push_back(std::make_unique<Dog>());
animals.push_back(std::make_unique<Monkey>());

Megalo

Answer 6

The Cat's destructor is not called because the pointer J is declared as pointer to a constant Animal. When you delete an array of objects through a pointer to a base class with a virtual destructor the destructor of each derived class object is not called unless the pointer type is of derived class.

So for calling the Cat Destructor. modity the pointer type to

int main()
{
const Cat* j=new Cat[1];
delete[] j;
}

Aurelea

Answer 7

Make the Cat destructor also virtual and the code works as expected.

Kilovolt

As the destructor of the base class (Animal) is declared virtual, the destructor of the subclass Cat will implicitly be virtual too, as virtual-ness is propagated. – Trump

Please read attentively all answers and comments, as the issue is tricky. You'll see that it may "work" but not on all compilers and not in all situations, and rightly so. – Duffie