This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH C] PR32207


Hi,
This patch is to fix pr32207. For the test case:

extern void z();
void f() { if ( z ) z(); }
void g() { if ( z != 0 ) z(); }
void h() { if ( z != (void*)0 ) z(); }

The warning are:
t.c: In function ‘f’:
t.c:2:17: warning: the address of ‘z’ will always evaluate as ‘true’
t.c: In function ‘g’:
t.c:3:19: warning: the comparison will always evaluate as 'true' for the address of ‘z’ will never be NULL
t.c: In function ‘h’:
t.c:4:19: warning: the comparison will always evaluate as 'true' for the address of ‘z’ will never be NULL


The warning message for `z!=0` and `z!=(void*)0` is changed from "the address of ‘z’ will never be NULL" to "the comparison will always evaluate as 'true' for the address of ‘z’ will never be NULL". Because for the users, the second and the third conditional expression are comparison expression, and the warning message location is focus on the operator "!=" or "==". Consider the user end and the location, I choose to add "the comparison ..." to the warning message. Is it ok?

Some test cases are adjusted expected warning.
Test on i686-pc-linux-gnu without regression.

Ok for trunk?

Thanks
Pearly
/gcc
2010-04-23  Shujing Zhao  <pearly.zhao@oracle.com>

	PR c/32207
	* c-typeck.c (build_binary_op): Move forward check for comparison
	pointer with null pointer constant and adjust the diagnostic message.

/gcc/testcase
2010-04-23  Shujing Zhao  <pearly.zhao@oracle.com>

	* gcc.dg/pr32207.c: New test.
	* gcc.dg/misc-column.c: Adjust expected warning.
	* gcc.dg/Walways-true-1.c: Likewise.
	* gcc.dg/Walways-true-2.c: Likewise.
	* gcc.dg/warn-addr-cmp.c: Likewise.

Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 158539)
+++ c-typeck.c	(working copy)
@@ -9470,6 +9470,46 @@ build_binary_op (location_t location, en
 	  && (code1 == INTEGER_TYPE || code1 == REAL_TYPE
 	      || code1 == FIXED_POINT_TYPE || code1 == COMPLEX_TYPE))
 	short_compare = 1;
