[cfe-dev] null pointer literals, warnings, and fixups

[Douglas Gregor]

On Aug 26, 2011, at 11:37 AM, David Blaikie wrote:

>> Just because someone is compiling with C++0x doesn't mean they want to automatically upgrade their code to C++0x.
>> 
>> Sure - though warnings can be disabled by default and/or by users.
> 
> Off-by-default warnings are not a mechanism to subvert our normal processes for vetting a warning. In general, we should avoid off-by-default warnings: if it's not good enough to turn on by default, why do we have it at all? The vast majority of users will never see an off-by-default warning.
> 
> Hmm - I can see the concept, but it doesn't see to match up with my experience/understanding. I tend to compile with everything on (the old incantation with G++ was "-Wall -Wextra -pedantic -ansi") which provides a whole bunch of things over the baseline. It's just the case that this usage is really that rare? That most people compile without any warning flags at all?

Yes. The vast majority of projects I see compile without changing a single warning flag. And, as Ted's recent experiments with -Weverything have shown us, warnings that aren't turned on by default get very little testing coverage and tend to be broken (or break over time).

> I suppose I could believe that & most users aren't nearly so interested in correctness/portability as the language lawyers/academic purists/etc. Though if it's just a matter of adding a feature that'd be useful for some people & unseen by the man-on-the-street, that doesn't seem to dismiss the idea entirely. Then the only reason not to do it is cost (which I'm offering) & ongoing cost of having that added complexity in the compiler (which I agree, is something to be cautious about at every turn - I don't like adding unnecessary surface area if I can help it).

I am very concerned with the ongoing cost, and the accumulated drag on code side and compilation performance from having so many (typically unused) knobs.

>  
>> It seems like it would be pretty limiting to be entirely agnostic to 'better' ways of doing things until the old way is explicitly deprecated by the standard. 
> 
> This is an intentional and desirable limitation. A compiler is not a style checker, nor should it ever be. Now, if the warning is pointing out an actual problem that couldn't be caught before… that's something entirely different.
> 
>> Is it reasonable to generalize the existing checks into:
>> 
>> * using NULL in a non-pointer context (potentially still just leveraging GCC's __null). Special casing for arithmetic doesn't catch lots of other places. Is "int i = NULL;" (the more common "char c = NULL;" I suppose - which is arguable, perhaps? but seems to me as wrong as the other cases that already have NULL warnings) any more reasonable than "NULL < 3"?
> 
>> * using anything other than 0/NULL/nullptr in a pointer context? Special casing for boolean seems to be a bit overly narrow. What about char zero, for example? Though I suppose 0L might still be in the realm of "things people do intentionally", so perhaps a blacklist (boolean & char) is still preferred over a whitelist (0, NULL, nullptr)?
> 
> Any integral literal 0 seems like it should be allowed here. Otherwise, it seems entirely reasonable to warn about NULL vs 0 confusion, since this is a source of bugs in practice.
> 
> Sorry, I'm not quite following you here. Which cases of NULL vs 0 confusion do you think should be caught & which ones should be allowed? 
> 
> I take it you mean that integer 0 literals should be acceptable as pointer literals (agreed, at least in C++98 - Stroustrup's argument that it helps show you the reality/ambiguity in your code is sufficient for me to believe/understand that some people do this intentionally.

Yes, 0 should be acceptable as a pointer literal, as should 0L. This is convention for a large number of projects and programmers.

> But in C++11 I think it can do better & can help eliminate a source of bugs that use of the literal 0 or NULL can produce that cannot be caught in C++03 code because there is no better feature to describe the author's intent) but uses of NULL in non-pointer contexts should be caught.

You're assuming that programmers want to migrate all of their code to C++0x and its idioms. Perhaps when C++0x is ubiquitous and the majority of code we see if compiled as C++0x, this will make sense, but that's certainly not the case now. Perhaps in the future, (long?) after Clang has switched its default dialect to C++0x, we can make this assumption. 

For the other side of the equation, check out the warning + Fix-It for this code:

	struct Point { int x, y; } p = { x: 10 };

Here, we do have a Fix-It that migrates from (old, non-standard) GNU designated initializer syntax to the C99 syntax. There are two major differences between this warning and a warning that would replace "0" with "nullptr":

	1) The old syntax is not supported by any standard
	2) Clang defaults to the standard providing the new syntax (C99)
	3) The standard providing the new syntax has been available for a very long time and is widely implemented

> I'd say all uses of NULL in non-pointer contexts should be caught.

I'm fine with this.

> The question is whether such a warning will produce too much noise; the only way to figure that out is to implement it and run it across a pile of code.
> 
> Fair enough. Is there an appropriate reference set/process that should be used - is this the sort of thing where we would add the warning, on by default, see what the buildbots/etc cough up, or is there some way to replicate all that test coverage locally to avoid randomizing the bots/developers?

There's no reference set, save LLVM and Clang itself. Different people use their own favored projects to see how a warning fares.

	- Doug

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110826/5573cbe7/attachment.html>