Re: [C++ patch] accept __null as sentinel

[Michael Matz <matz at suse dot de>]

Hi,

On Fri, 29 Apr 2005, Richard Henderson wrote:

> On Fri, Apr 29, 2005 at 09:43:09PM +0200, Michael Matz wrote:
> > 	PR c++/19542
> > 	* c-common.c (c_common_nodes_and_builtins): Create global null_node.
> > 	(check_function_sentinel): Check for null_node as valid sentinel too.
> > 
> > 	* tree.h (tree_index): Added TI_NULL.
> > 	Define null_node as global_tree[TI_NULL].
> 
> You can move this to c/c++ common code without moving it to code
> common to all languages.

Like so.  (this now also introduces a -Wstrict-null-sentinel option, to be 
put whereever it could fit).  I've tested this by building compiler, docu 
and testcase only.  Bootstrap and regtest for C and C++ in progress (as 
the code didn't really change I don't expect surprises, though).


Ciao,
Michael.
-- 
gcc/Changelog:
	PR c++/19542
	* c-common.c (c_common_nodes_and_builtins): Create global null_node.
	(warn_strict_null_sentinel): Define.
	(check_function_sentinel): Check for null_node as valid sentinel too.
	* c-common.h (c_tree_index): Added CTI_NULL.
	(null_node) Define global_tree[CTI_NULL].
	(warn_strict_null_sentinel): Declare.
	* c-opts.c: (c_common_handle_option): Handle -Wstrict-null-sentinel.
	* c.opt: (Wstrict-null-sentinel): New C++ option.
	* doc/invoke.texi (C++ Options): Document -Wstrict-null-sentinel.

gcc/cp/Changelog:
	PR c++/19542
	* cp-tree.h (cp_tree_index): Remove CPTI_NULL, to be defined in C
	common frontend.
	(null_node): Remove.
	* lex.c (cxx_init): Move null_node initialisation to C common frontend.

gcc/testsuite/Changelog:
	PR c++/19542
	* g++.dg/warn/sentinel.C: New testcase for __null sentinels added.


Index: gcc/c-common.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.h,v
retrieving revision 1.277.2.1
diff -u -p -r1.277.2.1 c-common.h
--- gcc/c-common.h	30 Mar 2005 01:38:07 -0000	1.277.2.1
+++ gcc/c-common.h	30 Apr 2005 01:52:35 -0000
@@ -160,6 +160,8 @@ enum c_tree_index
     
     CTI_VOID_ZERO,
 
+    CTI_NULL,
+
     CTI_MAX
 };
 
@@ -204,6 +206,9 @@ struct c_common_identifier GTY(())
 /* A node for `((void) 0)'.  */
 #define void_zero_node                  c_global_trees[CTI_VOID_ZERO]
 
+/* The node for C++ `__null'.  */
+#define null_node                       c_global_trees[CTI_NULL]
+
 extern GTY(()) tree c_global_trees[CTI_MAX];
 
 /* In a RECORD_TYPE, a sorted array of the fields of the type, not a
@@ -575,6 +580,12 @@ extern int flag_threadsafe_statics;
 
 extern int warn_implicit;
 
+/* Warn about using __null (as NULL in C++) as sentinel.  For code compiled
+   with GCC this doesn't matter as __null is guaranteed to have the right
+   size.  */
+
+extern int warn_strict_null_sentinel;
+
 /* Maximum template instantiation depth.  This limit is rather
    arbitrary, but it exists to limit the time it takes to notice
    infinite template instantiations.  */
Index: gcc/c-common.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-common.c,v
retrieving revision 1.606.2.2
diff -u -p -r1.606.2.2 c-common.c
--- gcc/c-common.c	30 Mar 2005 01:38:04 -0000	1.606.2.2
+++ gcc/c-common.c	30 Apr 2005 01:52:36 -0000
@@ -284,6 +284,12 @@ int warn_unknown_pragmas; /* Tri state v
 
 int warn_format;
 
+/* Warn about using __null (as NULL in C++) as sentinel.  For code compiled
+   with GCC this doesn't matter as __null is guaranteed to have the right
+   size.  */
+
+int warn_strict_null_sentinel;
+
 /* Zero means that faster, ...NonNil variants of objc_msgSend...
    calls will be used in ObjC; passing nil receivers to such calls
    will most likely result in crashes.  */
