public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ preview patch] PR 44277
@ 2011-10-30 19:39 Paolo Carlini
  2011-10-31  7:06 ` Paolo Carlini
  2011-10-31 15:56 ` Jason Merrill
  0 siblings, 2 replies; 7+ messages in thread
From: Paolo Carlini @ 2011-10-30 19:39 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill

[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]

Hi,

thus I'm working on a new warning for 0 as null ptr constant, which 
should be useful for people moving to C++11' nullptr. Looks like a good 
place to check for that is cp_convert_to_pointer, then the difficulty is 
coping correctly with things like:

     int* p;

     if (p)
       ;

where certainly we don't want to warn, but the front-end would, 
normally, because the latter is implicitly transformed via 
cp_truthvalue_conversion to

     if (p != 0)
       ;

Thus my idea: stop generating those implicit zeros and generate instead, 
internally, exactly nullptrs when pointers are involved! The idea 
appears to work pretty well, the substance of it is in very few lines in 
cp/typeck.c and cp/init.c, in particular in cp_truthvalue_conversion itself.

However, there is some complexity in the actual implementation, because 
we forward to the shared c_common_truthvalue_conversion, which does some 
non-trivial simplifications, and of course doesn't know about 
nullptr_node. To solve this I'm adding a parameter to it and passing 
nullptr_node or integer_zero_node, when appropriate. If unsharing 
c_common_truthvalue_conversion isn't an option , definitely would add a 
lot of redundant code, I don't see a neater way to accomplish the same 
result. Well, unless we could move nullptr_node from cp/ only to the 
global tree?!? In this case the patch would become tiny.

Thanks for any feedback!

(patch below passes the testsuite and does the right thing for the basic 
testcase)

Paolo.

/////////////////////


[-- Attachment #2: patch_44277_draft --]
[-- Type: text/plain, Size: 10074 bytes --]

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 180671)
+++ c-family/c.opt	(working copy)
@@ -685,6 +685,9 @@ Wpointer-sign
 C ObjC Var(warn_pointer_sign) Init(-1) Warning
 Warn when a pointer differs in signedness in an assignment
 
