Re: [PATCH, C] Warn ordered comparison pointer with null pointer constant

[Shujing Zhao <pearly dot zhao at oracle dot com>]

On 04/30/2010 06:56 PM, Joseph S. Myers wrote:
> On Fri, 30 Apr 2010, Shujing Zhao wrote:
>
> >  If
> > > you also want to warn - not pedwarn, and so under -Wextra only - for ordered
> > > comparisons with a pointer-type null pointer constant, I think you should do
> > > so inside the case where comp_target_types has passed.
> > If warn ordered comparison of pinter with null pointer, I think besides the
> > void pointer with null pointer, the other pointer with null pointer need be
> > warned too.
>
> I can't make sense of what you are saying here, but this patch is also 
> wrong.  By turning the
>
>           if (comp_target_types (location, type0, type1))
>
> into an "else if", you are losing both the errors and pedwarns if 
> comp_target_types fails (for comparisons of distinct types) and those if 
> it succeeds (for comparison of pointers to complete and incomplete, or of 
> pointers to functions).  That is, your patch would cause cases that 
> violate constraints not to get the required errors with -pedantic-errors.
>
> As I said, put this *inside* the above conditional.  If something is more 
> seriously wrong - if the types don't match and so ISO C requires a 
> diagnostic - there is no need to try to give this additional optional 
> diagnostic; the user will need to fix the serious problems in their 
> program first before dealing with -Wextra issues.  Either make this new 
> check independent of the checks inside that conditional (complete and 
> incomplete, function pointers), or only give the new warning if neither of 
> the previous pedwarn conditions passed.
I agree with to warn the serious problems firstly.
Put this -Wextra inside the warning would warn the new warning "ordered 
comparison of pointer with null pointer"

void *p;
if ( p > (void *)0)

but would warn "comparison of distinct pointer types lacks a cast" for

extern void p()
if ( p > (void *)0)

since the types are not equivalent. My question is should the warnings to be 
kept consistent to warn the "null pointer" for the latter test case?
If it is yes, besides warning inside the "comp_target_types" conditional, a else 
if null_pointer_constant_p conditional would be needed before the "else" 
conditional.

Thanks
Pearly