| Message ID | 53DFB3AE.1030706@oracle.com |
|---|---|
| State | New |
| Headers | show |
On 08/04/2014 12:24 PM, Paolo Carlini wrote:
> + || (TYPE_PTR_P (type1) && VOID_TYPE_P (TREE_TYPE (type1))
Why check for VOID_TYPE_P? I'd think we would want to warn about
comparing to other null pointer values as well.
Jason
Hi, On 08/04/2014 10:45 PM, Jason Merrill wrote: > On 08/04/2014 12:24 PM, Paolo Carlini wrote: >> + || (TYPE_PTR_P (type1) && VOID_TYPE_P (TREE_TYPE (type1)) > > Why check for VOID_TYPE_P? I'd think we would want to warn about > comparing to other null pointer values as well. In fact I wondered about that a few minutes after sending my message... And this is what I figured out: normally we have hard errors from composite_pointer_type (eg, try scalar types, class types), even for null values. The only exception I have been able to find earlier today is that of pointer to the same function type, eg: extern void z(); typedef void (*ptr)(); void i() { if ( z != (ptr)0 ); } but in this case the C front-end too doesn't warn. In short, the case of (void*)0 seems very special. However, something I did *not* notice earlier today, is that comparing a pointer to function to a generic void* leads to a pedwarn at the beginning of composite_pointer_type about the comparison itself. Thus it's debatable whether we also want the -Waddress warning... If you ask me, closing the bug with a testcase which checks that we warn for -pedantic about the comparison (if we don't have one already) would be Ok (vs EDG 4.9 too) What do you think? Paolo.
On 08/04/2014 07:01 PM, Paolo Carlini wrote: > In fact I wondered about that a few minutes after sending my message... > And this is what I figured out: normally we have hard errors from > composite_pointer_type (eg, try scalar types, class types), even for > null values. The only exception I have been able to find earlier today > is that of pointer to the same function type, eg: > > extern void z(); > typedef void (*ptr)(); > void i() { if ( z != (ptr)0 ); } > > but in this case the C front-end too doesn't warn. I don't see why we wouldn't want to warn in this case; it's still the case thet the comparison will always be false. We can also see this situation for non-function pointers: void f() { int i; if (&i != (int*)0); } Jason
Index: cp/typeck.c =================================================================== --- cp/typeck.c (revision 213573) +++ cp/typeck.c (working copy) @@ -4353,12 +4353,11 @@ cp_build_binary_op (location_t location, && (code1 == INTEGER_TYPE || code1 == REAL_TYPE || code1 == COMPLEX_TYPE || code1 == ENUMERAL_TYPE)) short_compare = 1; - else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE) - || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1))) - result_type = composite_pointer_type (type0, type1, op0, op1, - CPO_COMPARISON, complain); else if ((code0 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type0)) - && null_ptr_cst_p (op1)) + && (null_ptr_cst_p (op1) + /* Handle (void*)0 too. */ + || (TYPE_PTR_P (type1) && VOID_TYPE_P (TREE_TYPE (type1)) + && integer_zerop (op1)))) { if (TREE_CODE (op0) == ADDR_EXPR && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0))) @@ -4371,7 +4370,10 @@ cp_build_binary_op (location_t location, result_type = type0; } else if ((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1)) - && null_ptr_cst_p (op0)) + && (null_ptr_cst_p (op0) + /* Handle (void*)0 too. */ + || (TYPE_PTR_P (type0) && VOID_TYPE_P (TREE_TYPE (type0)) + && integer_zerop (op0)))) { if (TREE_CODE (op1) == ADDR_EXPR && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0))) @@ -4383,6 +4385,10 @@ cp_build_binary_op (location_t location, } result_type = type1; } + else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE) + || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1))) + result_type = composite_pointer_type (type0, type1, op0, op1, + CPO_COMPARISON, complain); else if (null_ptr_cst_p (op0) && null_ptr_cst_p (op1)) /* One of the operands must be of nullptr_t type. */ result_type = TREE_TYPE (nullptr_node); Index: testsuite/g++.dg/warn/Waddress-1.C =================================================================== --- testsuite/g++.dg/warn/Waddress-1.C (revision 0) +++ testsuite/g++.dg/warn/Waddress-1.C (working copy) @@ -0,0 +1,7 @@ +// PR c++/43906 +// { dg-options "-Waddress" } + +extern void z(); +void f() { if ( z ) z(); } // { dg-warning "address" } +void g() { if ( z != 0 ) z(); } // { dg-warning "address" } +void h() { if ( z != (void*)0 ) z(); } // { dg-warning "address" }
Hi, I suppose we can quickly resolve, one way or another, this rather old issue. Considering: extern void z(); void h() { if ( z != (void*)0 ); } we -Waddress warn in C and we don't in C++, due to the rather subtle differences between null_pointer_constant_p and null_ptr_cst_p. I believe we could as well warn in C++ too, but then I'm afraid we have to handle the case specially, like in the below. What do you think? Thanks! Paolo. //////////////////////