+Wzero-as-null-pointer-constant
+C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++)
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 180671)
+++ c-family/c-common.c	(working copy)
@@ -3898,7 +3898,7 @@ decl_with_nonnull_addr_p (const_tree expr)
    The resulting type should always be `truthvalue_type_node'.  */
 
 tree
-c_common_truthvalue_conversion (location_t location, tree expr)
+c_common_truthvalue_conversion (location_t location, tree expr, tree zeronode)
 {
   switch (TREE_CODE (expr))
     {
@@ -3921,9 +3921,11 @@ tree
 	return expr;
       expr = build2 (TREE_CODE (expr), truthvalue_type_node,
 		     c_common_truthvalue_conversion (location,
-						     TREE_OPERAND (expr, 0)),
+						     TREE_OPERAND (expr, 0),
+						     zeronode),
 		     c_common_truthvalue_conversion (location,
-						     TREE_OPERAND (expr, 1)));
+						     TREE_OPERAND (expr, 1),
+						     zeronode));
       goto ret;
 
     case TRUTH_NOT_EXPR:
@@ -3931,7 +3933,8 @@ tree
 	return expr;
       expr = build1 (TREE_CODE (expr), truthvalue_type_node,
 		     c_common_truthvalue_conversion (location,
-						     TREE_OPERAND (expr, 0)));
+						     TREE_OPERAND (expr, 0),
+						     zeronode));
       goto ret;
 
     case ERROR_MARK:
@@ -3976,9 +3979,11 @@ tree
 			      (TREE_SIDE_EFFECTS (TREE_OPERAND (expr, 1))
 			       ? TRUTH_OR_EXPR : TRUTH_ORIF_EXPR),
 		c_common_truthvalue_conversion (location,
-						TREE_OPERAND (expr, 0)),
+						TREE_OPERAND (expr, 0),
+						zeronode),
 		c_common_truthvalue_conversion (location,
-						TREE_OPERAND (expr, 1)),
+						TREE_OPERAND (expr, 1),
+						zeronode),
 			      0);
       goto ret;
 
@@ -3987,7 +3992,8 @@ tree
     case FLOAT_EXPR:
     case EXCESS_PRECISION_EXPR:
       /* These don't change whether an object is nonzero or zero.  */
-      return c_common_truthvalue_conversion (location, TREE_OPERAND (expr, 0));
+      return c_common_truthvalue_conversion (location, TREE_OPERAND (expr, 0),
+					     zeronode);
 
     case LROTATE_EXPR:
     case RROTATE_EXPR:
@@ -3998,12 +4004,13 @@ tree
 	  expr = build2 (COMPOUND_EXPR, truthvalue_type_node,
 			 TREE_OPERAND (expr, 1),
 			 c_common_truthvalue_conversion
-			 (location, TREE_OPERAND (expr, 0)));
+			 (location, TREE_OPERAND (expr, 0), zeronode));
 	  goto ret;
 	}
       else
 	return c_common_truthvalue_conversion (location,
-					       TREE_OPERAND (expr, 0));
+					       TREE_OPERAND (expr, 0),
+					       zeronode);
 
     case COND_EXPR:
       /* Distribute the conversion into the arms of a COND_EXPR.  */
@@ -4013,9 +4020,9 @@ tree
 	  tree op2 = TREE_OPERAND (expr, 2);
 	  /* In C++ one of the arms might have void type if it is throw.  */
 	  if (!VOID_TYPE_P (TREE_TYPE (op1)))
-	    op1 = c_common_truthvalue_conversion (location, op1);
+	    op1 = c_common_truthvalue_conversion (location, op1, zeronode);
 	  if (!VOID_TYPE_P (TREE_TYPE (op2)))
-	    op2 = c_common_truthvalue_conversion (location, op2);
+	    op2 = c_common_truthvalue_conversion (location, op2, zeronode);
 	  expr = fold_build3_loc (location, COND_EXPR, truthvalue_type_node,
 				  TREE_OPERAND (expr, 0), op1, op2);
 	  goto ret;
@@ -4026,9 +4033,11 @@ tree
 	  expr = build3 (COND_EXPR, truthvalue_type_node,
 			 TREE_OPERAND (expr, 0),
 			 c_common_truthvalue_conversion (location,
-							 TREE_OPERAND (expr, 1)),
+							 TREE_OPERAND (expr, 1),
+							 zeronode),
 			 c_common_truthvalue_conversion (location,
-							 TREE_OPERAND (expr, 2)));
+							 TREE_OPERAND (expr, 2),
+							 zeronode));
 	  goto ret;
 	}
 
@@ -4050,7 +4059,8 @@ tree
 	/* If this isn't narrowing the argument, we can ignore it.  */
 	if (TYPE_PRECISION (totype) >= TYPE_PRECISION (fromtype))
 	  return c_common_truthvalue_conversion (location,
-						 TREE_OPERAND (expr, 0));
+						 TREE_OPERAND (expr, 0),
+						 zeronode);
       }
       break;
 
@@ -4077,10 +4087,10 @@ tree
 		? TRUTH_OR_EXPR : TRUTH_ORIF_EXPR),
 	c_common_truthvalue_conversion
 	       (location,
-		build_unary_op (location, REALPART_EXPR, t, 0)),
+		build_unary_op (location, REALPART_EXPR, t, 0), zeronode),
 	c_common_truthvalue_conversion
 	       (location,
-		build_unary_op (location, IMAGPART_EXPR, t, 0)),
+		build_unary_op (location, IMAGPART_EXPR, t, 0), zeronode),
 	       0));
       goto ret;
     }
@@ -4093,7 +4103,7 @@ tree
       return build_binary_op (location, NE_EXPR, expr, fixed_zero_node, 1);
     }
   else
-    return build_binary_op (location, NE_EXPR, expr, integer_zero_node, 1);
+    return build_binary_op (location, NE_EXPR, expr, zeronode, 1);
 
  ret:
   protected_set_expr_location (expr, location);
Index: c-family/c-common.h
===================================================================
--- c-family/c-common.h	(revision 180671)
+++ c-family/c-common.h	(working copy)
@@ -753,7 +753,7 @@ extern tree c_fully_fold (tree, bool, bool *);
 extern tree decl_constant_value_for_optimization (tree);
 extern tree c_wrap_maybe_const (tree, bool);
 extern tree c_save_expr (tree);
-extern tree c_common_truthvalue_conversion (location_t, tree);
+extern tree c_common_truthvalue_conversion (location_t, tree, tree);
 extern void c_apply_type_quals_to_decl (int, tree);
 extern tree c_sizeof_or_alignof_type (location_t, tree, bool, int);
 extern tree c_alignof_expr (location_t, tree);
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 180665)
+++ cp/typeck.c	(working copy)
@@ -4058,7 +4058,9 @@ cp_build_binary_op (location_t location,
      	  else 
 	    {
 	      op0 = build_ptrmemfunc_access_expr (op0, pfn_identifier);
-	      op1 = cp_convert (TREE_TYPE (op0), integer_zero_node); 
+	      op1 = cp_convert (TREE_TYPE (op0),
+				NULLPTR_TYPE_P (TREE_TYPE (op1))
+				? nullptr_node : integer_zero_node);
 	    }
 	  result_type = TREE_TYPE (op0);
 	}
@@ -4668,9 +4670,12 @@ cp_truthvalue_conversion (tree expr)
   tree type = TREE_TYPE (expr);
   if (TYPE_PTRMEM_P (type))
     return build_binary_op (EXPR_LOCATION (expr),
-			    NE_EXPR, expr, integer_zero_node, 1);
+			    NE_EXPR, expr, nullptr_node, 1);
   else
-    return c_common_truthvalue_conversion (input_location, expr);
+    return c_common_truthvalue_conversion (input_location, expr,
+					   (TYPE_PTR_P (type)
+					    || TYPE_PTRMEMFUNC_P (type))
+					   ? nullptr_node : integer_zero_node);
 }
 
 /* Just like cp_truthvalue_conversion, but we want a CLEANUP_POINT_EXPR.  */
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 180665)
+++ cp/init.c	(working copy)
@@ -176,6 +176,8 @@ build_zero_init_1 (tree type, tree nelts, bool sta
        items with static storage duration that are not otherwise
        initialized are initialized to zero.  */
     ;
+  else if (TYPE_PTR_P (type) || TYPE_PTR_TO_MEMBER_P (type))
+    init = convert (type, nullptr_node);
   else if (SCALAR_TYPE_P (type))
     init = convert (type, integer_zero_node);
   else if (CLASS_TYPE_P (type))
@@ -1112,8 +1114,10 @@ expand_cleanup_for_base (tree binfo, tree flag)
   if (flag)
     expr = fold_build3_loc (input_location,
 			COND_EXPR, void_type_node,
-			c_common_truthvalue_conversion (input_location, flag),
-			expr, integer_zero_node);
+			    c_common_truthvalue_conversion (input_location,
+							    flag,
+							    integer_zero_node),
+			    expr, integer_zero_node);
 
   finish_eh_cleanup (expr);
 }
Index: cp/rtti.c
===================================================================
--- cp/rtti.c	(revision 180665)
+++ cp/rtti.c	(working copy)
@@ -747,7 +747,8 @@ build_dynamic_cast_1 (tree type, tree expr, tsubst
 	      tree neq;
 
 	      result = save_expr (result);
-	      neq = c_common_truthvalue_conversion (input_location, result);
+	      neq = c_common_truthvalue_conversion (input_location, result,
+						    integer_zero_node);
 	      return cp_convert (type,
 				 build3 (COND_EXPR, TREE_TYPE (result),
 					 neq, result, bad));
Index: cp/cvt.c
===================================================================
--- cp/cvt.c	(revision 180665)
+++ cp/cvt.c	(working copy)
@@ -198,6 +198,10 @@ cp_convert_to_pointer (tree type, tree expr)
 
   if (null_ptr_cst_p (expr))
     {
+      if (!NULLPTR_TYPE_P (TREE_TYPE (expr)))
+	warning (OPT_Wzero_as_null_pointer_constant,
+		 "zero as null pointer constant");
+
       if (TYPE_PTRMEMFUNC_P (type))
 	return build_ptrmemfunc (TYPE_PTRMEMFUNC_FN_TYPE (type), expr, 0,
 				 /*c_cast_p=*/false, tf_warning_or_error);
Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 180665)
+++ c-typeck.c	(working copy)
@@ -9869,8 +9869,10 @@ build_binary_op (location_t location, enum tree_co
 	     but that does not mean the operands should be
 	     converted to ints!  */
 	  result_type = integer_type_node;
-	  op0 = c_common_truthvalue_conversion (location, op0);
-	  op1 = c_common_truthvalue_conversion (location, op1);
+	  op0 = c_common_truthvalue_conversion (location, op0,
+						integer_zero_node);
+	  op1 = c_common_truthvalue_conversion (location, op1,
+						integer_zero_node);
 	  converted = 1;
 	  boolean_op = true;
 	}
@@ -10587,7 +10589,7 @@ c_objc_common_truthvalue_conversion (location_t lo
 
   /* ??? Should we also give an error for vectors rather than leaving
      those to give errors later?  */
-  expr = c_common_truthvalue_conversion (location, expr);
+  expr = c_common_truthvalue_conversion (location, expr, integer_zero_node);
 
   if (TREE_CODE (expr) == INTEGER_CST && int_operands && !int_const)
     {

[-- Attachment #3: wzero.C --]
[-- Type: text/x-c++src, Size: 624 bytes --]

struct A;

typedef int (A::*pointmemfun) (int);
typedef int (A::*pointdmem);
typedef int (*pointfun) (int);

pointmemfun pmf;
pointdmem   pdm;
pointfun    pf;     
int*        p;

void f()
{
  if (pmf)
    ;

  if (pdm)
    ;

  if (pf)
    ;

  if (p)
    ;

  if (pmf == 0)     // { dg-warning "zero" }
    ;

  if (pdm == 0)     // { dg-warning "zero" }
    ;

  if (pf == 0)       // { dg-warning "zero" }
    ;

  if (p == 0)       // { dg-warning "zero" }
    ;

#ifdef __GXX_EXPERIMENTAL_CXX0X__
  if (pmf == nullptr)
    ;

  if (pdm == nullptr)
    ;

  if (pf == nullptr)
    ;

  if (p == nullptr)
    ;
#endif
}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [C++ preview patch] PR 44277
  2011-10-30 19:39 [C++ preview patch] PR 44277 Paolo Carlini
@ 2011-10-31  7:06 ` Paolo Carlini
  2011-10-31 15:56 ` Jason Merrill
  1 sibling, 0 replies; 7+ messages in thread
From: Paolo Carlini @ 2011-10-31  7:06 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches, Jason Merrill

[-- Attachment #1: Type: text/plain, Size: 179 bytes --]

... this is another option: use an 'integer_zero_or_nullptr_node (tree)' 
defined differently for C and C++. Certainly the whole thing becomes 
smaller.

Paolo.

////////////////

[-- Attachment #2: patch_44277_draft_2 --]
[-- Type: text/plain, Size: 4393 bytes --]

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 180690)
+++ c-family/c.opt	(working copy)
@@ -685,6 +685,9 @@ Wpointer-sign
 C ObjC Var(warn_pointer_sign) Init(-1) Warning
 Warn when a pointer differs in signedness in an assignment
 
+Wzero-as-null-pointer-constant
+C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++)
Index: c-family/c-common.c
===================================================================
--- c-family/c-common.c	(revision 180690)
+++ c-family/c-common.c	(working copy)
@@ -4093,7 +4093,8 @@ c_common_truthvalue_conversion (location_t locatio
       return build_binary_op (location, NE_EXPR, expr, fixed_zero_node, 1);
     }
   else
-    return build_binary_op (location, NE_EXPR, expr, integer_zero_node, 1);
+    return build_binary_op (location, NE_EXPR, expr,
+			    integer_zero_or_nullptr_node (expr), 1);
 
  ret:
   protected_set_expr_location (expr, location);
Index: c-family/c-common.h
===================================================================
--- c-family/c-common.h	(revision 180690)
+++ c-family/c-common.h	(working copy)
@@ -889,6 +889,8 @@ extern tree common_type (tree, tree);
 
 extern tree decl_constant_value (tree);
 
+extern tree integer_zero_or_nullptr_node (tree);
+
 /* Handle increment and decrement of boolean types.  */
 extern tree boolean_increment (enum tree_code, tree);
 
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 180690)
+++ cp/typeck.c	(working copy)
@@ -4058,7 +4058,9 @@ cp_build_binary_op (location_t location,
      	  else 
 	    {
 	      op0 = build_ptrmemfunc_access_expr (op0, pfn_identifier);
-	      op1 = cp_convert (TREE_TYPE (op0), integer_zero_node); 
+	      op1 = cp_convert (TREE_TYPE (op0),
+				NULLPTR_TYPE_P (TREE_TYPE (op1))
+				? nullptr_node : integer_zero_node);
 	    }
 	  result_type = TREE_TYPE (op0);
 	}
@@ -4658,6 +4660,16 @@ build_x_unary_op (enum tree_code code, tree xarg,
   return exp;
 }
 
+/*
+ */
+tree
+integer_zero_or_nullptr_node (tree expr)
+{
+  tree type = TREE_TYPE (expr);
+  return (TYPE_PTR_P (type) || TYPE_PTRMEMFUNC_P (type)
+	  ? nullptr_node : integer_zero_node);
+}
+
 /* Like c_common_truthvalue_conversion, but handle pointer-to-member
    constants, where a null value is represented by an INTEGER_CST of
    -1.  */
@@ -4668,7 +4680,7 @@ cp_truthvalue_conversion (tree expr)
   tree type = TREE_TYPE (expr);
   if (TYPE_PTRMEM_P (type))
     return build_binary_op (EXPR_LOCATION (expr),
-			    NE_EXPR, expr, integer_zero_node, 1);
+			    NE_EXPR, expr, nullptr_node, 1);
   else
     return c_common_truthvalue_conversion (input_location, expr);
 }
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 180690)
+++ cp/init.c	(working copy)
@@ -176,6 +176,8 @@ build_zero_init_1 (tree type, tree nelts, bool sta
        items with static storage duration that are not otherwise
        initialized are initialized to zero.  */
     ;
+  else if (TYPE_PTR_P (type) || TYPE_PTR_TO_MEMBER_P (type))
+    init = convert (type, nullptr_node);
   else if (SCALAR_TYPE_P (type))
     init = convert (type, integer_zero_node);
   else if (CLASS_TYPE_P (type))
Index: cp/cvt.c
===================================================================
--- cp/cvt.c	(revision 180690)
+++ cp/cvt.c	(working copy)
@@ -198,6 +198,10 @@ cp_convert_to_pointer (tree type, tree expr)
 
   if (null_ptr_cst_p (expr))
     {
+      if (!NULLPTR_TYPE_P (TREE_TYPE (expr)))
+	warning (OPT_Wzero_as_null_pointer_constant,
+		 "zero as null pointer constant");
+
       if (TYPE_PTRMEMFUNC_P (type))
 	return build_ptrmemfunc (TYPE_PTRMEMFUNC_FN_TYPE (type), expr, 0,
 				 /*c_cast_p=*/false, tf_warning_or_error);
Index: c-typeck.c
===================================================================
--- c-typeck.c	(revision 180690)
+++ c-typeck.c	(working copy)
@@ -9510,6 +9510,15 @@ scalar_to_vector (location_t loc, enum tree_code c
 
   return stv_nothing;
 }
+
+/*
+ */
+tree
+integer_zero_or_nullptr_node (tree type ATTRIBUTE_UNUSED)
+{
+  return integer_zero_node;
+}
+
 \f
 /* Build a binary-operation expression without default conversions.
    CODE is the kind of expression to build.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [C++ preview patch] PR 44277
  2011-10-30 19:39 [C++ preview patch] PR 44277 Paolo Carlini
  2011-10-31  7:06 ` Paolo Carlini
