7

I have spending ages trying to figure out why this is happening.

  struct Element
    {
        Element(int number) : number_ID(number) 
        { std::cout << "Element number " << number_ID << " is being constructed\n"; }
        ~Element() 
        { std::cout << "Element number " << number_ID << " is being destroyed\n"; }
        int number_ID;
    };

    void createVector()
    {
        std::vector<Element> objArr;
        objArr.reserve(10);        // So it doesn't have to reallocate

        objArr.emplace_back(1);
        objArr.emplace_back(2);
        objArr.emplace_back(3);
        objArr.emplace_back(4);

        objArr.erase(objArr.begin());      // When I erase the first element only element 4 has destructor called
    }

    int main()
    {
        createVector();


        system("pause");
    }

I get:

Element number 1 is being constructed
Element number 2 is being constructed
Element number 3 is being constructed
Element number 4 is being constructed
//The following called when doing objArr.erase(objArr.begin());
Element number 4 is being destroyed
//The following called after createVector() function exits
Element number 2 is being destroyed
Element number 3 is being destroyed
Element number 4 is being destroyed

The destructor for Element 1 never gets called? At first I didn't know why Element number 4 destructor would get called when erasing the first element, and then I thought when it shifts its members maybe the destructor has to get called. But the documentation says that all of the members after the one deleted get shifted, and the destructors of 2 and 3 didn't get called. I am really confused.

EDIT: Well if this is an optimisation step, then the documentation is wrong, because:

Removes from the vector either a single element (position) or a range of elements ([first,last)).

This effectively reduces the container size by the number of elements removed, which are destroyed.

That's not destroying then.

Zebrafish
  • 11,682
  • 3
  • 43
  • 119
  • You erase the first element. All elements that follow get copied/moved (by assignment) to the left to fill the empty space. After that the tail element gets destroyed. That's what you see. – AnT stands with Russia Jan 26 '17 at 03:31
  • @AnT I don't get it, I thought vector is always careful to call any destructors, like for example when reallocating it has to call all destructors. So if I need the destructor called I need to call it myself? – Zebrafish Jan 26 '17 at 03:36
  • 1
    But there's no need to call more than one destructor in this case. There's no reallocation. And shifting of elements is naturally achieved by a chain of assignments. So, all you need is to destroy the tail element after these reassignments. You can also think of assignmet as of destruction followed by immediate construction of its LHS: you have to destroy the old value of the LHS and construct the new value in it. In that sense, all remaining elements get "destructed" and "re-constructed". – AnT stands with Russia Jan 26 '17 at 04:18
  • @Ant My object manages something when it's constructed, so it needs to delete something in its destructor. I assumed its safe for the vector to destroy the object since it's supposed to "manage" the elements in the vector. Destruction in C++ has a specific meaning, that's why I made the point that the documentation is clearly wrong. The erased element is NOT destroyed. – Zebrafish Jan 26 '17 at 04:22
  • @TitoneMaurice: Where does it say that the "erased element" is the object that will be destroyed? It only says that *an* object will be destroyed. If you want an object to manage something, then you have to do it *correctly*. And your current code does not do it right. This is not the fault of `vector` or its documentation. This is your class's fault. – Nicol Bolas Jan 26 '17 at 04:54
  • 2
    @TitoneMaurice To phrase things a different way: These couts are only showing half the picture. Try implementing the copy constructor and assignment operator (and maybe move constructor and move assignment for completeness) and put cout statements in them too. Then you should see how values are being transferred and overwritten such that the first element is effectively being eliminated. – TheUndeadFish Jan 26 '17 at 05:27
  • 1
    That quote isn't from the standard but from cplusplus.com, which is notoriously sub-standard. The Standard doesn't specify which elements get destroyed, only how many. – molbdnilo Jan 26 '17 at 09:53

1 Answers1

14

vector is trying to save performance.

What erase does is it copies elements 2, 3 and 4 down one element, overwriting the element containing 1. So the first element now contains 2, etc. Then it destroys the last element, which is a copy of 4.

That has the effect of erasing the first element, but not quite in the way you thought it would. It erases the contents of the first element, but not the object itself.

Depending on the type you store in your vector, this will be less expensive than destroying the first element, only to reconstruct a copy of the second element in the storage of the first.

Well if this is an optimisation step, then the documentation is wrong, because:

...

That's not destroying then.

The documentation is correct, and it is destroying them.

Your objects are the values they contain. You declared that your classes were copyable, so vector is free to copy them. That means it's perfectly legitimate for two instances to have the same value. vector exploits that.

If you had made your classes non-copyable but noexcept moveable, then it would work correctly. But your move constructor would also need to null-out the moved-from object, so that no two instances would have the same value.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • The first element is holding an OpenGL VBO ID, which I have to delete, oh man. – Zebrafish Jan 26 '17 at 03:34
  • 9
    @TitoneMaurice: Then your class ***should not be copyable!*** A class that is copyable is a class where two instances can have the same value, and you can pass values from one to the other. That's not what you want. [This is how you encapsulate OpenGL objects](http://stackoverflow.com/documentation/opengl/7556/encapsulating-opengl-objects-with-c-raii#t=201701260337507343073) – Nicol Bolas Jan 26 '17 at 03:38
  • Man, I should have known writing up the proof would take too long. Oh well: http://ideone.com/vlxlae – user4581301 Jan 26 '17 at 03:38
  • The docs say the objects erased get destroyed, that isn't the case. I just don't know. – Zebrafish Jan 26 '17 at 03:39
  • 2
    @TitoneMaurice It's not a problem if you write (corrects) copy and move assignment operators/constructors. See [rule of three](http://stackoverflow.com/a/4172724/3893262) (five since C++11). – O'Neil Jan 26 '17 at 03:40
  • @TitoneMaurice: The documentation *never* says that that particular object will be destroyed. Only that *one* of the objects will be destroyed and the container will shrink. And that no remaining object will have the value of the one you erased. – Nicol Bolas Jan 26 '17 at 03:42
  • @Nicol Bolas It actually says the elements removed are destroyed. I removed the first element, I don't see how it destroyed it if its destructor wasn't called. – Zebrafish Jan 26 '17 at 03:48
  • @TitoneMaurice It is not destroyed, it's been assigned to the second element. See the link in my previous comment. – O'Neil Jan 26 '17 at 03:51
  • @TitoneMaurice: It says that it *removes* the element from the array. Which is 100% true; after `erase` you would be unable to find any element in the array containing the value 1. It *never* says that it explicitly *deletes* that particular object. What you're not understanding is the distinction between which object you're talking about and what that object's *value* is. `vector` only cares about the *value* of the objects it stores, not their identity. – Nicol Bolas Jan 26 '17 at 03:52
  • If you want an object who's value and its identity are one in the same and unique, then that object cannot be copyable or moveable. If you want an object whose value is unique but the value can be moved around, it cannot be copayble. – Nicol Bolas Jan 26 '17 at 03:54