@@ -3269,6 +3275,11 @@ c_common_nodes_and_builtins (void)
     mudflap_init ();
 
   main_identifier_node = get_identifier ("main");
+
+  /* Create the built-in __null node.  It is important that this is
+     not shared.  */
+  null_node = make_node (INTEGER_CST);
+  TREE_TYPE (null_node) = c_common_type_for_size (POINTER_SIZE, 0);
 }
 
 /* Look up the function in built_in_decls that corresponds to DECL
@@ -5115,8 +5126,15 @@ check_function_sentinel (tree attrs, tre
 	    }
 
 	  /* Validate the sentinel.  */
-	  if (!POINTER_TYPE_P (TREE_TYPE (TREE_VALUE (sentinel)))
-	      || !integer_zerop (TREE_VALUE (sentinel)))
+	  if ((!POINTER_TYPE_P (TREE_TYPE (TREE_VALUE (sentinel)))
+	       || !integer_zerop (TREE_VALUE (sentinel)))
+	      /* Although __null (in C++) is only an integer we allow it
+		 nevertheless, as we are guaranteed that it's exactly
+		 as wide as a pointer, and we don't want to force
+		 users to cast the NULL they have written there.
+		 We warn with -Wstrict-null-sentinel, though.  */
+              && (warn_strict_null_sentinel
+		  || null_node != TREE_VALUE (sentinel)))
 	    warning ("missing sentinel in function call");
 	}
     }
