nickolay
nickolay

Reputation: 3895

C++: STL: vector: remove: destructor calls

The code is following:

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

struct A {
  A(int i = -1): i_(i) {
    wcout << "Constructor: i = " << i_ << endl;
  }
  A(A const &a) {
    wcout << "Copy constructor: i = " << i_ << " a.i = " << a.i_ << endl;
    *this = a;
  }
  ~A() { wcout << "Destructor: i = " << i_ << endl; }
  A &operator=(A const& a) {
    wcout << "Copy assignment operator: i = " << i_ << " a.i = " << a.i_ << endl;
    i_ = a.i_;
    return *this;
  }
  bool operator==(A const& rhs) { return i_ == rhs.i_; }
  int get() { return i_; }
private:
  int i_;
};

int wmain() {
  A a[] = {1, 2, 3, 2, 4, 5};
  vector<A> v(a, a + sizeof a/sizeof a[0]);
  wcout << "==== Just before remove ====" << endl;
 remove(v.begin(), v.end(), 2);
  wcout << "==== Just after remove ====" << endl;

  return 0;
}

Output:

==== Just before remove ====
Constructor: i = 2
Destructor: i = 2
Constructor: i = 2
Destructor: i = 2
Constructor: i = 2
Destructor: i = 2
Copy assignment operator: i = 2 a.i = 3
Constructor: i = 2
Destructor: i = 2
Constructor: i = 2
Destructor: i = 2
Copy assignment operator: i = 3 a.i = 4
Constructor: i = 2
Destructor: i = 2
Copy assignment operator: i = 2 a.i = 5
==== Just after remove ====

The question is: why destructor is called 6 times while remove() was running? I need this mess to be clarified.

NOTE: execute this code on your system, please, before answering. NOTE: MSVCPP 11

Upvotes: 2

Views: 1048

Answers (4)

NPE
NPE

Reputation: 500703

The question is: why destructor is called 6 times while remove() was running?

In summary, the destructor calls have to do with the 2 getting implicitly converted to A by remove(). Every time the result of such an implicit conversion goes out of scope, A's destructor gets called.

The reason for those implicit conversions is that remove() needs to compare every element of a to 2. The only way to do this is by calling A::operator==(const A&):

  bool operator==(A const& rhs) { ... }

Since rhs is of type const A&, the compiler:

  1. calls A(int) to convert the 2 to an A;
  2. calls operator==(const A&);
  3. calls A::~A() to destroy the temporary.

The latter are the destructor calls that you're seeing.

If you were to add the following comparison operator to A, you'll see those destructor calls disappear:

  bool operator==(int rhs) { return i_ == rhs; }

Alternatively, if you were to call remove() like so, you'll see all bar one destructor calls disappear:

remove( v.begin(), v.end(), A(2) );

Finally, if you were to make A::A(int) explicit, the compiler would not allow you to call remove() with 2 as the last argument (you'd have to call it with A(2)).

Upvotes: 7

bronekk
bronekk

Reputation: 2129

Well it's quite simple really; you only need to understand what std::remove does and how it does it. Hint: it does not remove elements from the vector. It moves then around instead, to the back of your collection. Moving an element in vector involves destroying original element. So this is where (part of) your destructor calls is coming from.

The other part is coming from temporary objects - since you passed an int (and not an instance of struct A) as a last parameter to std::remove, an instance of A has to be constructed for the purpose of comparison. If you want to force a little more discipline to your code, try to make it a habit to prefix one-parameter constructors with keyword explicit. It's very efficient at banishing such temporary objects. You will then have to create comparison object explicitly:

remove(v.begin(), v.end(), A(2));

Upvotes: 1

visitor
visitor

Reputation: 1801

std::remove is declared as

 template<typename ForwardIterator, typename Tp>
 ForwardIterator remove(ForwardIterator first, ForwardIterator last,
   const Tp& value);

In your usage, the third argument (2) is deduced as int. Because an int variable is not directly comparable against an A object, first a temporary has to be constructed for every

if (*it == value) ...

At the end of the comparison, the temporary is destroyed.

(All in all, you are having a hidden performance problem, resulting from having non-explicit single-argument, a.k.a. conversion constructor.)

Upvotes: 2

BЈовић
BЈовић

Reputation: 64253

The question is: why destructor is called 6 times while remove() was running?

Because std::remove just reorders the elements, but doesn't remove anything from the vector. In the process of reordering, some elements are copy-constructed.


Next code shows in details what happens :

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

struct A {
  A(int i = -1): i_(i) {cout << "I was created" << i_ << endl;}
  ~A() {
    cout << "I was destroyed" << i_ << endl;
  }
  A(const A& o):i_(o.i_)
  {
    cout << "I was copied" << i_ << endl;
  }
  A& operator=(const A& o)
  {
    i_=o.i_;
    cout << "I was assigned" << i_ << endl;
    return *this;
  }
  int get() { return i_; }
  bool operator==(A const& rhs) { return i_ == rhs.i_; }
private:
  int i_;
};

int main() {
std::cout<<"start"<<std::endl;
  A a[] = {1, 2, 3, 2, 4, 5};

  std::cout<<"creating"<<std::endl;
  vector<A> v(a, a + sizeof a/sizeof a[0]);
  std::cout<<"removing"<<std::endl;
  remove( v.begin(), v.end(), 2 );
  std::cout<<"end"<<std::endl;
}

The remove is placing "removed" elements at the end of the vector.

Upvotes: 3

Related Questions