[Lldb-commits] [PATCH] D30054: Delete DataBufferMemoryMap

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

The main concern I have with adding null checks is that I think it actually *increases* your risk of crashing.  All of a sudden you've introduced an entirely new branch / code-path that is completely untested.  Worse, it only occurs in a situation where you've explicitly told the user not to do something, and they accidentally did it anyway.

As a general rule, you just can't protect against that, and you shouldn't try to because there are an infinite number of ways it could occur in practice.  It's the halting problem.  If you tell the user of an API to check a pointer for null before using the API, then you can make a ton of assumptions inside the API that greatly simplify things and increase test coverage.  Once you start propagating this kind of thing up through the API, tons of error checking and error handling code (which is all likely untested, and nobody understands how it can ever happen or if it's even possible to happen) just goes away because there is just no way for those errors to happen anymore.  (In fact, in many cases maybe the errors already can't happen, but there is so much propagating of null checks through various levels of the API that you can't really reason about whether it does or doesn't).

If an API documents that it never returns NULL, for example, then you don't need to check the return value for null.  Just like if an API documents that it doesn't accept null, then you do need to check the parameter you pass in.

BTW, not sure if this interests you, but C++ people have been working on the C++ Core Guidelines, some of which will eventually make it into the standard.  One of those things is a class called `not_null<T>` (link here <https://github.com/Microsoft/GSL/blob/master/include/gsl/gsl>) which restricts a pointer to be non null.  That would solve a lot of problems like this at the type system level, and the conversion will be much easier / less painful / less churn if we already know which pointers we can make assumptions about.


https://reviews.llvm.org/D30054