Why was destructor executed twice?

Question

#include <iostream>
using namespace std;

class Car
{
public:
    ~Car()  { cout << "Car is destructed." << endl; }
};

class Taxi :public Car
{
public:
    ~Taxi() {cout << "Taxi is destructed." << endl; }
};

void test(Car c) {}

int main()
{
    Taxi taxi;
    test(taxi);
    return 0;
}

this is output:

Car is destructed.
Car is destructed.
Taxi is destructed.
Car is destructed.

I use MS Visual Studio Community 2017(Sorry, I don't know how to see the Visual C++'s edition). When I used debug mode. I find one destructor is executed when leaving thevoid test(Car c){ } function body as expected. And an extra destructor appeared when the test(taxi);is over.

The test(Car c) function uses value as formal parameter. A Car is copied when going to the function. So I thought there will be only one "Car is destructed" when leaving the function. But actually there are two "Car is destructed" when leaving the function.(the first and second line as showed in the output) Why are there two "Car is destructed"? Thank you.

===============

when I add a virtual function in class Car for example:virtual void drive() {} Then I get the expected output.

Car is destructed.
Taxi is destructed.
Car is destructed.

Vendee

Maybe this helps: #14184841. Probably Taxi calls the destructor of its mainclass Car in your case. – Verleneverlie

Could be an issue in how the compiler handles the object slicing when passing a Taxi object to a function taking a Car object by value? – Tubule

Must be your old C++ compiler. g++ 9 gives the expected results. Use a debugger to determine the reason why an extra copy of the object is made. – Anjelicaanjou

I have tested g++ with version 7.4.0 and clang++ with version 6.0.0. They gave expected output which differs from op's output. So problem might be about compiler that he uses. – Dysuria

I reproduced with MS Visual C++. If I add a user-defined copy-constructor and default constructor for Car then this issue disappears and it gives expected results. – Adon

@Adon Really? Blimey! I guess since copying in this case is trivial it's permitted to do that, but whyyyyyyyyy would it – Arne

Please add compiler and version to the question – Arne

@Stanojkovic Careful with those edits, please. Car.class would be Java. This is not Java. – Arne

The following experiment test(*static_cast<Car*>(&taxi)) results in exactly 3 destructor message whereas test(static_cast<Car>(taxi)) still results in ' destructor messages, confirming that the compiler does not detect the best way to get a Car from a Taxi. – Weathered

Answer 1

It looks like the Visual Studio compiler is taking a bit of a shortcut when slicing your taxi for the function call, which ironically results in it doing more work than one might expect.

First, it's taking your taxi and copy-constructing a Car from it, so that the argument matches.

Then, it's copying the Car again for the pass-by-value.

This behaviour goes away when you add a user-defined copy constructor, so the compiler seems to be doing this for its own reasons (perhaps, internally, it's a simpler code path), using the fact that it is "allowed" to because the copy itself is trivial. The fact that you can still observe this behaviour using a non-trivial destructor is a bit of an aberration.

I don't know the extent to which this is legal (particularly since C++17), or why the compiler would take this approach, but I would agree that it's not the output I would have intuitively expected. Neither GCC nor Clang do this, though it may be that they do things the same way but are then better at eliding the copy. I have noticed that even VS 2019 is still not great at guaranteed elision.

Arne

Sorry, but isn't this exactly what I said with " conversion from Taxi into Car if your compiler doesn't do the copy elision." – Weathered

That is an unfair remark, because the pass by value vs pass by refernece to avoid slicing was only added in an edit, to help OP beyond this question. Then my answer was not a shot in the dark, it was clearly explained from the start where it may come from and I'm glad to see that you come to the same conclusions. Now looking at your formulation, "It looks like... I don't know", I think there is the same amount of uncertainy here, because frankly neither do I nor you understand why the compiler needs to generate this temp. – Weathered

Okay then remove the unrelated parts of your answer leaving just the single related paragraph behind – Arne

Ok, I removed the distracting slicing para, and I've justified the point about copy elision with precise references to the standard. – Weathered

Could you explain why a temporary Car should be copy-constructed from the Taxi and then copied again into the parameter ? And why the compiler does not do this when provided with a plain car ? – Weathered

Answer 2

What is happening ?

When you create a Taxi, you also create a Car subobject. And when the taxi gets destroyed, both objects are destructed. When you call test() you pass the Car by value. So a second Car gets copy-constructed and will get destructed when test() is left. So we have an explanation for 3 destructors: the first and the two last in the sequence.

The fourth destructor (that is the second in the sequence) is unexpected and I couldn't reproduce with other compilers.

It can only be a temporary Car created as source for the Car argument. Since it doesn't happen when providing directly a Car value as argument, I suspect it is for transforming the Taxi into Car. This is unexpected, since there is already a Car subobject in every Taxi. Therefore I think that the compiler does make an unnecessary conversion into a temp and doesn't do the copy elision that could have avoided this temp.

Clarification given in the comments:

Here the clarification with reference to the standard for language-lawyer to verify my claims:

• The conversion I am referring to here, is a conversion by constructor [class.conv.ctor], i.e. constructing an object of one class (here Car) based on an argument of another type (here Taxi).
• This conversion uses then a temporary object for returning its Car value. The compiler would be allowed to make a copy elision according [class.copy.elision]/1.1, since instead of constructing a temporary, it could construct the value to be returned directly into the parameter.
• So if this temp gives side-effects, it's because the compiler apparently doesn't make use of this possible copy-elision. It's not wrong, since copy elision is not mandatory.

Experimental confirmation of the anaysis

I could now reproduce your case by using the same compiler and draw an experiment to confirm what is going on.

My assumption above was that the compiler selected a suboptimal parameter passing process, using the constructor conversion Car(const &Taxi) instead of copy constructing directly from the Car subobject of Taxi.

So I tried calling test() but explicitly casting the Taxi into a Car.

My first attempt did not succeed to improve the situation. The compiler still used the suboptimal constructor conversion:

test(static_cast<Car>(taxi));  // produces the same result with 4 destructor messages

My second attempt succeeded. It does the casting as well, but uses pointer casting in order to strongly suggest the compiler to use the Car subobject of the Taxi and without creating this silly temporary object:

test(*static_cast<Car*>(&taxi));  //  :-)

And surprise: it works as expected, producting only 3 destruction message :-)

