This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFA [PR objc/29200][diagnostic] Make diagnostics translation friendly


Am Mittwoch, den 08.04.2009, 16:49 +0000 schrieb Joseph S. Myers:
> On Sat, 4 Apr 2009, David Ayers wrote:
> 
> There is still a problem that you are translating the single word
> "using" 
> or "found" and requiring it to go in a particular position in the 
> "%J%s %<%c%s%>" format.
> 
> Some languages might want the translated word or phrase to go after the 
> %<%c%s%> instead of before; although the "%J%s %<%c%s%>" string is 
> translated, it hardly provides enough context for translators to know they 
> should move the %s to the end.  Some languages might want translated text 
> on both sides of the %<%c%s%>, or want the choice of ordering to differ in 
> the "using" and "found" cases.  Think diagnostics of the form "the method 
> %<%c%s%> is being used", if that is an idiomatic style for some languages.
> 
> I think there are two reasonable approaches: either translate the 
> "%Jusing <%c%s%>" and "%Jfound <%c%s%>" strings separately through 
> passing the full string as an argument to warn_with_method, or probably 
> better define an enum that says whether "using" or "found" is to be used, 
> use that enum as the first parameter of warn_with_method, and select 
> between two calls to warning with different format strings within that 
> function.

There are actually 5 occurrences/messages that are currently processed
via warn_with_method:
- "using"
- "found"
- "also found"
- "conflicting types for"
- "previous declaration of"

First I added the enum for all 5 warnings and populated warn_with_method
with the separate warnings, but I found the result rather bulky.  Then I
tried to extract only the method declaration formatting into a helper
function, but both memory management for the temporary string and the
gcc specific formatting of "%<%>" made that bulky as well.  So I propose
to go with your first suggestion of removing warn_with_method entirely
and simply calling warning/inform with consistent format strings.

> As a separate issue, it looks like this diagnostic is an informative 
> message that should use "inform" not "warning".

The selection of the wrong method signature can lead to the caller
misinterpreting the parameters or the callee misinterpreting the return
type.  So I believe the issue as a whole should be treated as a warning.

Yet we already emit a separate warning for the issue and the subsequent
calls to warn_with_method merely add supplementary information.  So
treating these subsequent messages as informative messages seems
appropriate.  Is that what you were referring to?

It seems that the test suite doesn't differentiate between warning and
inform, so there is no need to adapt anything there.

I'm bootstrapping this on i686-pc-gnu-linux.
OK for trunk if that and regression tests pass?

Cheers,
David

2009-00-00  David Ayers  <ayers@fsfe.org>

	PR objc/29200
	* objc/objc-act.c (warn_with_method): Remove helper function.
	(check_duplicates): Call warning and inform directly.
	(really_start_method): Likewise.

Index: gcc/objc/objc-act.c
===================================================================
--- gcc/objc/objc-act.c	(Revision 145609)
+++ gcc/objc/objc-act.c	(Arbeitskopie)
@@ -243,7 +243,6 @@
 
 /* Utilities for debugging and error diagnostics.  */
 
-static void warn_with_method (const char *, int, tree);
 static char *gen_type_name (tree);
 static char *gen_type_name_0 (tree);
 static char *gen_method_decl (tree);
@@ -6103,22 +6102,37 @@
 	    }
 
 	issue_warning:
-	  warning (0, "multiple %s named %<%c%s%> found",
-		   methods ? "methods" : "selectors",
-		   (is_class ? '+' : '-'),
-		   IDENTIFIER_POINTER (METHOD_SEL_NAME (meth)));
+	  if (methods)
+	    {
+	      bool type = TREE_CODE (meth) == INSTANCE_METHOD_DECL;
 
-	  warn_with_method (methods ? "using" : "found",
-			    ((TREE_CODE (meth) == INSTANCE_METHOD_DECL)
-			     ? '-'
-			     : '+'),
-			    meth);
+	      warning (0, "multiple methods named %<%c%s%> found",
+		       is_class ? '+' : '-',
+		       IDENTIFIER_POINTER (METHOD_SEL_NAME (meth)));
+	      inform (0, "%Jusing %<%c%s%>", meth,
+		      type ? '-' : '+',
+		      gen_method_decl(meth));
+	    }
+	  else
+	    {
+	      bool type = TREE_CODE (meth) == INSTANCE_METHOD_DECL;
+
+	      warning (0, "multiple selectors named %<%c%s%> found",
+		       is_class ? '+' : '-',
+		       IDENTIFIER_POINTER (METHOD_SEL_NAME (meth)));
+	      inform (0, "%Jfound %<%c%s%>", meth,
+		      type ? '-' : '+',
+		      gen_method_decl(meth));
+	    }
+
 	  for (loop = hsh->list; loop; loop = loop->next)
-	    warn_with_method ("also found",
-			      ((TREE_CODE (loop->value) == INSTANCE_METHOD_DECL)
-			       ? '-'
-			       : '+'),
-			      loop->value);
+	    {
+	      bool type = TREE_CODE (loop->value) == INSTANCE_METHOD_DECL;
+
+	      inform (0, "%Jalso found %<%c%s%>", loop->value, 
+		      type ? '-' : '+',
+		      gen_method_decl(loop->value));
+	    }
         }
     }
   return meth;
@@ -8427,14 +8441,6 @@
   really_start_method (objc_method_context, parm_info);
 }
 
-static void
-warn_with_method (const char *message, int mtype, tree method)
-{
-  /* Add a readable method name to the warning.  */
-  warning (0, "%J%s %<%c%s%>", method,
-           message, mtype, gen_method_decl (method));
-}
-
 /* Return 1 if TYPE1 is equivalent to TYPE2
    for purposes of method overloading.  */
 
@@ -8677,10 +8683,14 @@
 	{
 	  if (!comp_proto_with_proto (method, proto, 1))
 	    {
-	      char type = (TREE_CODE (method) == INSTANCE_METHOD_DECL ? '-' : '+');
+	      bool type = TREE_CODE (method) == INSTANCE_METHOD_DECL;
 
-	      warn_with_method ("conflicting types for", type, method);
-	      warn_with_method ("previous declaration of", type, proto);
+	      warning (0, "%Jconflicting types for %<%c%s%>", method,
+		       type ? '-' : '+',
+		       gen_method_decl(method));
+	      inform (0, "%Jprevious declaration of %<%c%s%>", proto,
+		      type ? '-' : '+',
+		      gen_method_decl(proto));
 	    }
 	}
       else

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]