[Libreoffice] [PUSHED, partial] Remove NULL checks from delete

[Stephan Bergmann]

On 11/10/2011 12:05 AM, Andrew Douglas Pitonyak wrote:
> I would feel safer if pointers were set to NULL (or nullptr if we
> support C++11) since it is not safe to delete a pointer twice.

I would rather argue that adding redundant "p = 0;" following a "delete 
p;" makes our code worse, not better (even if that may sound paradoxical 
at first):

For code to be maintainable, you need to be able to reason about it. 
One important concept here is to abstract over the typically huge space 
of potential program states, by introducing invariants.

For each use of a pointer in a C/C++ program text, an important piece of 
information is whether the pointer (a) is guaranteed to always point to 
valid memory when execution of the program passes that textual 
occurrence, (b) is guaranteed to never point to valid memory, or (c) can 
point to either valid or invalid memory.  It is highly desirable to be 
able to attach an invariant to each such textual occurrence.  Either the 
invariant that the pointer is guaranteed to always point to valid memory 
(so that, e.g., the pointer can be dereferenced), or the invariant that 
it is guaranteed to always not point to valid memory (so that, e.g., the 
pointer can be assigned a new value without having to delete its old 
value first).

If such an invariant can be attached to each occurrence of a given 
pointer, there is obviously no need to null the pointer after deletion.

However, if you do add that nulling, people will start to wonder what it 
is good for.  They will assume that it is there for a reason, that from 
this textual occurrence of nulling the pointer other occurrences of 
using that pointer in the program text can be reached for which it 
cannot be guaranteed that the pointer always either points to valid or 
invalid memory.  So that those other occurrences need a way to check 
what state the pointer is in before they dereference it, by first 
comparing the pointer against null, like

   if (p) {
      p->foo();
   } else {
      ???
   }

So the code is made more complex for no reason.  People will stop to 
understand the invariants that were once attached to each occurrence of 
p, and will instead assume that at each occurrence of p the worst will 
have to be assumed and catered for.  But what to put into that else 
branch?  Defensive programming perversion.  It won't be long until the 
first bug is introduced that goes unnoticed because it is hidden by one 
of those "defensive" else branches.

Stephan