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

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

[Shujing Zhao]

[-- 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;

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

[Joseph S. Myers]

On Tue, 8 Jun 2010, Shujing Zhao wrote:

> This part is to fix some C++ i18n problems. Bootstrap with no regressions on
> the current trunk.
> Is it ok?

OK if there are no objections from C++ front-end maintainers within 48 
hours.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Gabriel Dos Reis]

On Tue, Jun 8, 2010 at 5:42 AM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Tue, 8 Jun 2010, Shujing Zhao wrote:
>
>> This part is to fix some C++ i18n problems. Bootstrap with no regressions on
>> the current trunk.
>> Is it ok?
>
> OK if there are no objections from C++ front-end maintainers within 48
> hours.
OK.

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

[Jason Merrill]

On 06/08/2010 12:10 AM, Shujing Zhao wrote:
> 	* cp-tree.h (expr_list): New type.

Please call this expr_list_kind so people don't think it's a TREE_LIST. 
  Or don't bother giving it a name and just pass it as an int.

> 	(impl_conv_rhs): New type.

Why is this different in name and capitalization from impl_conv in the C 
front end?

> 	* decl.c (bad_spec): New enum.

This enum isn't telling us which bad specs we're dealing with, so the 
name is misleading.  Call it something like "bad_spec_place" or, as 
above, don't give it a name.

> +	  case EL_FUNC_CAST:
> +	    permerror (input_location, "function cast expression "
> +	    			       "list treated as compound expression");

"functional cast", not "function cast".

>        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);

I believe this code is a relic of the old named return value extension, 
and can be removed along with EL_RETURN_VAL_INIT.

While we're messing with these messages I'd also like to reword them a 
bit: rather than

"initializer expression list treated as compound expression"

let's say

"expression list treated as compound expression in initializer"

and so on.

> +		    case ICR_DEFAULT_ARGUMENT:
> +		      error ("cannot convert %qT to %qT in default argument",
> +			     rhstype, type);
> +		    case ICR_ARGPASS:
> +		      error ("cannot convert %qT to %qT in argument passing",
> +			     rhstype, type);
> +		      break;

These cases are unreachable, as they are covered by the previous if 
(fndecl) test.  Either remove the if (fndecl) message and use 
parmnum/fndecl in these cases, or just use gcc_unreachable() in these cases.

> +		      error ("cannot convert %qT to %qT in converting",
> +			     rhstype, type);

Drop "in converting".

> +	    case ICR_DEFAULT_ARGUMENT:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "default argument might be a candidate "
> +		       "for a format attribute");

"parameter %d of %qD might be [...]", parmnum, fndecl

> +	      break;
> +	    case ICR_CONVERTING:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "converting might be a candidate "
> +		       "for a format attribute");

"target of conversion might be [...]"

> +	      break;
> +	    case ICR_INIT:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "initialization might be a candidate "
> +		       "for a format attribute");

"target of initialization might be..."

> +	      break;
> +	    case ICR_ARGPASS:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "argument passing might be a candidate "
> +		       "for a format attribute");
> +	      break;

Same as ICR_DEFAULT_ARGUMENT.

> +	    case ICR_RETURN:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "return might be a candidate for a format attribute");
> +	      break;

"return type might..."

> +	    case ICR_ASSIGN:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "assignment might be a candidate "
> +		       "for a format attribute");

"left-hand side of assignment might..."

Jason

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

[Jason Merrill]

On 06/08/2010 12:10 AM, Shujing Zhao wrote:
> 	* cp-tree.h (expr_list): New type.

Please call this expr_list_kind so people don't think it's a TREE_LIST. 
  Or don't bother giving it a name and just pass it as an int.

> 	(impl_conv_rhs): New type.

Why is this different in name and capitalization from impl_conv in the C 
front end?

> 	* decl.c (bad_spec): New enum.

This enum isn't telling us which bad specs we're dealing with, so the 
name is misleading.  Call it something like "bad_spec_place" or, as 
above, don't give it a name.

> +	  case EL_FUNC_CAST:
> +	    permerror (input_location, "function cast expression "
> +	    			       "list treated as compound expression");

"functional cast", not "function cast".

>        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);

I believe this code is a relic of the old named return value extension, 
and can be removed along with EL_RETURN_VAL_INIT.

While we're messing with these messages I'd also like to reword them a 
bit: rather than

"initializer expression list treated as compound expression"

let's say

"expression list treated as compound expression in initializer"

and so on.

> +		    case ICR_DEFAULT_ARGUMENT:
> +		      error ("cannot convert %qT to %qT in default argument",
> +			     rhstype, type);
> +		    case ICR_ARGPASS:
> +		      error ("cannot convert %qT to %qT in argument passing",
> +			     rhstype, type);
> +		      break;

These cases are unreachable, as they are covered by the previous if 
(fndecl) test.  Either remove the if (fndecl) message and use 
parmnum/fndecl in these cases, or just use gcc_unreachable() in these cases.

> +		      error ("cannot convert %qT to %qT in converting",
> +			     rhstype, type);

Drop "in converting".

> +	    case ICR_DEFAULT_ARGUMENT:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "default argument might be a candidate "
> +		       "for a format attribute");

"parameter %d of %qD might be [...]", parmnum, fndecl

> +	      break;
> +	    case ICR_CONVERTING:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "converting might be a candidate "
> +		       "for a format attribute");

"target of conversion might be [...]"

> +	      break;
> +	    case ICR_INIT:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "initialization might be a candidate "
> +		       "for a format attribute");

"target of initialization might be..."

> +	      break;
> +	    case ICR_ARGPASS:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "argument passing might be a candidate "
> +		       "for a format attribute");
> +	      break;

Same as ICR_DEFAULT_ARGUMENT.

> +	    case ICR_RETURN:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "return might be a candidate for a format attribute");
> +	      break;

"return type might..."

> +	    case ICR_ASSIGN:
> +	      warning (OPT_Wmissing_format_attribute,
> +		       "assignment might be a candidate "
> +		       "for a format attribute");

