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

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

On 04/29/2010 08:19 PM, Joseph S. Myers wrote:
> On Thu, 29 Apr 2010, Shujing Zhao wrote:
>
> > Hi,
> > As Manuel's reply at http://gcc.gnu.org/ml/gcc-patches/2010-04/msg01425.html,
> > this patch is to change the warning of (z >= (void *)0) as same as the warning
> > of (z >= 0), if z is a pointer.
> >
> > Tested on i686-pc-linux-gnu.
> > Is it ok for trunk?
>
> This patch is not correct.  Ordered comparisons of pointers to void are 
> valid and so must not receive a pedwarn (must not receive an error with 
> -pedantic-errors).
>
> The existing code for these warnings about comparison with integer zero is 
> certainly problematic - it makes no sense for
>
> void *p;
> void
> f (void)
> {
>   (void) (p > 0);
>   (void) (0 > p);
> }
>
> to get only one warning with -Wextra but two with -pedantic.  (Both 
> alternatives should pedwarn if -pedantic, otherwise warn if -Wextra.) 
Ok. Add -Wextra warn for 0 > p.
 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.
> This sort of thing should best get four copies of any testcase (options 
> "", "-pedantic", "-pedantic-errors", "-Wextra"), to verify properly what 
> form of diagnostic is given in each case.
Add four test cases at the updated patch. Retested on i686-pc-linux-gnu.
Is it OK?

Thanks
Pearly

2010-04-30  Shujing Zhao  <pearly.zhao@oracle.com>

	* c-typeck.c (build_binary_op): Warn ordered comparison of pointer
	with null pointer and also warn about ordered comparison of zero
	with pointer if -Wextra.

Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 158919)
+++ c-typeck.c	(working copy)
@@ -9614,8 +9614,20 @@ build_binary_op (location_t location, en
 	  addr_space_t as0 = TYPE_ADDR_SPACE (TREE_TYPE (type0));
 	  addr_space_t as1 = TYPE_ADDR_SPACE (TREE_TYPE (type1));
 	  addr_space_t as_common;
-
-	  if (comp_target_types (location, type0, type1))
+	  
+	  if (null_pointer_constant_p (orig_op1))
+	    {
+	      result_type = type0;
+	      warning_at (location, OPT_Wextra,
+			  "ordered comparison of pointer with null pointer");
+	    }
+	  else if (null_pointer_constant_p (orig_op0))
+	    {
+	      result_type = type1;
+	      warning_at (location, OPT_Wextra,
+			  "ordered comparison of pointer with null pointer");
+	    }
+	  else if (comp_target_types (location, type0, type1))
 	    {
 	      result_type = common_pointer_type (type0, type1);
 	      if (!COMPLETE_TYPE_P (TREE_TYPE (type0))
@@ -9649,13 +9661,17 @@ build_binary_op (location_t location, en
 		     "ordered comparison of pointer with integer zero");
 	  else if (extra_warnings)
 	    warning_at (location, OPT_Wextra,
-		     "ordered comparison of pointer with integer zero");
+			"ordered comparison of pointer with integer zero");
 	}
       else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
 	{
 	  result_type = type1;
-	  pedwarn (location, OPT_pedantic,
-		   "ordered comparison of pointer with integer zero");
+	  if (pedantic)
+	    pedwarn (location, OPT_pedantic,
+		     "ordered comparison of pointer with integer zero");
+	  else if (extra_warnings)
+	    warning_at (location, OPT_Wextra,
+			"ordered comparison of pointer with integer zero");
 	}
       else if (code0 == POINTER_TYPE && code1 == INTEGER_TYPE)
 	{
Index: testsuite/gcc.dg/ordered-comparison-1.c
===================================================================
--- testsuite/gcc.dg/ordered-comparison-1.c	(revision 0)
+++ testsuite/gcc.dg/ordered-comparison-1.c	(revision 0)
@@ -0,0 +1,16 @@
+/* Test warning for ordered comparison pointer with null pointer constant. */
+/* Tested with no warning option. */
+/* { dg-do compile } */
+/* { dg-options "" } */
+void z();
+
+void f() {
+ if ( z >= 0 )
+   z();
+ if ( z >= (void*)0 )
+    z();
+ if ( 0 >= z)
+    z();
+ if ( (void*)0 >= z)
+    z();
+}
Index: testsuite/gcc.dg/ordered-comparison-2.c
===================================================================
--- testsuite/gcc.dg/ordered-comparison-2.c	(revision 0)
+++ testsuite/gcc.dg/ordered-comparison-2.c	(revision 0)
@@ -0,0 +1,16 @@
+/* Test warning for ordered comparison pointer with null pointer constant. */
+/* Tested with -pedantic. */
+/* { dg-do compile } */
+/* { dg-options "-pedantic" } */
+void z();
+
+void f() {
+ if ( z >= 0 ) /* { dg-warning "ordered comparison of pointer with" } */
+   z();
+ if ( z >= (void*)0 )
+    z();
+ if ( 0 >= z) /* { dg-warning "ordered comparison of pointer with" } */
+    z();
+ if ( (void*)0 >= z)
+    z();
+}
Index: testsuite/gcc.dg/ordered-comparison-3.c
===================================================================
--- testsuite/gcc.dg/ordered-comparison-3.c	(revision 0)
+++ testsuite/gcc.dg/ordered-comparison-3.c	(revision 0)
@@ -0,0 +1,16 @@
+/* Test warning for ordered comparison pointer with null pointer constant. */
+/* Test with -pedantic-errors. */
+/* { dg-do compile } */
+/* { dg-options "-pedantic-errors" } */
+void z();
+
+void f() {
+ if ( z >= 0 ) /* { dg-error "ordered comparison of pointer with" } */
+   z();
+ if ( z >= (void*)0 )
+    z();
+ if ( 0 >= z) /* { dg-error "ordered comparison of pointer with" } */
+    z();
+ if ( (void*)0 >= z)
+    z();
+}
Index: testsuite/gcc.dg/ordered-comparison-4.c
===================================================================
--- testsuite/gcc.dg/ordered-comparison-4.c	(revision 0)
+++ testsuite/gcc.dg/ordered-comparison-4.c	(revision 0)
@@ -0,0 +1,16 @@
+/* Test warning for ordered comparison pointer with null pointer constant. */
+/* Test with -Wextra. */
+/* { dg-do compile } */
+/* { dg-options "-Wextra" } */
+void z();
+
+void f() {
+ if ( z >= 0 ) /* { dg-warning "ordered comparison of pointer with" } */
+   z();
+ if ( z >= (void*)0 ) /* { dg-warning "ordered comparison of pointer with null pointer" } */
+    z();
+ if ( 0 >= z) /* { dg-warning "ordered comparison of pointer with" } */
+    z();
+ if ( (void*)0 >= z) /* { dg-warning "ordered comparison of pointer with null pointer" } */
+    z();
+}