Index: gcc/c-opts.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c-opts.c,v
retrieving revision 1.136.2.2
diff -u -p -r1.136.2.2 c-opts.c
--- gcc/c-opts.c	23 Mar 2005 01:38:39 -0000	1.136.2.2
+++ gcc/c-opts.c	30 Apr 2005 01:52:36 -0000
@@ -464,6 +464,10 @@ c_common_handle_option (size_t scode, co
       warn_return_type = value;
       break;
 
+    case OPT_Wstrict_null_sentinel:
+      warn_strict_null_sentinel = value;
+      break;
+
     case OPT_Wsystem_headers:
       cpp_opts->warn_system_headers = value;
       break;
Index: gcc/c.opt
===================================================================
RCS file: /cvs/gcc/gcc/gcc/c.opt,v
retrieving revision 1.34.8.3
diff -u -p -r1.34.8.3 c.opt
--- gcc/c.opt	6 Apr 2005 19:35:31 -0000	1.34.8.3
+++ gcc/c.opt	30 Apr 2005 01:52:36 -0000
@@ -386,6 +386,10 @@ Wsign-promo
 C++ ObjC++ Var(warn_sign_promo)
 Warn when overload promotes from unsigned to signed
 
+Wstrict-null-sentinel
+C++ ObjC++
+Warn about uncasted NULL used as sentinel
+
 Wstrict-prototypes
 C ObjC Var(warn_strict_prototypes)
 Warn about unprototyped function declarations
Index: gcc/cp/cp-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/cp-tree.h,v
retrieving revision 1.1106.2.3
diff -u -p -r1.1106.2.3 cp-tree.h
--- gcc/cp/cp-tree.h	8 Apr 2005 19:36:00 -0000	1.1106.2.3
+++ gcc/cp/cp-tree.h	30 Apr 2005 01:52:36 -0000
@@ -513,7 +513,6 @@ enum cp_tree_index
     CPTI_LANG_NAME_JAVA,
 
     CPTI_EMPTY_EXCEPT_SPEC,
-    CPTI_NULL,
     CPTI_JCLASS,
     CPTI_TERMINATE,
     CPTI_CALL_UNEXPECTED,
@@ -609,9 +608,6 @@ extern GTY(()) tree cp_global_trees[CPTI
 /* Exception specifier used for throw().  */
 #define empty_except_spec               cp_global_trees[CPTI_EMPTY_EXCEPT_SPEC]
 
-/* The node for `__null'.  */
-#define null_node                       cp_global_trees[CPTI_NULL]
-
 /* If non-NULL, a POINTER_TYPE equivalent to (java::lang::Class*).  */
 #define jclass_node                     cp_global_trees[CPTI_JCLASS]
 
Index: gcc/cp/lex.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/lex.c,v
retrieving revision 1.353
diff -u -p -r1.353 lex.c
--- gcc/cp/lex.c	25 Nov 2004 11:13:34 -0000	1.353
+++ gcc/cp/lex.c	30 Apr 2005 01:52:36 -0000
@@ -348,11 +348,6 @@ cxx_init (void)
 
   cxx_init_decl_processing ();
 
-  /* Create the built-in __null node.  It is important that this is
-     not shared.  */
-  null_node = make_node (INTEGER_CST);
-  TREE_TYPE (null_node) = c_common_type_for_size (POINTER_SIZE, 0);
-
   /* The fact that G++ uses COMDAT for many entities (inline
      functions, template instantiations, virtual tables, etc.) mean
      that it is fundamentally unreliable to try to make decisions
Index: gcc/doc/invoke.texi
===================================================================
RCS file: /cvs/gcc/gcc/gcc/doc/invoke.texi,v
retrieving revision 1.583.2.15
diff -u -p -r1.583.2.15 invoke.texi
--- gcc/doc/invoke.texi	25 Apr 2005 14:45:06 -0000	1.583.2.15
+++ gcc/doc/invoke.texi	30 Apr 2005 01:52:37 -0000
@@ -185,7 +185,7 @@ in the following sections.
 -fno-default-inline  -fvisibility-inlines-hidden @gol
 -Wabi  -Wctor-dtor-privacy @gol
 -Wnon-virtual-dtor  -Wreorder @gol
--Weffc++  -Wno-deprecated @gol
+-Weffc++  -Wno-deprecated  -Wstrict-null-sentinel @gol
 -Wno-non-template-friend  -Wold-style-cast @gol
 -Woverloaded-virtual  -Wno-pmf-conversions @gol
 -Wsign-promo}
@@ -1735,6 +1735,14 @@ to filter out those warnings.
 @opindex Wno-deprecated
 Do not warn about usage of deprecated features.  @xref{Deprecated Features}.
 
+@item -Wstrict-null-sentinel @r{(C++ only)}
+@opindex Wstrict-null-sentinel
+Warn also about the use of an uncasted NULL as sentinel.  When
+compiling only with GCC this is a valid sentinel, as NULL is defined
+to __null.  Although it is a null pointer constant not a null pointer,
+it is guaranteed to of the same size as a pointer.  But this use is
+not portable across different compilers.
+
 @item -Wno-non-template-friend @r{(C++ only)}
 @opindex Wno-non-template-friend
 Disable warnings when non-templatized friend functions are declared
*** gcc/testsuite/g++.dg/warn/sentinel.C.sentinel	2005-04-01 10:16:45.290186926 +0200
--- gcc/testsuite/g++.dg/warn/sentinel.C	2005-04-01 10:16:45.289187140 +0200
***************
*** 0 ****
--- 1,11 ----
+ /* { dg-do compile } */
+ /* { dg-options "-Wall" } */
+ extern void ex (int i, ...) __attribute__ ((__sentinel__(0)));
+ 
+ void f()
+ {
+   ex (1, 0);		/* { dg-warning "missing sentinel in function call" "" } */
+   ex (1, 0L);		/* { dg-warning "missing sentinel in function call" "" } */
+   ex (1, (void *)0);
+   ex (1, __null);	/* { dg-bogus "sentinel" } */
+ }