@ 2011-10-31 15:56 ` Jason Merrill
  2011-10-31 18:09   ` Paolo Carlini
  2011-10-31 20:27   ` Paolo Carlini
  1 sibling, 2 replies; 7+ messages in thread
From: Jason Merrill @ 2011-10-31 15:56 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

How does it work to warn in convert_like_real instead?

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [C++ preview patch] PR 44277
  2011-10-31 15:56 ` Jason Merrill
@ 2011-10-31 18:09   ` Paolo Carlini
  2011-10-31 20:27   ` Paolo Carlini
  1 sibling, 0 replies; 7+ messages in thread
From: Paolo Carlini @ 2011-10-31 18:09 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi,
> How does it work to warn in convert_like_real instead?
the problem is that (expr, totype) can be a lot of different things for 
which we want to warn, can be a zero and a pointer for assignments, but, 
when totype is a BOOLEAN_TYPE expr can be an EQ_EXPR or NEQ_EXPR and 
then the operands various things depending on whether we are looking for 
pointer, data member pointer, etc, on the left or on the right of the == 
or != sign. In other terms, the pattern matching doesn't seem matter of 
a few lines. I'm annoyed by this. Also, for the assignment case, I'm 
getting duplicate warnings, maybe can be fixed.

Do you think there is no neat way to implement my idea of avoiding 
generating those implicit 0s in the first place? The internals of the 
front-end, seem very c++98-ish for null pointers ;)

