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

[David Blaikie]

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)

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.

But, again, perhaps -Wparentheses fits in the "We'd rather not have this, if
it weren't for GCC's legacy/compatibility", since I don't know which
warnings fit & which ones don't, it's hard for me to generalize.

> 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.

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).

> 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.

It almost makes more sense in a world where C++0x isn't the default dialect
- if a user is opting in to C++0x, it's likely they're trying to use its
features/benefits and this is one of the low-hanging fruit (if it's easy to
do the migration, which a fixup would provide - check the code compiles,
suppress all warnings except the null pointer usage warning and recompile
with -fixit).

> 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 =


> > 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)"))

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?

>>
>> 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.

Thanks again for taking the time to help explain all this,
- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20110826/d717edab/attachment.html>