[cfe-dev] [PATCH] -Wconversion-null

[David Blaikie]

On Thu, Mar 15, 2012 at 3:25 AM, James K. Lowden
<jklowden at schemamania.org> wrote:
> On Thu, 15 Mar 2012 09:21:31 +0100
> Lubos Lunak <l.lunak at suse.cz> wrote:
>
>> > #undef NULL
>> > #define NULL 0
>> >
>> > The code you presented is valid C++ and the proposed warnings are
>> > all misplaced.
>>
>>  It is valid C++, but that is why it is a warning and not an error.
>> All warnings are about code which is technically valid, yet is (more
>> or less) likely a mistake.
>
> No, I can't agree.  The purpose of a warning is to indicate when a
> particular operation or construction might be unsafe.  A warning about
> something that cannot possibly be an error (as far as the machine is
> concerned) is just noise.

If by "error" you mean undefined behavior, then that doesn't seem to
be the philosophy that Clang (or any other compiler that I know of)
has taken in its warning (or even error) design. The vast majority of
warnings in compilers are for code with well defined but surprising
semantics.

> You are diving here into the semantics of the name "NULL", and it's
> hopeless.  If I say
>
>        #define NOPTR NULL
> or
>        typedef NULL NOPTR;
>
> And then use NOPTR instead, have I not circumvented your warning and
> produced identical machine code?

The first example would not circumvent the warning (since it is based
on the magic __null implementation detail that NULL is defined as -
any indirection to it will still trigger the warning).
The second example would not compile in the presence of an existing
definition of NULL, and would not provide the same semantics as
standard NULL (since it's a type, not a value) in the absence fo the
standard NULL.

> Is the resulting source code any "(more or less) likely a mistake"?

Nope - in the functioning (first) example I'd say it's no less likely
a mistake - NULL is defined in C++ as follows:

18.2[support.types]p3

"The macro NULL is an implementation-defined C++ null pointer constant
in this International Standard
(4.10). foot 193"

(where foot 193 (non-normatively) says: "Possible definitions include 0
and 0L, but not (void*)0.")

Indeed, in C++11 it's even conforming to have NULL defined by the
implementation as nullptr - so any use of NULL as anything other than
a null pointer literal is at best non-portable, and at most represents
confusion/a mistake by the author.

Consider:

int *i;
...
*i = NULL;

Perhaps the user didn't intend to dereference the pointer?
Effectively, with good warnings, we should be able to make NULL as
typesafe as the new C++11 hotness, nullptr which is a proper type safe
null pointer.

> As the standard doesn't define NULL,

The C++ standard does define NULL.

> the user is free to define it at will.

That doesn't seem to be the case, per 17.6.4.3.1[macro.names]p1: "A
translation unit that includes a standard library header shall not
#define or #undef names declared in any standard library header."

> Since the compiler can't know what "NULL" means to the human,
> it can't offer any advice about it.

For what it's worth, GCC has been providing this advice for a couple
of major versions now.

- David