Deleting a element from a vector of pointers in C++.

Question

I remember hearing that the following code is not C++ compliant and was hoping someone with much more C++ legalese than me would be able to confirm or deny it.

std::vector<int*> intList;
intList.push_back(new int(2));
intList.push_back(new int(10));
intList.push_back(new int(17));

for(std::vector<int*>::iterator i = intList.begin(); i != intList.end(); ++i) {
  delete *i;
}
intList.clear()

The rationale was that it is illegal for a vector to contain pointers to invalid memory. Now obviously my example will compile and it will even work on all compilers I know of, but is it standard compliant C++ or am I supposed to do the following, which I was told is in fact the standard compliant approach:

while(!intList.empty()) {
  int* element = intList.back();
  intList.pop_back();
  delete element;
}

Answer 1

You code is valid, but the better solution will be to use smart pointers.

The thing is that all requirements to std::vector are located in 23.2.4 section of C++ Standard. There're no limitations about invalid pointers. std::vector works with int* as with any other type (we doesn't consider the case of vector<bool>), it doesn't care where they are point to.

Answer 2

Ultimately, this is a question of personal taste more than anything. It's not "standards non-compliant" to have a vector that contains invalid pointers, but it is dangerous, just like it's dangerous to have any pointer that points to invalid memory. Your latter example will ensure that your vector never contains a bad pointer, yes, so it's the safest choice.

But if you knew that the vector would never be used during your former example's loop (if the vector is locally scoped, for example), it's perfectly fine.

Answer 3

Where did you hear that? Consider this:

std::vector<int *> intList(5);

I just created a vector filled with 5 invalid pointers.

Answer 4

Your code is fine. If you're worried for some reason about the elements being invalid momentarily, then change the body of the loop to

int* tmp = 0;
swap (tmp, *i);
delete tmp;

Answer 5

In storing raw pointers in a container (I wouldn't recommend this) then having to do a 2 phase delete, I would choose your first option over the second.

I believe container::clear() will delete the contents of the map more efficiently than popping a single item at a time.

You could probably turn the for loop into a nice (psuedo) forall(begin(),end(),delete) and make it more generic so it didn't even matter if you changed from vector to some other container.

Answer 6

The C++ philosophy is to allow the programmer as much latitude as possible, and to only ban things that are actually going to cause harm. Invalid pointers do no harm in themselves, and therefore you can have them around freely. What will cause harm is using the pointer in any way, and that therefore invokes undefined behavior.

Answer 7

I don't believe this is an issue of standards compliance. The C++ standards define the syntax of the language and implementation requirements. You are using the STL which is a powerful library, but like all libraries it is not part of C++ itself...although I guess it could be argued that when used aggressively, libraries like STL and Qt extend the language into a different superset language.

Invalid pointers are perfectly compliant with the C++ standards, the computer just won't like it when you dereference them.

What you are asking is more of a best practices question. If your code is multi-threaded and intList is potentially shared, then your first approach may be more dangerous, but as Greg suggested if you know that intList can't be accessed then the first approach may be more efficient. That said, I believe safety should usually win in a trade-off until you know there is a performance problem.

As suggested by the Design by Contract concept, all code defines a contract whether implicit or explicit. The real issue with code like this is what are you promising the user: preconditions, postconditions, invariants, etc. The libraries make a certain contract and each function you write defines its own contract. You just need to pick the appropriate balance for you code, and as long as you make it clear to the user (or yourself six months from now) what is safe and what isn't, it will be okay.

If there are best practices documented with with an API, then use them whenever possible. They probably are best practices for a reason. But remember, a best practice may be in the eye of the beholder...that is they may not be a best practice in all situations.

Answer 8

it is illegal for a vector to contain pointers to invalid memory

This is what the Standard has to say about the contents of a container:

(23.3) : The type of objects stored in these components must meet the requirements of CopyConstructible types (20.1.3), and the additional requirements of Assignable types.

(20.1.3.1, CopyConstructible) : In the following Table 30, T is a type to be supplied by a C + + program instantiating a template, t is a value of type T, and u is a value of type const T.

expression  return type  requirement
xxxxxxxxxx    xxxxxxxxxxx  xxxxxxxxxxxxxxxxxxxxxxxxxxxxx
T(t)                       t is equivelant to T(t)
T(u)                       u is equivelant to T(u)
t.~T()      
&t          T*           denotes the address of t
&u          const T*     denotes the address of u

(23.1.4, Assignable) : 64, T is the type used to instantiate the container, t is a value of T, and u is a value of (possibly const) T.

expression  return type  requirement
xxxxxxxxxx    xxxxxxxxxxx  xxxxxxxxxxxxxxxxxxxxxxxxxxxxx
t = u         T&           t is equivilant to u

That's all that is says about the contents of an STL collection. It says nothing about pointers and it is particularly silent about the pointers pointing to valid memory.

Therefore, deleteing pointers in a vector, while most likely a very bad architectural decision and an invitation to pain and suffering with the debugger at 3:00 AM on a Saturday night, is perfectly legal.

EDIT:

Regarding Kranar's comment that "assigning a pointer to an invalid pointer value results in undefined behavior." No, this is incorrect. This code is perfectly valid:

Foo* foo = new Foo();
delete foo;
Foo* foo_2 = foo;  // This is legal

What is illegal is trying to do something with that pointer (or foo, for that matter):

delete foo_2; // UB
foo_2->do_something(); // UB
Foo& foo_ref = *foo_2; // UB

Simply creating a wild pointer is legal according to the Standard. Probably not a good idea, but legal nonetheless.

EDIT2:

More from the Standard regarding pointer types.

So sayeth the Standard (3.9.2.3) :

... A valid value of an object pointer type represents either the address of a byte in memory (1.7) or a null pointer (4.10)...

...and regarding "a byte in memory," (1.7.1) :

The fundamental storage unit in the C + + memory model is the byte. A byte is at least large enough to contain any member of the basic execution character set and is composed of a contiguous sequence of bits, the number of which is implementation-defined. The least significant bit is called the low-order bit; the most significant bit is called the high-order bit. The memory available to a C + + program consists of one or more sequences of contiguous bytes. Every byte has a unique address.

There is nothing here about that byte being part of a living Foo, about you having access to it, or anything of the sort. Its just a byte in memory.