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

[David Blaikie]

On Wed, Sep 7, 2011 at 3:29 PM, Douglas Gregor <dgregor at apple.com> wrote:

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

Alrighty - hope it's OK that I took this as "reviewed" & committed the
diagnostic message change by itself as 139465.


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

Oh, tricky. In that case we can't reliably suggest a non-macro null pointer
literal then, can we? Because then the code would be non-portable between 64
and 32 bit (etc) builds.

That sort of leaves the (void*)0 as the 'right' solution for a portable,
size-appropriate, non-macro'd, non-C++0x literal. Or is there another way?


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

OK.

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110909/7985cb0f/attachment.html>