Concluding experiment:

In a final experiment, I provided a custom constructor by conversion:

 class Car {
 ... 
     Car(const Taxi& t);  // not necessary but for experimental purpose
 }; 

and implement it with *this = *static_cast<Car*>(&taxi);. Sounds silly, but this also generates code that will only display 3 destructor messages, thus avoiding the unnecessary temporary object.

This leads to think that there could be a bug in the compiler that causes this behavior. It is af is the possibility of direct copy-constructing from the base class would be missed in some circumstances.

Weathered

Doesn't answer the question – Arne

@Vendee I think this confirms the hypothesis of the temporary for conversion without copy elision, because this temporary would be generated out of the function, in the caller's context. – Weathered

When saying "the conversion from Taxi into Car if your compiler doesn't do the copy elision", what copy elision are you referring to? There should be no copy which needs to be elided in the first place. – Adon

@Adon because the compiler does not need to construct a Car temporary based based on Taxi's Car sub-object for doing the conversion and then copy this temp into the Car parameter: it could elide the copy and directly construct the parameter from the original subobject. – Weathered

Copy elision is when the standard states that a copy should be created, but under certain circumstances allows the copy to be elided. In this case there is no reason for a copy to be created in the first place (a reference to Taxi can be passed directly to the Car copy constructor), so copy elision is irrelevant. – Adon

@Adon My assumption is that the compiler falsely uses this temp for what I call a conversion according to [class.conv.ctor], i.e. constructing an object of one based on another type. In this case we would be exactly in the case of [class.copy.elision]/1.1, where the return value of the conversion constructor could construct directly into the parameter. – Weathered

@Adon experimental proof added - using a far fetched casting allows to avoid the unnecessary temporary construction :-) – Weathered

@LightnessRacesinOrbit 1) could you please provide a reference to the standard ? I just found "When certain criteria are met, an implementation is allowed to omit the copy/move construction of a class object, even if the constructor selected for the copy/move operation and/or the destructor for the object have side effects." 2) I've now added an experimental demonstration that confirms what is going on. – Weathered

I didn't dispute that unnecessarily creating a temporary would cause this behavior. What I said is that the temporary shouldn't have been created in the first place, so explicitly creating a temporary doesn't contradict this. As the other answer says, this seems to be related to how the compiler treats trivially copyable objects, and I think it's a compiler bug. – Adon

@Weathered Don't know where you found that text but it is out of date. Google "mandatory copy elision". – Arne

@LightnessRacesinOrbit N4713 dated 2017-11-27 - also of interest: devblogs.microsoft.com/cppblog/… ? – Weathered

@Weathered You're splitting hairs. That article says "the new rules do not guarantee copy elision. Instead, the new value category rules are defined such that no copy exists in the first place". And, yes, that is true (though not a reason to say "elision is not guaranteed"; they're just being picky about wording). But clearly it is not the case here. The point remains, that what used to require a copy, now requires no copy, and as such there should be no copy in the OP's case (if we are to accept a few preconditions that I don't think anyone in this thread has really confirmed yet). – Arne

Since when does constructing a subclass result in the construction of another separate object for each superclass? There is no Car constructed object to be passed by value to test, it's a Taxi which is a subclass of Car and therefore typechecks, because it is a subclass, not because there is another separate Car object pointlessly created – Dissipate

@Dissipate That's the point. For some reason, the compiler doesn't use that shorter way. It does so only when lured with a tricky cast or when providing a user defined converting constructor. Also, some more experiments show that the compiler generate the right code if the Car class would contain a string member. For me it looks like a bug that prevent the copy-constructing the argument directly from Taxi's base. Now, if you have some nice precise standard references that support your point, feel free to share because this could help writing a bullet proof bug report. – Weathered