"left-hand side of assignment might..."

Jason

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

[Shujing Zhao]

On 06/09/2010 03:06 AM, Jason Merrill wrote:
> On 06/08/2010 12:10 AM, Shujing Zhao wrote:
>>     * cp-tree.h (expr_list): New type.
> 
> Please call this expr_list_kind so people don't think it's a TREE_LIST. 
>  Or don't bother giving it a name and just pass it as an int.
> 
>>     (impl_conv_rhs): New type.
> 
> Why is this different in name and capitalization from impl_conv in the C 
> front end?
There is an implicit conversion string parameter at function "convert_to_void". 
It maybe be used as an enumeration to emit the diagnostic messages some day. (I 
have a plan to do that, but it hasn't be finished). That enumeration maybe be 
named "impl_conv_void" etc. The above one is named "impl_conv_rhs" to 
distinguish it from "impl_conv_void".
Naming the capitalization members is to keep consistently with the other 
enumeration definition in c++.
> 
......
> 
>>        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);
> 
> I believe this code is a relic of the old named return value extension, 
> and can be removed along with EL_RETURN_VAL_INIT.
Do you mean remove them as the following?

@@ -727,10 +727,7 @@
    else if (TREE_CODE (init) == TREE_LIST
            && TREE_TYPE (init) != unknown_type_node)
      {
-      if (TREE_CODE (decl) == RESULT_DECL)
-       init = build_x_compound_expr_from_list (init,
-                                               "return value initializer");
-      else if (TREE_CODE (init) == TREE_LIST
+      if (TREE_CODE (init) == TREE_LIST
                && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
         {
           error ("cannot initialize arrays using this syntax");

  ......

Thanks
Pearly

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

[Jason Merrill]

On 06/08/2010 11:04 PM, Shujing Zhao wrote:
> Do you mean remove them as the following?
>
> @@ -727,10 +727,7 @@
>    else if (TREE_CODE (init) == TREE_LIST
>            && TREE_TYPE (init) != unknown_type_node)
>      {
> -      if (TREE_CODE (decl) == RESULT_DECL)
> -       init = build_x_compound_expr_from_list (init,
> -                                               "return value initializer");
> -      else if (TREE_CODE (init) == TREE_LIST
> +      if (TREE_CODE (init) == TREE_LIST
>                && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
>         {
>           error ("cannot initialize arrays using this syntax");

Yes.  But better would be to replace it with

gcc_assert (TREE_CODE (decl) != RESULT_DECL);

Jason

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

[Shujing Zhao]

On 06/09/2010 03:06 AM, Jason Merrill wrote:
> 
>> +            case ICR_DEFAULT_ARGUMENT:
>> +              error ("cannot convert %qT to %qT in default argument",
>> +                 rhstype, type);
>> +            case ICR_ARGPASS:
>> +              error ("cannot convert %qT to %qT in argument passing",
>> +                 rhstype, type);
>> +              break;
> 
> These cases are unreachable, as they are covered by the previous if 
> (fndecl) test.  Either remove the if (fndecl) message and use 
> parmnum/fndecl in these cases, or just use gcc_unreachable() in these 
> cases.
> 

At the test case testsuite/g++.old-deja/g++.mike/p10769b.C, the 'fndecl' is 
NULL, while the errtype is ICR_ARGPASS (argument passing). Is there something 
wrong at argument passing? With my experience, I can't figure it out. Thank you 
in advance if you can give me some guidance.

Thanks
Pearly

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

[Jason Merrill]

On 06/09/2010 04:13 AM, Shujing Zhao wrote:
> On 06/09/2010 03:06 AM, Jason Merrill wrote:
>>
>>> + case ICR_DEFAULT_ARGUMENT:
>>> + error ("cannot convert %qT to %qT in default argument",
>>> + rhstype, type);
>>> + case ICR_ARGPASS:
>>> + error ("cannot convert %qT to %qT in argument passing",
>>> + rhstype, type);
>>> + break;
>>
>> These cases are unreachable, as they are covered by the previous if
>> (fndecl) test. Either remove the if (fndecl) message and use
>> parmnum/fndecl in these cases, or just use gcc_unreachable() in these
>> cases.
>
> At the test case testsuite/g++.old-deja/g++.mike/p10769b.C, the 'fndecl'
> is NULL, while the errtype is ICR_ARGPASS (argument passing).

Ah, of course, when you're calling a function pointer you don't have a 
declaration.  Is parmnum still set?

Jason

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

[Shujing Zhao]

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

On 06/09/2010 08:55 PM, Jason Merrill wrote:
> On 06/09/2010 04:13 AM, Shujing Zhao wrote:
>> On 06/09/2010 03:06 AM, Jason Merrill wrote:
>>>
>>>> + case ICR_DEFAULT_ARGUMENT:
>>>> + error ("cannot convert %qT to %qT in default argument",
>>>> + rhstype, type);
>>>> + case ICR_ARGPASS:
>>>> + error ("cannot convert %qT to %qT in argument passing",
>>>> + rhstype, type);
>>>> + break;
>>>
>>> These cases are unreachable, as they are covered by the previous if
>>> (fndecl) test. Either remove the if (fndecl) message and use
>>> parmnum/fndecl in these cases, or just use gcc_unreachable() in these
>>> cases.
>>
>> At the test case testsuite/g++.old-deja/g++.mike/p10769b.C, the 'fndecl'
>> is NULL, while the errtype is ICR_ARGPASS (argument passing).
> 
> Ah, of course, when you're calling a function pointer you don't have a 
> declaration.  Is parmnum still set?

Jason, the pramnum isn't set too. Since if function pointer is passed, the bad 
conversion maybe still exist. The above cases still need be kept to error. 
Consider this status, the warning for a format attribute are changed to depend 
the value of (fndecl).
All the others are changed as your comment.
Retested on i686-pc-linux-gnu and no regression.
Is it ok?

Thanks
Pearly


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

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

	* cp-tree.h (expr_list_kind): 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 and change caller.
	(build_x_compound_expr_from_list): Use expr_list_kind and emit the
	diagnostics for easy translation. Change caller.
	* decl.c (bad_spec_place): 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: 19258 bytes --]

Index: cp-tree.h
===================================================================
--- cp-tree.h	(revision 160431)
+++ cp-tree.h	(working copy)
@@ -418,6 +418,23 @@ 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_kind {
+  ELK_INIT,		/* initializer */
+  ELK_MEM_INIT,		/* member initializer */
+  ELK_FUNC_CAST		/* functional cast */
+} expr_list_kind; 
+
+/* 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)	\
@@ -5400,7 +5417,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_kind);
 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);
@@ -5415,7 +5432,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: call.c
===================================================================
--- call.c	(revision 160431)
+++ call.c	(working copy)
@@ -5387,7 +5387,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
@@ -5401,7 +5401,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: cvt.c
===================================================================
--- cvt.c	(revision 160431)
+++ 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;
Index: decl.c
===================================================================
--- decl.c	(revision 160431)
+++ 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_place {
+  BSP_VAR,    /* variable */
+  BSP_PARM,   /* parameter */
+  BSP_TYPE,   /* type */
+  BSP_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_place, int, int, int, int,
 			    int);
 static void check_for_uninitialized_const_var (tree);
 static hashval_t typename_hash (const void *);
@@ -4455,7 +4463,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, ELK_INIT);
 
   if (TREE_CODE (TREE_TYPE (type)) != ARRAY_TYPE
       && TREE_CODE (TREE_TYPE (init)) == ARRAY_TYPE)
@@ -5676,7 +5684,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, ELK_INIT);
       if (describable_type (init))
 	{
 	  type = TREE_TYPE (decl) = do_auto_deduction (type, init, auto_node);
@@ -6592,21 +6600,54 @@ member_function_or_else (tree ctype, tre
 
 static void
 bad_specifiers (tree object,
-		const char* type,
+		enum bad_spec_place 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 BSP_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 BSP_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 BSP_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 BSP_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
@@ -9128,7 +9169,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, BSP_TYPE, virtualp,
 		      memfn_quals != TYPE_UNQUALIFIED,
 		      inlinep, friendp, raises != NULL_TREE);
 
@@ -9314,7 +9355,7 @@ grokdeclarator (const cp_declarator *dec
       {
 	decl = cp_build_parm_decl (unqualified_id, type);
 
-	bad_specifiers (decl, "parameter", virtualp,
+	bad_specifiers (decl, BSP_PARM, virtualp,
 			memfn_quals != TYPE_UNQUALIFIED,
 			inlinep, friendp, raises != NULL_TREE);
       }
@@ -9592,7 +9633,7 @@ grokdeclarator (const cp_declarator *dec
 		  }
 	      }
 
-	    bad_specifiers (decl, "field", virtualp,
+	    bad_specifiers (decl, BSP_FIELD, virtualp,
 			    memfn_quals != TYPE_UNQUALIFIED,
 			    inlinep, friendp, raises != NULL_TREE);
 	  }
@@ -9715,7 +9756,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, BSP_VAR, virtualp,
 			memfn_quals != TYPE_UNQUALIFIED,
 			inlinep, friendp, raises != NULL_TREE);
 
Index: init.c
===================================================================
--- init.c	(revision 160431)
+++ 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, ELK_MEM_INIT);
 
       if (init)
 	finish_expr_stmt (cp_build_modify_expr (decl, INIT_EXPR, init,
Index: pt.c
===================================================================
--- pt.c	(revision 160431)
+++ 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: typeck2.c
===================================================================
--- typeck2.c	(revision 160431)
+++ typeck2.c	(working copy)
@@ -727,10 +727,9 @@ store_init_value (tree decl, tree init, 
   else if (TREE_CODE (init) == TREE_LIST
 	   && TREE_TYPE (init) != unknown_type_node)
     {
-      if (TREE_CODE (decl) == RESULT_DECL)
-	init = build_x_compound_expr_from_list (init,
-						"return value initializer");
-      else if (TREE_CODE (init) == TREE_LIST
+      gcc_assert (TREE_CODE (decl) != RESULT_DECL);
+
+      if (TREE_CODE (init) == TREE_LIST
 	       && TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
 	{
 	  error ("cannot initialize arrays using this syntax");
@@ -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, ELK_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);
     }
 }
@@ -1596,7 +1595,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, ELK_FUNC_CAST);
       return cp_build_c_cast (type, parms, complain);
     }
 
Index: typeck.c
===================================================================
--- typeck.c	(revision 160431)
+++ 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, 
@@ -3493,7 +3493,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);
 	    }
 
@@ -5486,14 +5486,30 @@ 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_kind 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 ELK_INIT:
+	    permerror (input_location, "expression list treated as compound "
+				       "expression in initializer");
+	    break;
+	  case ELK_MEM_INIT:
+	    permerror (input_location, "member initializer expression "
+	    			       "list treated as compound expression");
+	    break;
+	  case ELK_FUNC_CAST:
+	    permerror (input_location, "functional 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), 
@@ -6767,10 +6783,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))
@@ -7177,7 +7193,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;
@@ -7214,23 +7230,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))
@@ -7267,8 +7285,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_ARGPASS:
+		      error ("cannot convert %qT to %qT in argument passing",
+			     rhstype, type);
+		      break;
+		    case ICR_CONVERTING:
+		      error ("cannot convert %qT to %qT",
+			     rhstype, type);
+		      break;
+		    case ICR_INIT:
+		      error ("cannot convert %qT to %qT in initialization",
+			     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;
 	}
@@ -7280,9 +7325,51 @@ 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_ARGPASS:
+	      if (fndecl)
+		warning (OPT_Wmissing_format_attribute,
+			 "parameter %d of %qD might be a candidate "
+			 "for a format attribute", parmnum, fndecl);
+	      else
+		warning (OPT_Wmissing_format_attribute,
+			 "target of argument passing might be a candidate "
+			 "for a format attribute");
+	      break;
+	    case ICR_DEFAULT_ARGUMENT:
+	      if (fndecl)
+		warning (OPT_Wmissing_format_attribute,
+			 "parameter %d of %qD might be a candidate "
+		       	 "for a format attribute", parmnum, fndecl);
+	      else
+		warning (OPT_Wmissing_format_attribute,
+			 "default argument might be a candidate "
+			 "for a format attribute");
+	      break;
+	    case ICR_CONVERTING:
+	      warning (OPT_Wmissing_format_attribute,
+		       "target of conversion might be might be a candidate "
+		       "for a format attribute");
+	      break;
+	    case ICR_INIT:
+	      warning (OPT_Wmissing_format_attribute,
+		       "target of initialization might be a candidate "
+		       "for a format attribute");
+	      break;
+	    case ICR_RETURN:
+	      warning (OPT_Wmissing_format_attribute,
+		       "return type might be a candidate "
+		       "for a format attribute");
+	      break;
+	    case ICR_ASSIGN:
+	      warning (OPT_Wmissing_format_attribute,
+		       "left-hand side of 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
@@ -7324,7 +7411,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);
@@ -7715,7 +7802,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);
 

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

[Jason Merrill]

On 06/10/2010 02:33 AM, Shujing Zhao wrote:
> +	    permerror (input_location, "member initializer expression "
> +	    			       "list treated as compound expression");

expression list treated as compound expression in mem-initializer

> +	  case ELK_FUNC_CAST:
> +	    permerror (input_location, "functional cast expression "
> +	    			       "list treated as compound expression");

expression list treated as compound expression in functional cast

> +		warning (OPT_Wmissing_format_attribute,
> +			 "target of argument passing might be a candidate "
> +			 "for a format attribute");

parameter might be...

> +		warning (OPT_Wmissing_format_attribute,
> +			 "default argument might be a candidate "
> +			 "for a format attribute");

parameter might be...

OK with those changes.

Jason

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

[Shujing Zhao]

On 06/10/2010 08:47 PM, Jason Merrill wrote:
> On 06/10/2010 02:33 AM, Shujing Zhao wrote:
>> +        permerror (input_location, "member initializer expression "
>> +                           "list treated as compound expression");
> 
> expression list treated as compound expression in mem-initializer
> 
>> +      case ELK_FUNC_CAST:
>> +        permerror (input_location, "functional cast expression "
>> +                           "list treated as compound expression");
> 
> expression list treated as compound expression in functional cast
> 
Ok.
>> +        warning (OPT_Wmissing_format_attribute,
>> +             "target of argument passing might be a candidate "
>> +             "for a format attribute");
> 
> parameter might be...
> 
>> +        warning (OPT_Wmissing_format_attribute,
>> +             "default argument might be a candidate "
>> +             "for a format attribute");
> 
> parameter might be...

Ok. Fix this part to

+           case ICR_ARGPASS:
+           case ICR_DEFAULT_ARGUMENT:
+             if (fndecl)
+               warning (OPT_Wmissing_format_attribute,
+                        "parameter %d of %qD might be a candidate "
+                        "for a format attribute", parmnum, fndecl);
+             else
+               warning (OPT_Wmissing_format_attribute,
+                        "parameter might be a candidate "
+                        "for a format attribute");
+             break;


Committed to trunk at revision 160591.

Thanks
Pearly

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

[Manuel López-Ibáñez]

On 11 June 2010 05:59, Shujing Zhao <pearly.zhao@oracle.com> wrote:
>
> +           case ICR_ARGPASS:
> +           case ICR_DEFAULT_ARGUMENT:
> +             if (fndecl)
> +               warning (OPT_Wmissing_format_attribute,
> +                        "parameter %d of %qD might be a candidate "
> +                        "for a format attribute", parmnum, fndecl);
> +             else

You have to use %qP for parmnum, otherwise the offsets would be
inconsistent with other diagnostics. See a previous error in the same
function.

Manuel.

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

[Manuel López-Ibáñez]

On 9 June 2010 14:55, Jason Merrill <jason@redhat.com> wrote:
> On 06/09/2010 04:13 AM, Shujing Zhao wrote:
>>
>> On 06/09/2010 03:06 AM, Jason Merrill wrote:
>>>
>>>> + case ICR_DEFAULT_ARGUMENT:
>>>> + error ("cannot convert %qT to %qT in default argument",
>>>> + rhstype, type);
>>>> + case ICR_ARGPASS:
>>>> + error ("cannot convert %qT to %qT in argument passing",
>>>> + rhstype, type);
>>>> + break;
>>>
>>> These cases are unreachable, as they are covered by the previous if
>>> (fndecl) test. Either remove the if (fndecl) message and use
>>> parmnum/fndecl in these cases, or just use gcc_unreachable() in these
>>> cases.
>>
>> At the test case testsuite/g++.old-deja/g++.mike/p10769b.C, the 'fndecl'
>> is NULL, while the errtype is ICR_ARGPASS (argument passing).
>
> Ah, of course, when you're calling a function pointer you don't have a
> declaration.  Is parmnum still set?

Why not document that in the function comment? I would say that the comment

/* If FNDECL is non-NULL, we
   are doing the conversion in order to pass the PARMNUMth argument of
   FNDECL. */

is wrong.

In any case, I don't understand why we cannot pass down the name of
the pointer for diagnostics purposes since fndecl does not determine
anymore whether is a argument conversion.

Cheers,

Manuel

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

[Shujing Zhao]

On 06/11/2010 05:17 PM, Manuel López-Ibáñez wrote:
> On 9 June 2010 14:55, Jason Merrill <jason@redhat.com> wrote:
>> On 06/09/2010 04:13 AM, Shujing Zhao wrote:
>>> On 06/09/2010 03:06 AM, Jason Merrill wrote:
>>>>> + case ICR_DEFAULT_ARGUMENT:
>>>>> + error ("cannot convert %qT to %qT in default argument",
>>>>> + rhstype, type);
>>>>> + case ICR_ARGPASS:
>>>>> + error ("cannot convert %qT to %qT in argument passing",
>>>>> + rhstype, type);
>>>>> + break;
>>>> These cases are unreachable, as they are covered by the previous if
>>>> (fndecl) test. Either remove the if (fndecl) message and use
>>>> parmnum/fndecl in these cases, or just use gcc_unreachable() in these
>>>> cases.
>>> At the test case testsuite/g++.old-deja/g++.mike/p10769b.C, the 'fndecl'
>>> is NULL, while the errtype is ICR_ARGPASS (argument passing).
>> Ah, of course, when you're calling a function pointer you don't have a
>> declaration.  Is parmnum still set?
> 
> Why not document that in the function comment? I would say that the comment
> 
> /* If FNDECL is non-NULL, we
>    are doing the conversion in order to pass the PARMNUMth argument of
>    FNDECL. */
> 
> is wrong.
> 
> In any case, I don't understand why we cannot pass down the name of
> the pointer for diagnostics purposes since fndecl does not determine
> anymore whether is a argument conversion.
> 
I agree with your last comment to use %qP for parmnum.

About this issue, I think the comment is not wrong when FNDECL is non-NULL. How 
about to add the case when FNDECL is NULL? just as the following:

/* If FNDECL is non-NULL, we are doing the conversion in order to pass the 
PARMNUMth argument of FNDECL. If FNDECL is NULL, we are doing the conversion in 
function pointer argument passing, conversion in initialization, etc. */

If you agree with it, I'll submit the patch.

Thanks
Pearly

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

[Manuel López-Ibáñez]

On 11 June 2010 11:54, Shujing Zhao <pearly.zhao@oracle.com> wrote:
> On 06/11/2010 05:17 PM, Manuel López-Ibáńez wrote:
>>
>> On 9 June 2010 14:55, Jason Merrill <jason@redhat.com> wrote:
>>>
>>> On 06/09/2010 04:13 AM, Shujing Zhao wrote:
>>>>
>>>> On 06/09/2010 03:06 AM, Jason Merrill wrote:
>>>>>>
>>>>>> + case ICR_DEFAULT_ARGUMENT:
>>>>>> + error ("cannot convert %qT to %qT in default argument",
>>>>>> + rhstype, type);
>>>>>> + case ICR_ARGPASS:
>>>>>> + error ("cannot convert %qT to %qT in argument passing",
>>>>>> + rhstype, type);
>>>>>> + break;
>>>>>
>>>>> These cases are unreachable, as they are covered by the previous if
>>>>> (fndecl) test. Either remove the if (fndecl) message and use
>>>>> parmnum/fndecl in these cases, or just use gcc_unreachable() in these
>>>>> cases.
>>>>
>>>> At the test case testsuite/g++.old-deja/g++.mike/p10769b.C, the 'fndecl'
>>>> is NULL, while the errtype is ICR_ARGPASS (argument passing).
>>>
>>> Ah, of course, when you're calling a function pointer you don't have a
>>> declaration.  Is parmnum still set?
>>
>> Why not document that in the function comment? I would say that the
>> comment
>>
>> /* If FNDECL is non-NULL, we
>>   are doing the conversion in order to pass the PARMNUMth argument of
>>   FNDECL. */
>>
>> is wrong.
>>
>> In any case, I don't understand why we cannot pass down the name of
>> the pointer for diagnostics purposes since fndecl does not determine
>> anymore whether is a argument conversion.
>>
> I agree with your last comment to use %qP for parmnum.
>
> About this issue, I think the comment is not wrong when FNDECL is non-NULL.
> How about to add the case when FNDECL is NULL? just as the following:
>
> /* If FNDECL is non-NULL, we are doing the conversion in order to pass the
> PARMNUMth argument of FNDECL. If FNDECL is NULL, we are doing the conversion
> in function pointer argument passing, conversion in initialization, etc. */
>
> If you agree with it, I'll submit the patch.

In that function, fndecl is only used for providing better
diagnostics. Right now, the diagnostic is less informative when using
a pointer than when not. I think it should be equally informative.
That said, your proposal is better than nothing of course.

Manuel.

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

[Shujing Zhao]

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

On 06/11/2010 06:03 PM, Manuel López-Ibáñez wrote:
> On 11 June 2010 11:54, Shujing Zhao <pearly.zhao@oracle.com> wrote:
>> I agree with your last comment to use %qP for parmnum.
>>
>> About this issue, I think the comment is not wrong when FNDECL is non-NULL.
>> How about to add the case when FNDECL is NULL? just as the following:
>>
>> /* If FNDECL is non-NULL, we are doing the conversion in order to pass the
>> PARMNUMth argument of FNDECL. If FNDECL is NULL, we are doing the conversion
>> in function pointer argument passing, conversion in initialization, etc. */
>>
>> If you agree with it, I'll submit the patch.
> 
> In that function, fndecl is only used for providing better
> diagnostics. Right now, the diagnostic is less informative when using
> a pointer than when not. I think it should be equally informative.
> That said, your proposal is better than nothing of course.
> 
> Manuel.
Jason,

This patch is to use %qP for parnum and fix the comment of 
convert_for_assignment and convert_for_initialization.

Retested on 1686-pc-linux-gnu. Is it ok?

Thanks
Pearly

[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 187 bytes --]

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

	* typeck.c (convert_for_assignment): Fix comment. Change message
	format from %d to %qP.
	(convert_for_initialization): Fix comment.


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

Index: typeck.c
===================================================================
--- typeck.c	(revision 160644)
+++ typeck.c	(working copy)
@@ -7186,10 +7186,11 @@ delta_from_ptrmemfunc (tree t)
 }
 
 /* Convert value RHS to type TYPE as preparation for an assignment to
-   an lvalue of type TYPE.  ERRTYPE is a string to use in error
-   messages: "assignment", "return", etc.  If FNDECL is non-NULL, we
-   are doing the conversion in order to pass the PARMNUMth argument of
-   FNDECL.  */
+   an lvalue of type TYPE.  ERRTYPE indicates what kind of error the
+   implicit conversion is.  If FNDECL is non-NULL, we are doing the
+   conversion in order to pass the PARMNUMth argument of FNDECL.
+   If FNDECL is NULL, we are doing the conversion in function pointer
+   argument passing, conversion in initialization, etc. */
 
 static tree
 convert_for_assignment (tree type, tree rhs,
@@ -7331,7 +7332,7 @@ convert_for_assignment (tree type, tree 
 	    case ICR_DEFAULT_ARGUMENT:
 	      if (fndecl)
 		warning (OPT_Wmissing_format_attribute,
-			 "parameter %d of %qD might be a candidate "
+			 "parameter %qP of %qD might be a candidate "
 			 "for a format attribute", parmnum, fndecl);
 	      else
 		warning (OPT_Wmissing_format_attribute,
@@ -7386,7 +7387,7 @@ convert_for_assignment (tree type, tree 
 
 /* Convert RHS to be of type TYPE.
    If EXP is nonzero, it is the target of the initialization.
-   ERRTYPE is a string to use in error messages.
+   ERRTYPE indicates what kind of error the implicit conversion is.
 
    Two major differences between the behavior of
    `convert_for_assignment' and `convert_for_initialization'

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

[Jason Merrill]

OK.

Jason

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

[Shujing Zhao]

On 06/07/2010 09:27 PM, Joseph S. Myers wrote:
> On Mon, 7 Jun 2010, Shujing Zhao wrote:
> 
>> Ok. Since there is no obvious requirement to change them, I only take out the
>> part that fixes the existing 18n problems.
>> The following is the patch for C frontend.
> 
> The patch you attached - which was for fold-const.c and tree-inline.c, not 
> the C front end - is OK if it passed bootstrap with no regressions.
> 
Bootstrap with no regressions. Committed trunk at revision 160419.

Thanks
Pearly

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

[Joseph S. Myers]

On Mon, 7 Jun 2010, Shujing Zhao wrote:

> Ok. Since there is no obvious requirement to change them, I only take out the
> part that fixes the existing 18n problems.
> The following is the patch for C frontend.

The patch you attached - which was for fold-const.c and tree-inline.c, not 
the C front end - is OK if it passed bootstrap with no regressions.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Shujing Zhao]

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

On 06/05/2010 09:18 PM, Joseph S. Myers wrote:
> On Fri, 4 Jun 2010, Shujing Zhao wrote:
> 
>> Hi,
>>
>> This patch is to cleanup the codes that pass ("%s", message) to diagnostic
>> functions, such as error, warning etc. If there some format specifiers in
>> 'message', they would not be recognized. If it need be translated, 'message'
>> need be wrapped with _() explicitly, while 'message' could be translated in
>> diagnostic functions. I thinks pass the 'message' directly to those functions
>> would be better.
> 
> 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.
> 
Ok. Since there is no obvious requirement to change them, I only take out the 
part that fixes the existing 18n problems.
The following is the patch for C frontend.




[-- Attachment #2: ChangeLog1 --]
[-- Type: text/plain, Size: 241 bytes --]

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

	* fold-const.c (fold_comparison): Remove redundant parenthesis.
	* tree-inline.c (expand_call_inline): Pass translated return value of
	cgraph_inline_failed_string to diagnostic function.

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

Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 160130)
+++ tree-inline.c	(working copy)
@@ -3774,7 +3774,7 @@ expand_call_inline (basic_block bb, gimp
 	  && cgraph_global_info_ready)
 	{
 	  sorry ("inlining failed in call to %q+F: %s", fn,
-		 cgraph_inline_failed_string (reason));
+		 _(cgraph_inline_failed_string (reason)));
 	  sorry ("called from here");
 	}
       else if (warn_inline && DECL_DECLARED_INLINE_P (fn)
@@ -3785,7 +3785,7 @@ expand_call_inline (basic_block bb, gimp
 	       && cgraph_global_info_ready)
 	{
 	  warning (OPT_Winline, "inlining failed in call to %q+F: %s",
-		   fn, cgraph_inline_failed_string (reason));
+		   fn, _(cgraph_inline_failed_string (reason)));
 	  warning (OPT_Winline, "called from here");
 	}
       goto egress;
Index: fold-const.c
===================================================================
--- fold-const.c	(revision 160130)
+++ fold-const.c	(working copy)
@@ -8651,9 +8651,9 @@ fold_comparison (location_t loc, enum tr
 	  && (TREE_CODE (lhs) != INTEGER_CST
 	      || !TREE_OVERFLOW (lhs)))
 	{
-	  fold_overflow_warning (("assuming signed overflow does not occur "
+	  fold_overflow_warning ("assuming signed overflow does not occur "
 				  "when changing X +- C1 cmp C2 to "
-				  "X cmp C1 +- C2"),
+				  "X cmp C1 +- C2",
 				 WARN_STRICT_OVERFLOW_COMPARISON);
 	  return fold_build2_loc (loc, code, type, variable, lhs);
 	}

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

[Joseph S. Myers]

On Fri, 4 Jun 2010, Shujing Zhao wrote:

> Hi,
> 
> This patch is to cleanup the codes that pass ("%s", message) to diagnostic
> functions, such as error, warning etc. If there some format specifiers in
> 'message', they would not be recognized. If it need be translated, 'message'
> need be wrapped with _() explicitly, while 'message' could be translated in
> diagnostic functions. I thinks pass the 'message' directly to those functions
> would be better.

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.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Shujing Zhao]

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

Hi,

This patch is to cleanup the codes that pass ("%s", message) to diagnostic 
functions, such as error, warning etc. If there some format specifiers in 
'message', they would not be recognized. If it need be translated, 'message' 
need be wrapped with _() explicitly, while 'message' could be translated in 
diagnostic functions. I thinks pass the 'message' directly to those functions 
would be better.

This patch is also fix some i18n problems at C++ FE. It still uses enums to emit 
the diagnostics.

Tested with enable bootstrap on 1686-pc-linux-gnu.
Is it ok?

Thanks
Pearly

[-- Attachment #2: ChangeLog --]
[-- Type: text/plain, Size: 1398 bytes --]

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

	* fold-const.c (fold_comparison): Remove redundant parenthesis.
	(fold_undefer_overflow_warnings): Pass message variable to diagnostic
	functions directly.
	* gcc.c (do_spec_1): Likewise.
	* loop-iv.c (get_simple_loop_desc): Likewise.
	* tree-vrp.c (vrp_evaluate_conditional): Likewise.
	* tree-ssa-loop-niter.c (number_of_iterations_exit): Likewise.
	* rtl-error.c (_fatal_insn): Likewise.
	* final.c (output_operand_lossage): Likewise.
	* toplev.c (crash_signal): Likewise.
	* tree-inline.c (expand_call_inline): Pass translated return value of
	cgraph_inline_failed_string to diagnostic function.

gcc/cp/
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.
	(cp_build_unary_op): Pass message variable to error directly.
	* 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: final.patch --]
[-- Type: text/x-patch, Size: 28265 bytes --]

Index: loop-iv.c
===================================================================
--- loop-iv.c	(revision 160130)
+++ loop-iv.c	(working copy)
@@ -2963,19 +2963,17 @@ get_simple_loop_desc (struct loop *loop)
 	    {
 	      wording =
 		flag_unsafe_loop_optimizations
-		? N_("assuming that the loop is not infinite")
-		: N_("cannot optimize possibly infinite loops");
-	      warning (OPT_Wunsafe_loop_optimizations, "%s",
-		       gettext (wording));
+		? G_("assuming that the loop is not infinite")
+		: G_("cannot optimize possibly infinite loops");
+	      warning (OPT_Wunsafe_loop_optimizations, wording);
 	    }
 	  if (desc->assumptions)
 	    {
 	      wording =
 		flag_unsafe_loop_optimizations
-		? N_("assuming that the loop counter does not overflow")
-		: N_("cannot optimize loop, the loop counter may overflow");
-	      warning (OPT_Wunsafe_loop_optimizations, "%s",
-		       gettext (wording));
+		? G_("assuming that the loop counter does not overflow")
+		: G_("cannot optimize loop, the loop counter may overflow");
+	      warning (OPT_Wunsafe_loop_optimizations, wording);
 	    }
 	}
 
Index: tree-inline.c
===================================================================
--- tree-inline.c	(revision 160130)
+++ tree-inline.c	(working copy)
@@ -3774,7 +3774,7 @@ expand_call_inline (basic_block bb, gimp
 	  && cgraph_global_info_ready)
 	{
 	  sorry ("inlining failed in call to %q+F: %s", fn,
-		 cgraph_inline_failed_string (reason));
+		 _(cgraph_inline_failed_string (reason)));
 	  sorry ("called from here");
 	}
       else if (warn_inline && DECL_DECLARED_INLINE_P (fn)
@@ -3785,7 +3785,7 @@ expand_call_inline (basic_block bb, gimp
 	       && cgraph_global_info_ready)
 	{
 	  warning (OPT_Winline, "inlining failed in call to %q+F: %s",
-		   fn, cgraph_inline_failed_string (reason));
+		   fn, _(cgraph_inline_failed_string (reason)));
 	  warning (OPT_Winline, "called from here");
 	}
       goto egress;
Index: tree-vrp.c
===================================================================
--- tree-vrp.c	(revision 160130)
+++ tree-vrp.c	(working copy)
@@ -5792,7 +5792,7 @@ vrp_evaluate_conditional (enum tree_code
 	    location = input_location;
 	  else
 	    location = gimple_location (stmt);
-	  warning_at (location, OPT_Wstrict_overflow, "%s", warnmsg);
+	  warning_at (location, OPT_Wstrict_overflow, warnmsg);
 	}
     }
 
Index: tree-ssa-loop-niter.c
===================================================================
--- tree-ssa-loop-niter.c	(revision 160130)
+++ tree-ssa-loop-niter.c	(working copy)
@@ -1872,16 +1872,16 @@ number_of_iterations_exit (struct loop *
 	  : (integer_onep (iv0.step) || integer_all_onesp (iv0.step)))
         wording =
           flag_unsafe_loop_optimizations
-          ? N_("assuming that the loop is not infinite")
-          : N_("cannot optimize possibly infinite loops");
+          ? G_("assuming that the loop is not infinite")
+          : G_("cannot optimize possibly infinite loops");
       else
 	wording =
 	  flag_unsafe_loop_optimizations
-	  ? N_("assuming that the loop counter does not overflow")
-	  : N_("cannot optimize loop, the loop counter may overflow");
+	  ? G_("assuming that the loop counter does not overflow")
+	  : G_("cannot optimize loop, the loop counter may overflow");
 
       warning_at ((LOCATION_LINE (loc) > 0) ? loc : input_location,
-		  OPT_Wunsafe_loop_optimizations, "%s", gettext (wording));
+		  OPT_Wunsafe_loop_optimizations, wording);
     }
 
   return flag_unsafe_loop_optimizations;
Index: rtl-error.c
===================================================================
--- rtl-error.c	(revision 160130)
+++ rtl-error.c	(working copy)
@@ -100,7 +100,7 @@ void
 _fatal_insn (const char *msgid, const_rtx insn, const char *file, int line,
 	     const char *function)
 {
-  error ("%s", _(msgid));
+  error (msgid);
 
   /* The above incremented error_count, but isn't an error that we want to
      count, so reset it here.  */
Index: final.c
===================================================================
--- final.c	(revision 160130)
+++ final.c	(working copy)
@@ -3085,9 +3085,9 @@ output_operand_lossage (const char *cmsg
   vasprintf (&new_message, fmt_string, ap);
 
   if (this_is_asm_operands)
-    error_for_asm (this_is_asm_operands, "%s", new_message);
+    error_for_asm (this_is_asm_operands, new_message);
   else
-    internal_error ("%s", new_message);
+    internal_error (new_message);
 
   free (fmt_string);
   free (new_message);
Index: gcc.c
===================================================================
--- gcc.c	(revision 160130)
+++ gcc.c	(working copy)
@@ -5186,7 +5186,7 @@ do_spec_1 (const char *spec, int inswitc
 	      buf = (char *) alloca (p - q + 1);
 	      strncpy (buf, q, p - q);
 	      buf[p - q] = 0;
-	      error ("%s", _(buf));
+	      error (buf);
 	      return -1;
 	    }
 	    break;
@@ -5200,7 +5200,7 @@ do_spec_1 (const char *spec, int inswitc
 	      buf = (char *) alloca (p - q + 1);
 	      strncpy (buf, q, p - q);
 	      buf[p - q] = 0;
-	      inform (0, "%s", _(buf));
+	      inform (0, buf);
 	      if (*p)
 		p++;
 	    }
Index: fold-const.c
===================================================================
--- fold-const.c	(revision 160130)
+++ fold-const.c	(working copy)
@@ -263,7 +263,7 @@ fold_undefer_overflow_warnings (bool iss
     locus = input_location;
   else
     locus = gimple_location (stmt);
-  warning_at (locus, OPT_Wstrict_overflow, "%s", warnmsg);
+  warning_at (locus, OPT_Wstrict_overflow, warnmsg);
 }
 
 /* Stop deferring overflow warnings, ignoring any deferred
@@ -8651,9 +8651,9 @@ fold_comparison (location_t loc, enum tr
 	  && (TREE_CODE (lhs) != INTEGER_CST
 	      || !TREE_OVERFLOW (lhs)))
 	{
-	  fold_overflow_warning (("assuming signed overflow does not occur "
+	  fold_overflow_warning ("assuming signed overflow does not occur "
 				  "when changing X +- C1 cmp C2 to "
-				  "X cmp C1 +- C2"),
+				  "X cmp C1 +- C2",
 				 WARN_STRICT_OVERFLOW_COMPARISON);
 	  return fold_build2_loc (loc, code, type, variable, lhs);
 	}
Index: toplev.c
===================================================================
--- toplev.c	(revision 160130)
+++ toplev.c	(working copy)
@@ -613,7 +613,7 @@ crash_signal (int signo)
       exit (FATAL_EXIT_CODE);
     }
 
-  internal_error ("%s", strsignal (signo));
+  internal_error (strsignal (signo));
 }
 
 /* Arrange to dump core on error.  (The regular error message is still
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);
 	    }
 
@@ -4784,8 +4784,8 @@ cp_build_unary_op (enum tree_code code, 
 	arg = build_expr_type_conversion (flags, arg, true);
 	if (!arg)
 	  errstring = (code == NEGATE_EXPR
-		       ? _("wrong type argument to unary minus")
-		       : _("wrong type argument to unary plus"));
+		       ? G_("wrong type argument to unary minus")
+		       : G_("wrong type argument to unary plus"));
 	else
 	  {
 	    if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
@@ -4808,14 +4808,14 @@ cp_build_unary_op (enum tree_code code, 
       else if (!(arg = build_expr_type_conversion (WANT_INT | WANT_ENUM
 						   | WANT_VECTOR_OR_COMPLEX,
 						   arg, true)))
-	errstring = _("wrong type argument to bit-complement");
+	errstring = G_("wrong type argument to bit-complement");
       else if (!noconvert && CP_INTEGRAL_TYPE_P (TREE_TYPE (arg)))
 	arg = perform_integral_promotions (arg);
       break;
 
     case ABS_EXPR:
       if (!(arg = build_expr_type_conversion (WANT_ARITH | WANT_ENUM, arg, true)))
-	errstring = _("wrong type argument to abs");
+	errstring = G_("wrong type argument to abs");
       else if (!noconvert)
 	arg = default_conversion (arg);
       break;
@@ -4823,7 +4823,7 @@ cp_build_unary_op (enum tree_code code, 
     case CONJ_EXPR:
       /* Conjugating a real value is a no-op, but allow it anyway.  */
       if (!(arg = build_expr_type_conversion (WANT_ARITH | WANT_ENUM, arg, true)))
-	errstring = _("wrong type argument to conjugation");
+	errstring = G_("wrong type argument to conjugation");
       else if (!noconvert)
 	arg = default_conversion (arg);
       break;
@@ -4834,7 +4834,7 @@ cp_build_unary_op (enum tree_code code, 
       val = invert_truthvalue_loc (input_location, arg);
       if (arg != error_mark_node)
 	return val;
-      errstring = _("in argument to unary !");
+      errstring = G_("in argument to unary !");
       break;
 
     case NOP_EXPR:
@@ -4897,13 +4897,13 @@ cp_build_unary_op (enum tree_code code, 
 					      arg, true)))
 	{
 	  if (code == PREINCREMENT_EXPR)
-	    errstring = _("no pre-increment operator for type");
+	    errstring = G_("no pre-increment operator for type");
 	  else if (code == POSTINCREMENT_EXPR)
-	    errstring = _("no post-increment operator for type");
+	    errstring = G_("no post-increment operator for type");
 	  else if (code == PREDECREMENT_EXPR)
-	    errstring = _("no pre-decrement operator for type");
+	    errstring = G_("no pre-decrement operator for type");
 	  else
-	    errstring = _("no post-decrement operator for type");
+	    errstring = G_("no post-decrement operator for type");
 	  break;
 	}
       else if (arg == error_mark_node)
@@ -5249,7 +5249,7 @@ cp_build_unary_op (enum tree_code code, 
     }
 
   if (complain & tf_error)
-    error ("%s", errstring);
+    error (errstring);
   return error_mark_node;
 }
 
@@ -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;