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

David Blaikie dblaikie at gmail.com
Fri Aug 26 15:40:45 PDT 2011


>
>   1) if (a = b) is very likely to be an error
>

Fair enough - not the best example, I'll admit.

In the case of NULL/0 usage we agree that some uses of NULL in non-pointer
contexts could be errors & we can help developers find bugs by suggesting
that the thing they're using NULL on isn't really a pointer & since they
used NULL we think they thought that they were talking about a pointer.
Likely to be an error.

But once they start using 0 we don't know. Some developers deliberately use
0 when they mean "null pointer" but we can't guess when they meant to & when
they didn't. If we suggest they use nullptr then we know & they'll get
useful error messages when they intended to use a pointer & weren't actually
using a pointer.


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


That seems to me to be more than a stylistic issue - that's a "we're not
sure what you intended here so you're not getting the helpful diagnostics
you could be if you were providing more semantics to the compiler" (the
helpful diagnostics we're already providing for nullptr and NULL usage where
we know what you meant - so we already have precedent that shows these
things are helpful/have some value).

I know GCC and Clang provide great diagnostics for NULL (could be better, we
agree & we can fix that) and nullptr usage - but all the value of that is
lost without any warning as soon as someone uses a 0 null pointer literal.
Seems to very easily defeat the value we agree exists in those diagnostics.


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

Indeed - in that case I think it'd be reasonable to suppress any warnings
about "old" C++98 constructs that have better C++0x equivalents, such as
nullptr. (I could still see having a warning that suggests NULL for pointer
literals over 0, so as to help developers recognize when their intent
differed from the reality of the code - in the "I want to build as C++0x and
C++98" scenario then you'd suppress the nullptr warning, but you could leave
the NULL mismatch warning on)

Of course I can try implementing this myself & running it on llvm/clang,
though it'll be hard to say if it finds any bugs since replacing 0 with NULL
won't tell me if the developer originally thought they were passing an
integer parameter. I don't know all the code/call sites well enough to be
able to judge that. But I would think going forward it could have value by
telling the developer that what they thought was an integer parameter was
really a pointer parameter (& perhaps they shouldn't be passing null, say).

This seems quite different from stylistic issues such as bracing, line
length, naming, or even the other thread about boolean testability (which
way should you test a null pointer: if (x), if(x == NULL), etc...).


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

Sent.


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

Sounds good.


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

Yep - and I've found some precedent for that in the uninitialized variable
warning code (sent out a related fix there, too) - perhaps this code that
decides on null pointer literals could be refactored out into some common
location.

[as mentioned in the code review email, the logic that detects if NULL is
defined is somewhat simplistic - I guess it takes into account the state of
macros at the end of parsing of the file so if NULL is defined after the
usage, it is still suggested (or if it's undefined later on, it is never
suggested). But I guess that's OK. It hardly seems practical to fix it]


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

Do you just set CXXFLAGS to -std=c++0x when you run make, then? I'll have to
give that a go.

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


More information about the cfe-dev mailing list