Paolo.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [C++ preview patch] PR 44277
  2011-10-31 15:56 ` Jason Merrill
  2011-10-31 18:09   ` Paolo Carlini
@ 2011-10-31 20:27   ` Paolo Carlini
  2011-10-31 21:10     ` Jason Merrill
  1 sibling, 1 reply; 7+ messages in thread
From: Paolo Carlini @ 2011-10-31 20:27 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 295 bytes --]

... so today I noticed the c_inhibit_evaluation_warnings use in 
cp_convert_and_check and occurred to me that we could use the existing 
mechanism for this warning too?

The below still passes checking and my small set of tests...

What do you think?

Thanks,
Paolo.

//////////////////////////

[-- Attachment #2: patch_44277_draft_3 --]
[-- Type: text/plain, Size: 3273 bytes --]

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 180705)
+++ c-family/c.opt	(working copy)
@@ -685,6 +685,9 @@ Wpointer-sign
 C ObjC Var(warn_pointer_sign) Init(-1) Warning
 Warn when a pointer differs in signedness in an assignment
 
+Wzero-as-null-pointer-constant
+C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++)
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 180705)
+++ cp/typeck.c	(working copy)
@@ -4057,8 +4057,13 @@ cp_build_binary_op (location_t location,
 	    }
      	  else 
 	    {
+	      bool inhibit = NULLPTR_TYPE_P (TREE_TYPE (op1));
 	      op0 = build_ptrmemfunc_access_expr (op0, pfn_identifier);
-	      op1 = cp_convert (TREE_TYPE (op0), integer_zero_node); 
+	      if (inhibit)
+		++c_inhibit_evaluation_warnings;
+	      op1 = cp_convert (TREE_TYPE (op0), integer_zero_node);
+	      if (inhibit)
+		--c_inhibit_evaluation_warnings;
 	    }
 	  result_type = TREE_TYPE (op0);
 	}
