Re: [PATCH C/C++] Fix some diagnostics problems

[Shujing Zhao <pearly.zhao@oracle.com>]

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

On 06/05/2010 09:18 PM, Joseph S. Myers wrote:
> On Fri, 4 Jun 2010, Shujing Zhao wrote:
> The Ubuntu etc. people enabling -Wformat-security by default will dislike 
> this sort of change.  I disagree with their choice, but it should be 
> possible to fix i18n issues in a way that is simultaneously friendly to 
> -Wformat-security - in particular, including strings directly in 
> diagnostic function calls whenever possible so that they can actually be 
> checked at compile time.
> 
> This patch suffers from doing far too many things at once.  Some bits 
> might be OK, but various parts are wrong, meaning the whole patch has to 
> be rejected.  Please submit patches that do just one thing and cannot 
> sensibly be subdivided into smaller patches.
> 
> As examples of the things done wrong, strsignal returns a *string* that is 
> not a *format string* and that it is definitely incorrect to treat as a 
> format string.  %e and %n specs are not documented (in the comment in 
> gcc.c that documents specs, or in the unfortunate duplicate documentation 
> in invoke.texi) as taking format strings; if you change the 
> implementation, you must change the interface documentation.  Likewise for 
> other cases of action at a distance in this patch: if you change something 
> from a verbatim string to a format string at the site where the string is 
> called, you must make sure the interface documentation describes this 
> change, and that the relevant strings are marked as gcc-internal-format in 
> gcc.pot to indicate this to translators, and confirm that you have checked 
> that this is appropriate for all callers.
> 
This part is to fix some C++ i18n problems. Bootstrap with no regressions on the 
current trunk.
Is it ok?




