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

[Douglas Gregor]

On Aug 26, 2011, at 1:39 PM, David Blaikie wrote:
> On Fri, Aug 26, 2011 at 11:56 AM, Douglas Gregor <dgregor at apple.com> wrote:
> >
> 
> > Yes, 0 should be acceptable as a pointer literal, as should 0L. This is
> > convention for a large number of projects and programmers.
> 
> Agreed for C++93.
> 
> For the C++11 case here's my attempt at a parallel to existing warnings -Wparentheses. When this warning was added, I assume lots of people would've had to change their code (or suppress the warning). It was added to help detect a common source of user error by imposing a new (well, probably partially adopted from some existing developers conventions, I assume), somewhat arbitrary (not a standard construct, just an arbitrary choice of syntax to indicate "I did this deliberately") syntax to express developer intent to the compiler (& other developers) so the compiler could more accurately detect possible errors.
> 
> foo.c:5:9: warning: using the result of an assignment as a condition without parentheses [-Wparentheses]
>   if (a = b)
>       ~~^~~
> That seems equivalent to suggesting NULL instead of 0 in pointer contexts in C++98, really. If you do that, then the compiler knows what you mean & can help you check that what you mean is what you actually got. (by checking NULL is used always and only in pointer contexts)

Two big differences here:

  1) if (a = b) is very likely to be an error
  2) GCC 4.3 introduced -Wparentheses with roughly the same semantics as Clang eventually implemented, so the convention was pre-established

> The C++11 case of suggesting nullptr seems even stronger than the case for -Wparentheses, in this case nullptr is a language-provided construct designed for the purpose.

… that has seen very little use in the field. The C++'0x standard hasn't been officially published yet, so it's way to early to use it as the basis for establishing convention.

> > You're assuming that programmers want to migrate all of their code to C++0x and its idioms. 
> 
> Not entirely - I'm assuming that it's the 'right' way to write C++0x and developers who want to write in some limited subset of C++0x can suppress it. If C++0x has been chosen, it makes sense to me to provide the best diagnostics we can for writing C++0x, not a subset of it. If using nullptr means we can diagnose more pointer mis-usage (indeed, as hard compiler errors, in fact - once you've switched to nullptr you won't just get a silence when you compare a pointer to 0 thinking it was an integer, you'll get an error) it seems we'd be doing a disservice to C++0x developers not to tell them they were being ambiguous & that the compiler won't be helping them as much as it could.

You're advocating a coding style that favors nullptr over 0. That's fine, and for a purely C++0x code base I *might* agree with you (although 0 is so much shorter than nullptr). However, the use of 0 rather than nullptr is not a good indication that there is actually a problem with the code, so we're outside the domain of what a compiler should do.

> It's probably one of the cheapest (especially with a fixup) migrations to make when compiling code as C++0x. The first 0x feature pretty much any compiler will implement, so I'm not sure many people would choose to compile as C++0x yet resist updating to a construct that helps diagnose bugs and expresses the author intent more clearly (to both other developers and the compiler itself).

Lots of code bases will be compiling both as C++98 and as C++0x for several years. I build Clang as C++0x with libc++; do you want me changing all of the 0's to nullptr's? The buildbots sure don't want me to.

> > For the other side of the equation, check out the warning + Fix-It for this code:
> >	struct Point { int x, y; } p = { x: 10 };
> 
> I agree, this is a much more compelling warning than the one I'm proposing - and I'm sure there are many other better/less ambiguous warnings than nullptr. My point is that there are many /less/ compelling warnings too (though I don't know which of those are consistent with your philosophy & which aren't).
> 
> Side note:
> /tmp/webcompile/_1251_0.c:1:37: warning: use of GNU old-style field designator extension [-Wgnu-designator]
> struct Point { int x, y; } p = { x: 10 };
>                                  ~~ ^
>                                  .x = 
> Shouldn't that ^ be below the x, not the 10? like this:
> 
> /tmp/webcompile/_1251_0.c:1:37: warning: use of GNU old-style field designator extension [-Wgnu-designator]
> struct Point { int x, y; } p = { x: 10 };
>                                  ^~ 
>                                  .x = 
Yes, that'd be better. Patch welcome :)

> > > I'd say all uses of NULL in non-pointer contexts should be caught.
> >
> > I'm fine with this.
> 
> Ok, great! I can do that, I think. Any idea how this would work with the existing -Wnull-arithmetic warning which is a subset of this? (it disallows "NULL + 2" and "NULL < 3" but doesn't stop "int i = NULL;" (or "void foo(int); foo(NULL)"))

It's easy to add diagnostic subgroups. -Wnull-arithmetic can be a subgroup of some new group covering NULL-as-integer warnings.

> Is a fixit to nullptr applicable here (in C++0x) similar to the diff I provided for the use of 'false' in pointer contexts to suggest nullptr in C++0x? Or should the output remain agnostic to null pointer literals in C++11?

I could live with suggesting nullptr in C++0x and NULL (if available) or 0 otherwise.

> >>
> >> 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.
> 
> Ok - though that won't help with C++0x warnings like I'm suggesting, unfortunately. I wonder if there are any major projects compiling as C++0x currently.

Well, you can compile LLVM and Clang as C++0x. 

	- Doug

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