@@ -4666,11 +4671,25 @@ tree
 cp_truthvalue_conversion (tree expr)
 {
   tree type = TREE_TYPE (expr);
+  tree ret;
+
   if (TYPE_PTRMEM_P (type))
-    return build_binary_op (EXPR_LOCATION (expr),
-			    NE_EXPR, expr, integer_zero_node, 1);
+    {
+      ++c_inhibit_evaluation_warnings;
+      ret = build_binary_op (EXPR_LOCATION (expr),
+			     NE_EXPR, expr, integer_zero_node, 1);
+      --c_inhibit_evaluation_warnings;
+    }
+  else if (TYPE_PTR_P (type) || TYPE_PTRMEMFUNC_P (type))
+    {
+      ++c_inhibit_evaluation_warnings;
+      ret = c_common_truthvalue_conversion (input_location, expr);
+      --c_inhibit_evaluation_warnings;
+    }
   else
-    return c_common_truthvalue_conversion (input_location, expr);
+    ret = c_common_truthvalue_conversion (input_location, expr);
+
+  return ret;
 }
 
 /* Just like cp_truthvalue_conversion, but we want a CLEANUP_POINT_EXPR.  */
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 180705)
+++ cp/init.c	(working copy)
@@ -176,6 +176,12 @@ build_zero_init_1 (tree type, tree nelts, bool sta
        items with static storage duration that are not otherwise
        initialized are initialized to zero.  */
     ;
+  else if (TYPE_PTR_P (type) || TYPE_PTR_TO_MEMBER_P (type))
+    {
+      ++c_inhibit_evaluation_warnings;
+      init = convert (type, integer_zero_node);
+      --c_inhibit_evaluation_warnings;
+    }
   else if (SCALAR_TYPE_P (type))
     init = convert (type, integer_zero_node);
   else if (CLASS_TYPE_P (type))
Index: cp/cvt.c
===================================================================
--- cp/cvt.c	(revision 180705)
+++ cp/cvt.c	(working copy)
@@ -198,6 +198,11 @@ cp_convert_to_pointer (tree type, tree expr)
 
   if (null_ptr_cst_p (expr))
     {
+      if (c_inhibit_evaluation_warnings == 0
+	  && !NULLPTR_TYPE_P (TREE_TYPE (expr)))
+	warning (OPT_Wzero_as_null_pointer_constant,
+		 "zero as null pointer constant");
+
       if (TYPE_PTRMEMFUNC_P (type))
 	return build_ptrmemfunc (TYPE_PTRMEMFUNC_FN_TYPE (type), expr, 0,
 				 /*c_cast_p=*/false, tf_warning_or_error);

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [C++ preview patch] PR 44277
  2011-10-31 20:27   ` Paolo Carlini
@ 2011-10-31 21:10     ` Jason Merrill
  2011-10-31 21:28       ` Paolo Carlini
  0 siblings, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2011-10-31 21:10 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

On 10/31/2011 04:09 PM, Paolo Carlini wrote:
> ... so today I noticed the c_inhibit_evaluation_warnings use in
> cp_convert_and_check and occurred to me that we could use the existing
> mechanism for this warning too?
>
> The below still passes checking and my small set of tests...

I notice that this patch only changes the C++ front end, and it seems 
like you already have special cases for pointers/pointers to members, so 
you might as well go ahead and use nullptr_node.

Jason

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [C++ preview patch] PR 44277
  2011-10-31 21:10     ` Jason Merrill
@ 2011-10-31 21:28       ` Paolo Carlini
  0 siblings, 0 replies; 7+ messages in thread
From: Paolo Carlini @ 2011-10-31 21:28 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On 10/31/2011 09:29 PM, Jason Merrill wrote:
> On 10/31/2011 04:09 PM, Paolo Carlini wrote:
>> ... so today I noticed the c_inhibit_evaluation_warnings use in
>> cp_convert_and_check and occurred to me that we could use the existing
>> mechanism for this warning too?
>>
>> The below still passes checking and my small set of tests...
>
> I notice that this patch only changes the C++ front end, and it seems 
> like you already have special cases for pointers/pointers to members, 
> so you might as well go ahead and use nullptr_node.
Right. Thus essentially a mix of the two recent tries, like the below, 
right? Patch becomes even simpler and more importantly we rely on 
c_inhibit_* only for c code proper.

If you think I'm on the right track, I will add the testcases, 
documentation, etc.

Is the name of the warning ok? It's a bit long...

Thanks,
Paolo.

////////////////////

[-- Attachment #2: patch_44277_draft_4 --]
[-- Type: text/plain, Size: 2948 bytes --]

Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 180705)
+++ c-family/c.opt	(working copy)
@@ -685,6 +685,9 @@ Wpointer-sign
 C ObjC Var(warn_pointer_sign) Init(-1) Warning
 Warn when a pointer differs in signedness in an assignment
 
+Wzero-as-null-pointer-constant
+C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
+
 ansi
 C ObjC C++ ObjC++
 A synonym for -std=c89 (for C) or -std=c++98 (for C++)
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 180705)
+++ cp/typeck.c	(working copy)
@@ -4058,7 +4058,9 @@ cp_build_binary_op (location_t location,
      	  else 
 	    {
 	      op0 = build_ptrmemfunc_access_expr (op0, pfn_identifier);
-	      op1 = cp_convert (TREE_TYPE (op0), integer_zero_node); 
+	      op1 = cp_convert (TREE_TYPE (op0),
+				NULLPTR_TYPE_P (TREE_TYPE (op1))
+				? nullptr_node : integer_zero_node);
 	    }
 	  result_type = TREE_TYPE (op0);
 	}
@@ -4666,11 +4668,21 @@ tree
 cp_truthvalue_conversion (tree expr)
 {
   tree type = TREE_TYPE (expr);
+  tree ret;
+
   if (TYPE_PTRMEM_P (type))
-    return build_binary_op (EXPR_LOCATION (expr),
-			    NE_EXPR, expr, integer_zero_node, 1);
+    ret = build_binary_op (EXPR_LOCATION (expr),
+			   NE_EXPR, expr, nullptr_node, 1);
+  else if (TYPE_PTR_P (type) || TYPE_PTRMEMFUNC_P (type))
+    {
+      ++c_inhibit_evaluation_warnings;
+      ret = c_common_truthvalue_conversion (input_location, expr);
+      --c_inhibit_evaluation_warnings;
+    }
   else
-    return c_common_truthvalue_conversion (input_location, expr);
+    ret = c_common_truthvalue_conversion (input_location, expr);
+
+  return ret;
 }
 
 /* Just like cp_truthvalue_conversion, but we want a CLEANUP_POINT_EXPR.  */
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 180705)
+++ cp/init.c	(working copy)
@@ -176,6 +176,8 @@ build_zero_init_1 (tree type, tree nelts, bool sta
        items with static storage duration that are not otherwise
        initialized are initialized to zero.  */
     ;
+  else if (TYPE_PTR_P (type) || TYPE_PTR_TO_MEMBER_P (type))
+    init = convert (type, nullptr_node);
   else if (SCALAR_TYPE_P (type))
     init = convert (type, integer_zero_node);
   else if (CLASS_TYPE_P (type))
Index: cp/cvt.c
===================================================================
--- cp/cvt.c	(revision 180705)
+++ cp/cvt.c	(working copy)
@@ -198,6 +198,11 @@ cp_convert_to_pointer (tree type, tree expr)
 
   if (null_ptr_cst_p (expr))
     {
+      if (c_inhibit_evaluation_warnings == 0
+	  && !NULLPTR_TYPE_P (TREE_TYPE (expr)))
+	warning (OPT_Wzero_as_null_pointer_constant,
+		 "zero as null pointer constant");
+
       if (TYPE_PTRMEMFUNC_P (type))
 	return build_ptrmemfunc (TYPE_PTRMEMFUNC_FN_TYPE (type), expr, 0,
 				 /*c_cast_p=*/false, tf_warning_or_error);

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2011-10-31 20:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-10-30 19:39 [C++ preview patch] PR 44277 Paolo Carlini
2011-10-31  7:06 ` Paolo Carlini
2011-10-31 15:56 ` Jason Merrill
2011-10-31 18:09   ` Paolo Carlini
2011-10-31 20:27   ` Paolo Carlini
2011-10-31 21:10     ` Jason Merrill
2011-10-31 21:28       ` Paolo Carlini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).