[-- Attachment #2: ChangeLog2 --]
[-- Type: text/plain, Size: 669 bytes --]

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

	* cp-tree.h (expr_list): New type.
	(impl_conv_rhs): New type.
	(build_x_compound_expr_from_list, convert_for_initialization): Adjust
	prototype.
	(typeck.c (convert_arguments): Use impl_conv_rhs and emit the
	diagnostics for easy translation. Change caller.
	(convert_for_initialization): Use impl_conv_rhs.
	(build_x_compound_expr_from_list): Use expr_list and emit the
	diagnostics for easy translation. Change caller.
	* decl.c (bad_spec): New enum.
	(bad_specifiers): Use it and emit the diagnostics for easy
	translation. Change caller.
	* pt.c (coerce_template_parms): Put the diagnostics in full sentence.



[-- Attachment #3: part2.patch --]
[-- Type: text/x-patch, Size: 18935 bytes --]

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 160130)
+++ cp/cp-tree.h	(working copy)
@@ -418,6 +418,24 @@ typedef enum readonly_error_kind
   REK_DECREMENT
 } readonly_error_kind;
 
+/* Possible cases of expression list used by build_x_compound_expr_from_list. */
+typedef enum expr_list {
+  EL_INIT,		/* initializer */
+  EL_MEM_INIT,		/* member initializer */
+  EL_RETURN_VAL_INIT,	/* return value initializer */
+  EL_FUNC_CAST		/* function cast */
+} expr_list; 
+
+/* Possible cases of implicit bad rhs conversions. */
+typedef enum impl_conv_rhs {
+  ICR_DEFAULT_ARGUMENT,	/* default argument */
+  ICR_CONVERTING,	/* converting */
+  ICR_INIT,		/* initialization */
+  ICR_ARGPASS,		/* argument passing */
+  ICR_RETURN,		/* return */
+  ICR_ASSIGN		/* assignment */
+} impl_conv_rhs;
+
 /* Macros for access to language-specific slots in an identifier.  */
 
 #define IDENTIFIER_NAMESPACE_BINDINGS(NODE)	\
@@ -5387,7 +5405,7 @@ extern tree cp_build_unary_op           
 extern tree unary_complex_lvalue		(enum tree_code, tree);
 extern tree build_x_conditional_expr		(tree, tree, tree, 
                                                  tsubst_flags_t);
-extern tree build_x_compound_expr_from_list	(tree, const char *);
+extern tree build_x_compound_expr_from_list	(tree, expr_list);
 extern tree build_x_compound_expr_from_vec	(VEC(tree,gc) *, const char *);
 extern tree build_x_compound_expr		(tree, tree, tsubst_flags_t);
 extern tree build_compound_expr                 (location_t, tree, tree);
@@ -5402,7 +5420,7 @@ extern tree build_x_modify_expr			(tree,
 extern tree cp_build_modify_expr		(tree, enum tree_code, tree,
 						 tsubst_flags_t);
 extern tree convert_for_initialization		(tree, tree, tree, int,
-						 const char *, tree, int,
+						 impl_conv_rhs, tree, int,
                                                  tsubst_flags_t);
 extern int comp_ptr_ttypes			(tree, tree);
 extern bool comp_ptr_ttypes_const		(tree, tree);
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 160130)
+++ cp/typeck.c	(working copy)
@@ -44,7 +44,7 @@ along with GCC; see the file COPYING3.  
 
 static tree pfn_from_ptrmemfunc (tree);
 static tree delta_from_ptrmemfunc (tree);
-static tree convert_for_assignment (tree, tree, const char *, tree, int,
+static tree convert_for_assignment (tree, tree, impl_conv_rhs, tree, int,
 				    tsubst_flags_t, int);
 static tree cp_pointer_int_sum (enum tree_code, tree, tree);
 static tree rationalize_conditional_expr (enum tree_code, tree, 
@@ -3473,7 +3473,7 @@ convert_arguments (tree typelist, VEC(tr
 	    {
 	      parmval = convert_for_initialization
 		(NULL_TREE, type, val, flags,
-		 "argument passing", fndecl, i, complain);
+		 ICR_ARGPASS, fndecl, i, complain);
 	      parmval = convert_for_arg_passing (type, parmval);
 	    }
 
@@ -5466,14 +5466,34 @@ build_x_conditional_expr (tree ifexp, tr
 /* Given a list of expressions, return a compound expression
    that performs them all and returns the value of the last of them.  */
 
-tree build_x_compound_expr_from_list (tree list, const char *msg)
+tree
+build_x_compound_expr_from_list (tree list, expr_list exp)
 {
   tree expr = TREE_VALUE (list);
 
   if (TREE_CHAIN (list))
     {
-      if (msg)
-	permerror (input_location, "%s expression list treated as compound expression", msg);
+      switch (exp)
+	{
+	  case EL_INIT:
+	    permerror (input_location, "initializer expression "
+	    			       "list treated as compound expression");
+	    break;
+	  case EL_MEM_INIT:
+	    permerror (input_location, "member initializer expression "
+	    			       "list treated as compound expression");
+	    break;
+	  case EL_RETURN_VAL_INIT:
+	    permerror (input_location, "return value initializer expression "
+	    			       "list treated as compound expression");
+	    break;
+	  case EL_FUNC_CAST:
+	    permerror (input_location, "function cast expression "
+	    			       "list treated as compound expression");
+	    break;
+	  default:
+	    gcc_unreachable ();
+	}
 
       for (list = TREE_CHAIN (list); list; list = TREE_CHAIN (list))
 	expr = build_x_compound_expr (expr, TREE_VALUE (list), 
@@ -6747,10 +6767,10 @@ cp_build_modify_expr (tree lhs, enum tre
     /* Calls with INIT_EXPR are all direct-initialization, so don't set
        LOOKUP_ONLYCONVERTING.  */
     newrhs = convert_for_initialization (lhs, olhstype, newrhs, LOOKUP_NORMAL,
-					 "initialization", NULL_TREE, 0,
+					 ICR_INIT, NULL_TREE, 0,
                                          complain);
   else
-    newrhs = convert_for_assignment (olhstype, newrhs, "assignment",
+    newrhs = convert_for_assignment (olhstype, newrhs, ICR_ASSIGN,
 				     NULL_TREE, 0, complain, LOOKUP_IMPLICIT);
 
   if (!same_type_p (lhstype, olhstype))
@@ -7157,7 +7177,7 @@ delta_from_ptrmemfunc (tree t)
 
 static tree
 convert_for_assignment (tree type, tree rhs,
-			const char *errtype, tree fndecl, int parmnum,
+			impl_conv_rhs errtype, tree fndecl, int parmnum,
 			tsubst_flags_t complain, int flags)
 {
   tree rhstype;
@@ -7194,23 +7214,25 @@ convert_for_assignment (tree type, tree 
   if (c_dialect_objc ())
     {
       int parmno;
+      tree selector;
       tree rname = fndecl;
 
-      if (!strcmp (errtype, "assignment"))
-	parmno = -1;
-      else if (!strcmp (errtype, "initialization"))
-	parmno = -2;
-      else
-	{
-	  tree selector = objc_message_selector ();
-
-	  parmno = parmnum;
-
-	  if (selector && parmno > 1)
-	    {
-	      rname = selector;
-	      parmno -= 1;
-	    }
+      switch (errtype)
+        {
+	  case ICR_ASSIGN:
+	    parmno = -1;
+	    break;
+	  case ICR_INIT:
+	    parmno = -2;
+	    break;
+	  default:
+	    selector = objc_message_selector ();
+	    parmno = parmnum;
+	    if (selector && parmno > 1)
+	      {
+		rname = selector;
+		parmno -= 1;
+	      }
 	}
 
       if (objc_compare_types (type, rhstype, parmno, rname))
@@ -7247,8 +7269,35 @@ convert_for_assignment (tree type, tree 
 		error ("cannot convert %qT to %qT for argument %qP to %qD",
 		       rhstype, type, parmnum, fndecl);
 	      else
-		error ("cannot convert %qT to %qT in %s", rhstype, type,
-		       errtype);
+		switch (errtype)
+		  {
+		    case ICR_DEFAULT_ARGUMENT:
+		      error ("cannot convert %qT to %qT in default argument",
+			     rhstype, type);
+		      break;
+		    case ICR_CONVERTING:
+		      error ("cannot convert %qT to %qT in converting",
+			     rhstype, type);
+		      break;
+		    case ICR_INIT:
+		      error ("cannot convert %qT to %qT in initialization",
+			     rhstype, type);
+		      break;
+		    case ICR_ARGPASS:
+		      error ("cannot convert %qT to %qT in argument passing",
+			     rhstype, type);
+		      break;
+		    case ICR_RETURN:
+		      error ("cannot convert %qT to %qT in return",
+			     rhstype, type);
+		      break;
+		    case ICR_ASSIGN:
+		      error ("cannot convert %qT to %qT in assignment",
+			     rhstype, type);
+		      break;
+		    default:
+		      gcc_unreachable();
+		  }
 	    }
 	  return error_mark_node;
 	}
@@ -7260,9 +7309,41 @@ convert_for_assignment (tree type, tree 
 	  && coder == codel
 	  && check_missing_format_attribute (type, rhstype)
 	  && (complain & tf_warning))
-	warning (OPT_Wmissing_format_attribute,
-		 "%s might be a candidate for a format attribute",
-		 errtype);
+	switch (errtype)
+	  {
+	    case ICR_DEFAULT_ARGUMENT:
+	      warning (OPT_Wmissing_format_attribute,
+		       "default argument might be a candidate "
+		       "for a format attribute");
+	      break;
+	    case ICR_CONVERTING:
+	      warning (OPT_Wmissing_format_attribute,
+		       "converting might be a candidate "
+		       "for a format attribute");
+	      break;
+	    case ICR_INIT:
+	      warning (OPT_Wmissing_format_attribute,
+		       "initialization might be a candidate "
+		       "for a format attribute");
+	      break;
+	    case ICR_ARGPASS:
+	      warning (OPT_Wmissing_format_attribute,
+		       "argument passing might be a candidate "
+		       "for a format attribute");
+	      break;
+	    case ICR_RETURN:
+	      warning (OPT_Wmissing_format_attribute,
+		       "return might be a candidate for a format attribute");
+	      break;
+	    case ICR_ASSIGN:
+	      warning (OPT_Wmissing_format_attribute,
+		       "assignment might be a candidate "
+		       "for a format attribute");
+	      break;
+	    default:
+	      gcc_unreachable();
+	  }
+
     }
 
   /* If -Wparentheses, warn about a = b = c when a has type bool and b
@@ -7304,7 +7385,7 @@ convert_for_assignment (tree type, tree 
 
 tree
 convert_for_initialization (tree exp, tree type, tree rhs, int flags,
-			    const char *errtype, tree fndecl, int parmnum,
+			    impl_conv_rhs errtype, tree fndecl, int parmnum,
                             tsubst_flags_t complain)
 {
   enum tree_code codel = TREE_CODE (type);
@@ -7695,7 +7776,7 @@ check_return_expr (tree retval, bool *no
 	 to the type of return value's location to handle the
 	 case that functype is smaller than the valtype.  */
       retval = convert_for_initialization
-	(NULL_TREE, functype, retval, flags, "return", NULL_TREE, 0,
+	(NULL_TREE, functype, retval, flags, ICR_RETURN, NULL_TREE, 0,
          tf_warning_or_error);
       retval = convert (valtype, retval);
 
Index: cp/init.c
===================================================================
--- cp/init.c	(revision 160130)
+++ cp/init.c	(working copy)
@@ -526,7 +526,7 @@ perform_member_init (tree member, tree i
       else if (TREE_CODE (init) == TREE_LIST)
 	/* There was an explicit member initialization.  Do some work
 	   in that case.  */
-	init = build_x_compound_expr_from_list (init, "member initializer");
+	init = build_x_compound_expr_from_list (init, EL_MEM_INIT);
 
       if (init)
 	finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 160130)
+++ cp/decl.c	(working copy)
@@ -56,6 +56,14 @@ along with GCC; see the file COPYING3.  
 #include "splay-tree.h"
 #include "plugin.h"
 
+/* Possible cases of bad specifiers type used by bad_specifiers. */
+enum bad_spec {
+  BS_VAR,    /* variable */
+  BS_PARM,   /* parameter */
+  BS_TYPE,   /* type */
+  BS_FIELD   /* field */
+};
+
 static tree grokparms (tree parmlist, tree *);
 static const char *redeclaration_error_message (tree, tree);
 
@@ -71,7 +79,7 @@ static void record_unknown_type (tree, c
 static tree builtin_function_1 (tree, tree, bool);
 static tree build_library_fn_1 (tree, enum tree_code, tree);
 static int member_function_or_else (tree, tree, enum overload_flags);
-static void bad_specifiers (tree, const char *, int, int, int, int,
+static void bad_specifiers (tree, enum bad_spec, int, int, int, int,
 			    int);
 static void check_for_uninitialized_const_var (tree);
 static hashval_t typename_hash (const void *);
@@ -4437,7 +4445,7 @@ grok_reference_init (tree decl, tree typ
     }
 
   if (TREE_CODE (init) == TREE_LIST)
-    init = build_x_compound_expr_from_list (init, "initializer");
+    init = build_x_compound_expr_from_list (init, EL_INIT);
 
   if (TREE_CODE (TREE_TYPE (type)) != ARRAY_TYPE
       && TREE_CODE (TREE_TYPE (init)) == ARRAY_TYPE)
@@ -5657,7 +5665,7 @@ cp_finish_decl (tree decl, tree init, bo
 	  return;
 	}
       if (TREE_CODE (init) == TREE_LIST)
-	init = build_x_compound_expr_from_list (init, "initializer");
+	init = build_x_compound_expr_from_list (init, EL_INIT);
       if (describable_type (init))
 	{
 	  type = TREE_TYPE (decl) = do_auto_deduction (type, init, auto_node);
@@ -6573,21 +6581,54 @@ member_function_or_else (tree ctype, tre
 
 static void
 bad_specifiers (tree object,
-		const char* type,
+		enum bad_spec type,
 		int virtualp,
 		int quals,
 		int inlinep,
 		int friendp,
 		int raises)
 {
-  if (virtualp)
-    error ("%qD declared as a %<virtual%> %s", object, type);
-  if (inlinep)
-    error ("%qD declared as an %<inline%> %s", object, type);
-  if (quals)
-    error ("%<const%> and %<volatile%> function specifiers on "
-	   "%qD invalid in %s declaration",
-	   object, type);
+  switch (type)
+    {
+      case BS_VAR:
+	if (virtualp)
+	  error ("%qD declared as a %<virtual%> variable", object);
+	if (inlinep)
+	  error ("%qD declared as an %<inline%> variable", object);
+	if (quals)
+	  error ("%<const%> and %<volatile%> function specifiers on "
+	         "%qD invalid in variable declaration", object);
+	break;
+      case BS_PARM:
+	if (virtualp)
+	  error ("%qD declared as a %<virtual%> parameter", object);
+	if (inlinep)
+	  error ("%qD declared as an %<inline%> parameter", object);
+	if (quals)
+	  error ("%<const%> and %<volatile%> function specifiers on "
+	  	 "%qD invalid in parameter declaration", object);
+	break;
+      case BS_TYPE:
+	if (virtualp)
+	  error ("%qD declared as a %<virtual%> type", object);
+	if (inlinep)
+	  error ("%qD declared as an %<inline%> type", object);
+	if (quals)
+	  error ("%<const%> and %<volatile%> function specifiers on "
+	  	 "%qD invalid in type declaration", object);
+	break;
+      case BS_FIELD:
+	if (virtualp)
+	  error ("%qD declared as a %<virtual%> field", object);
+	if (inlinep)
+	  error ("%qD declared as an %<inline%> field", object);
+	if (quals)
+	  error ("%<const%> and %<volatile%> function specifiers on "
+	  	 "%qD invalid in field declaration", object);
+	break;
+      default:
+        gcc_unreachable();
+    }
   if (friendp)
     error ("%q+D declared as a friend", object);
   if (raises
@@ -9107,7 +9148,7 @@ grokdeclarator (const cp_declarator *dec
 	  || (typedef_decl && C_TYPEDEF_EXPLICITLY_SIGNED (typedef_decl)))
 	C_TYPEDEF_EXPLICITLY_SIGNED (decl) = 1;
 
-      bad_specifiers (decl, "type", virtualp,
+      bad_specifiers (decl, BS_TYPE, virtualp,
 		      memfn_quals != TYPE_UNQUALIFIED,
 		      inlinep, friendp, raises != NULL_TREE);
 
@@ -9293,7 +9334,7 @@ grokdeclarator (const cp_declarator *dec
       {
 	decl = cp_build_parm_decl (unqualified_id, type);
 
-	bad_specifiers (decl, "parameter", virtualp,
+	bad_specifiers (decl, BS_PARM, virtualp,
 			memfn_quals != TYPE_UNQUALIFIED,
 			inlinep, friendp, raises != NULL_TREE);
       }
@@ -9571,7 +9612,7 @@ grokdeclarator (const cp_declarator *dec
 		  }
 	      }
 
-	    bad_specifiers (decl, "field", virtualp,
+	    bad_specifiers (decl, BS_FIELD, virtualp,
 			    memfn_quals != TYPE_UNQUALIFIED,
 			    inlinep, friendp, raises != NULL_TREE);
 	  }
@@ -9694,7 +9735,7 @@ grokdeclarator (const cp_declarator *dec
 			    initialized,
 			    (type_quals & TYPE_QUAL_CONST) != 0,
 			    ctype ? ctype : in_namespace);
-	bad_specifiers (decl, "variable", virtualp,
+	bad_specifiers (decl, BS_VAR, virtualp,
 			memfn_quals != TYPE_UNQUALIFIED,
 			inlinep, friendp, raises != NULL_TREE);
 
Index: cp/typeck2.c
===================================================================
--- cp/typeck2.c	(revision 160130)
+++ cp/typeck2.c	(working copy)
@@ -728,8 +728,7 @@ store_init_value (tree decl, tree init, 
 	   && TREE_TYPE (init) != unknown_type_node)
     {
       if (TREE_CODE (decl) == RESULT_DECL)
-	init = build_x_compound_expr_from_list (init,
-						"return value initializer");
+	init = build_x_compound_expr_from_list (init, EL_RETURN_VAL_INIT);
       else if (TREE_CODE (init) == TREE_LIST
 	       && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
 	{
@@ -738,7 +737,7 @@ store_init_value (tree decl, tree init, 
 	}
       else
 	/* We get here with code like `int a (2);' */
-	init = build_x_compound_expr_from_list (init, "initializer");
+	init = build_x_compound_expr_from_list (init, EL_INIT);
     }
 
   /* End of special C++ code.  */
@@ -909,7 +908,7 @@ digest_init_r (tree type, tree init, boo
       if (cxx_dialect != cxx98 && nested)
 	check_narrowing (type, init);
       init = convert_for_initialization (0, type, init, flags,
-					 "initialization", NULL_TREE, 0,
+					 ICR_INIT, NULL_TREE, 0,
 					 tf_warning_or_error);
       exp = &init;
 
@@ -963,7 +962,7 @@ digest_init_r (tree type, tree init, boo
 
       return convert_for_initialization (NULL_TREE, type, init,
 					 flags,
-					 "initialization", NULL_TREE, 0,
+					 ICR_INIT, NULL_TREE, 0,
                                          tf_warning_or_error);
     }
 }
@@ -1598,7 +1597,7 @@ build_functional_cast (tree exp, tree pa
 	return cp_convert (type, integer_zero_node);
 
       /* This must build a C cast.  */
-      parms = build_x_compound_expr_from_list (parms, "functional cast");
+      parms = build_x_compound_expr_from_list (parms, EL_FUNC_CAST);
       return cp_build_c_cast (type, parms, complain);
     }
 
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 160130)
+++ cp/pt.c	(working copy)
@@ -5922,15 +5922,15 @@ coerce_template_parms (tree parms,
     {
       if (complain & tf_error)
 	{
-          const char *or_more = "";
           if (variadic_p)
             {
-              or_more = " or more";
               --nparms;
+	      error ("wrong number of template arguments "
+		     "(%d, should be %d or more)", nargs, nparms);
             }
-
-	  error ("wrong number of template arguments (%d, should be %d%s)",
-                 nargs, nparms, or_more);
+	  else
+	     error ("wrong number of template arguments "
+		    "(%d, should be %d)", nargs, nparms);
 
 	  if (in_decl)
 	    error ("provided for %q+D", in_decl);
Index: cp/call.c
===================================================================
--- cp/call.c	(revision 160130)
+++ cp/call.c	(working copy)
@@ -5339,7 +5339,7 @@ convert_default_arg (tree type, tree arg
     {
       arg = digest_init (type, arg);
       arg = convert_for_initialization (0, type, arg, LOOKUP_NORMAL,
-					"default argument", fn, parmnum,
+					ICR_DEFAULT_ARGUMENT, fn, parmnum,
                                         tf_warning_or_error);
     }
   else
@@ -5353,7 +5353,7 @@ convert_default_arg (tree type, tree arg
       if (!CONSTANT_CLASS_P (arg))
 	arg = unshare_expr (arg);
       arg = convert_for_initialization (0, type, arg, LOOKUP_NORMAL,
-					"default argument", fn, parmnum,
+					ICR_DEFAULT_ARGUMENT, fn, parmnum,
                                         tf_warning_or_error);
       arg = convert_for_arg_passing (type, arg);
     }
Index: cp/cvt.c
===================================================================
--- cp/cvt.c	(revision 160130)
+++ cp/cvt.c	(working copy)
@@ -481,7 +481,7 @@ convert_to_reference (tree reftype, tree
   else
     {
       rval = convert_for_initialization (NULL_TREE, type, expr, flags,
-					 "converting", 0, 0,
+					 ICR_CONVERTING, 0, 0,
                                          tf_warning_or_error);
       if (rval == NULL_TREE || rval == error_mark_node)
 	return rval;