+      else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
+	{
+	  if (TREE_CODE (op0) == ADDR_EXPR
+	      && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
+	    {
+	      if (code == EQ_EXPR)
+		warning_at (location,
+			    OPT_Waddress,
+			    "the comparison will always evaluate as 'false' "
+			    "for the address of %qD will never be NULL",
+			    TREE_OPERAND (op0, 0));
+	      else
+		warning_at (location,
+			    OPT_Waddress,
+			    "the comparison will always evaluate as 'true' "
+			    "for the address of %qD will never be NULL",
+                            TREE_OPERAND (op0, 0));
+	    }
+	  result_type = type0;
+	}
+      else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
+	{
+	  if (TREE_CODE (op1) == ADDR_EXPR
+	      && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
+	    {
+	      if (code == EQ_EXPR)
+		warning_at (location,
+			    OPT_Waddress, 
+			    "the comparison will always evaluate as 'false' "
+			    "for the address of %qD will never be NULL",
+                            TREE_OPERAND (op1, 0));
+	      else
+		warning_at (location,
+			    OPT_Waddress,
+			    "the comparison will always evaluate as 'true' "
+			    "for the address of %qD will never be NULL",
+                            TREE_OPERAND (op1, 0));
+	    }
+	  result_type = type1;
+	}
       else if (code0 == POINTER_TYPE && code1 == POINTER_TYPE)
 	{
 	  tree tt0 = TREE_TYPE (type0);
@@ -9483,10 +9523,6 @@ build_binary_op (location_t location, en
 	     and both must be object or both incomplete.  */
 	  if (comp_target_types (location, type0, type1))
 	    result_type = common_pointer_type (type0, type1);
-	  else if (null_pointer_constant_p (orig_op0))
-	    result_type = type1;
-	  else if (null_pointer_constant_p (orig_op1))
-	    result_type = type0;
 	  else if (!addr_space_superset (as0, as1, &as_common))
 	    {
 	      error_at (location, "comparison of pointers to "
@@ -9518,24 +9554,6 @@ build_binary_op (location_t location, en
 			      (build_qualified_type (void_type_node, qual));
 	    }
 	}
-      else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
-	{
-	  if (TREE_CODE (op0) == ADDR_EXPR
-	      && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
-	    warning_at (location,
-			OPT_Waddress, "the address of %qD will never be NULL",
-			TREE_OPERAND (op0, 0));
-	  result_type = type0;
-	}
-      else if (code1 == POINTER_TYPE && null_pointer_constant_p (orig_op0))
-	{
-	  if (TREE_CODE (op1) == ADDR_EXPR
-	      && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
-	    warning_at (location,
-			OPT_Waddress, "the address of %qD will never be NULL",
-			TREE_OPERAND (op1, 0));
-	  result_type = type1;
-	}
       else if (code0 == POINTER_TYPE && code1 == INTEGER_TYPE)
 	{
 	  result_type = type0;
Index: testsuite/gcc.dg/warn-addr-cmp.c
===================================================================
--- testsuite/gcc.dg/warn-addr-cmp.c	(revision 158539)
+++ testsuite/gcc.dg/warn-addr-cmp.c	(working copy)
@@ -36,13 +36,13 @@ test_func_cmp (void)
 int
 test_func_cmp_rhs_zero (void)
 {
-  if (func == 0)     /* { dg-warning "the address of 'func'" } */
+  if (func == 0)     /* { dg-warning "the comparison will always evaluate to 'false'" } */
     return 1;
-  if (func != 0)     /* { dg-warning "the address of 'func'" } */
+  if (func != 0)     /* { dg-warning "the comparison will always evaluate to 'true'" } */
     return 1;
-  if (&var == 0)     /* { dg-warning "the address of 'var'" } */
+  if (&var == 0)     /* { dg-warning "the comparison will always evaluate to 'false'" } */
     return 1;
-  if (&var != 0)     /* { dg-warning "the address of 'var'" } */
+  if (&var != 0)     /* { dg-warning "the comparison will always evaluate to 'true'" } */
     return 1;
   if (weak_func == 0)
     return 1;
@@ -59,13 +59,13 @@ test_func_cmp_rhs_zero (void)
 int
 test_func_cmp_lhs_zero (void)
 {
-  if (0 == func)     /* { dg-warning "the address of 'func'" } */
+  if (0 == func)     /* { dg-warning "the comparison will always evaluate to 'false'" } */
     return 1;
-  if (0 != func)     /* { dg-warning "the address of 'func'" } */
+  if (0 != func)     /* { dg-warning "the comparison will always evaluate to 'true'" } */
     return 1;
-  if (0 == &var)     /* { dg-warning "the address of 'var'" } */
+  if (0 == &var)     /* { dg-warning "the comparison will always evaluate to 'false'" } */
     return 1;
-  if (0 != &var)     /* { dg-warning "the address of 'var'" } */
+  if (0 != &var)     /* { dg-warning "the comparison will always evaluate to 'true'" } */
     return 1;
   if (0 == weak_func)
     return 1;
Index: testsuite/gcc.dg/Walways-true-1.c
===================================================================
--- testsuite/gcc.dg/Walways-true-1.c	(revision 158539)
+++ testsuite/gcc.dg/Walways-true-1.c	(working copy)
@@ -26,32 +26,32 @@ bar (int a)
     foo (5);
   if (&&lab)	/* { dg-warning "7:always evaluate as" "correct warning" } */
     foo (6);
-  if (foo == 0)	/* { dg-warning "11:never be NULL" "correct warning" } */
+  if (foo == 0)	/* { dg-warning "11:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (7);
   if (foo (1) == 0)
     foo (8);
-  if (&i == 0)	/* { dg-warning "10:never be NULL" "correct warning" } */
+  if (&i == 0)	/* { dg-warning "10:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (9);
   if (i == 0)
     foo (10);
-  if (&a == 0)	/* { dg-warning "10:never be NULL" "correct warning" } */
+  if (&a == 0)	/* { dg-warning "10:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (11);
   if (a == 0)
     foo (12);
-  if (&&lab == 0) /* { dg-warning "13:never be NULL" "correct warning" } */
+  if (&&lab == 0) /* { dg-warning "13:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (13);
-  if (0 == foo)	/* { dg-warning "9:never be NULL" "correct warning" } */
+  if (0 == foo)	/* { dg-warning "9:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (14);
   if (0 == foo (1))
     foo (15);
-  if (0 == &i)	/* { dg-warning "9:never be NULL" "correct warning" } */
+  if (0 == &i)	/* { dg-warning "9:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (16);
   if (0 == i)
     foo (17);
-  if (0 == &a)	/* { dg-warning "9:never be NULL" "correct warning" } */
+  if (0 == &a)	/* { dg-warning "9:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (18);
   if (0 == a)
     foo (19);
-  if (0 == &&lab) /* { dg-warning "9:never be NULL" "correct warning" } */
+  if (0 == &&lab) /* { dg-warning "9:the comparison will always evaluate to 'false'" "correct warning" } */
     foo (20);
 }
Index: testsuite/gcc.dg/pr32207.c
===================================================================
--- testsuite/gcc.dg/pr32207.c	(revision 0)
+++ testsuite/gcc.dg/pr32207.c	(revision 0)
@@ -0,0 +1,9 @@
+/* Test warning for comparison non-null address with null pointer constant. */
+/* Origin: Pawel Sikora <pluto@agmk.net>*/
+/* { dg-do compile } */
+/* { dg-options "-Waddress" } */
+extern void z();
+
+void f() { if ( z ) z(); } /* { dg-warning "always evaluate as" } */
+void g() { if ( z != 0 ) z(); } /* { dg-warning "the comparison will always evaluate to 'true'" } */
+void h() { if ( z != (void*)0 ) z(); } /* { dg-warning "the comparison will always evaluate to 'true'" } */
Index: testsuite/gcc.dg/misc-column.c
===================================================================
--- testsuite/gcc.dg/misc-column.c	(revision 158539)
+++ testsuite/gcc.dg/misc-column.c	(working copy)
@@ -19,7 +19,7 @@ void foo (void)
   if (p < q) /* { dg-warning "9:comparison of distinct pointer types" } */
     bar ();
 
-  if (&p == 0) /* { dg-warning "10:will never be NULL" } */
+  if (&p == 0) /* { dg-warning "10:comparison will always evaluate to 'false'" } */
     bar();
 
   if (p == 4) /* { dg-warning "9:comparison between pointer and integer" } */
Index: testsuite/gcc.dg/Walways-true-2.c
===================================================================
--- testsuite/gcc.dg/Walways-true-2.c	(revision 158539)
+++ testsuite/gcc.dg/Walways-true-2.c	(working copy)
@@ -37,11 +37,11 @@ bar (int a)
     foo (9);
   if (i == 0)
     foo (10);
-  if (&a == 0)	/* { dg-warning "never be NULL" "correct warning" } */
+  if (&a == 0)	/* { dg-warning "the comparison will always evaluate to 'false'" "correct warning" } */
     foo (11);
   if (a == 0)
     foo (12);
-  if (&&lab == 0) /* { dg-warning "never be NULL" "correct warning" } */
+  if (&&lab == 0) /* { dg-warning "the comparison will always evaluate to 'false'" "correct warning" } */
     foo (13);
   if (0 == foo)
     foo (14);
@@ -51,10 +51,10 @@ bar (int a)
     foo (16);
   if (0 == i)
     foo (17);
-  if (0 == &a)	/* { dg-warning "never be NULL" "correct warning" } */
+  if (0 == &a)	/* { dg-warning "the comparison will always evaluate to 'false'" "correct warning" } */
     foo (18);
   if (0 == a)
     foo (19);
-  if (0 == &&lab) /* { dg-warning "never be NULL" "correct warning" } */
+  if (0 == &&lab) /* { dg-warning "the comparison will always evaluate to 'false'" "correct warning" } */
     foo (20);
 }

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]