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

[Douglas Gregor]

On Aug 30, 2011, at 7:37 PM, David Blaikie wrote:
> On Mon, Aug 29, 2011 at 5:46 PM, Douglas Gregor <dgregor at apple.com> wrote:
> 
> On Aug 21, 2011, at 9:18 PM, David Blaikie wrote:
> 
> > As a small indirect first attempt I've included a patch that:
> >
> > 1) changes "NULL" to "null" in "initialization of pointer of type %0
> > to NULL from a constant boolean expression" - since NULL is a specific
> > macro but the general concept is simply a "null pointer".
> 
> Looks good.
> 
> Hmm - I see you checked in the fixit fix, but not this text change. Was there a reason? Good thing in any case, since I'd failed to update the tests. The attached patch includes the updated tests.

I... think there was some redundancy in messages that confused me. The change to the message text is good.

> [though these test cases surprise me a bit - I hadn't realized that any integral zero constant (not just literals) were valid null pointer literals. Sounds like another case for a warning. Seems unlikely anyone's legitimately writing "const int null = 0; int* i = null;" are they? Well, I'll see about handling that in the overall/whitelisting way. Accept zero integer literals, NULL, and nullptr as null pointers & warn on anything else]
>  
> 
> > 2) adds a fixup for this warning in C++0x to suggest using nullptr
> >
> > I think there's still a bunch of things worth discussing about when to
> > suggest nullptr and what to suggest in C++98 (0, NULL, nothing at
> > all?) & I'd like to apply this kind of suggestion/fixup in a much more
> > general way (to all null pointer literals that aren't nullptr, not
> > just the false boolean literal) - but I thought this might be a
> > moderately easy experiment, even if it's throwaway, and might be a
> > more concrete thing to start such discussions.
> 
> 
> Seems like we should suggest nullptr, NULL, or 0, depending on which dialect we're in, as we've done elsewhere.
> 
> Agreed - I hadn't seen the smarts to do this when I sent this patch out, & I stumbled across it in the uninitialized variable case afterwards. 
> 
> I've put a public utility function in Sema to produce a StringRef of the appropriate null pointer literal & used that in both cases.


> Looking at other similar cases I came across a case in ~line 250, SemaExpr.cpp - handling missing null sentinel nodes (in ObjC/C++ I assume). I don't fully understand the last case there, though - that chooses between "0" and "0L". Also, this seems inconsistent with the (matching) null case in SemaCodeComplete.cpp which suggests "nil", "NULL", or "(void*)0".

Ick. The reason for 0L is that, when we're dealing with C-style varargs, we need to make sure that the 0 value we suggest is the same length as a pointer. It would be good to give getNullPointerLiteral() the ability to produce 0L when needed, perhaps with a configuration option.

> What would be the right set of suggestions for null pointer literals across all clang languages (C needs the cast (or NULL), objc prefers nil over NULL - should it prefer nil over nullptr in C++0x? (nil > nullptr > NULL > 0))?

nil > nullptr > NULL > 0 is reasonable.

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110907/8efe2a19/attachment.html>