Re: [PATCH] i18n fixes in gimple-ssa-sprintf.c (take 2)

[Jeff Law <law at redhat dot com>]

On 03/02/2018 12:42 AM, Jakub Jelinek wrote:
> On Thu, Mar 01, 2018 at 06:05:30PM -0700, Martin Sebor wrote:
>>> --- gcc/gimple-ssa-sprintf.c.jj	2018-02-27 23:16:19.747948912 +0100
>>> +++ gcc/gimple-ssa-sprintf.c	2018-03-01 21:26:37.728861287 +0100
>>> @@ -592,14 +592,12 @@ get_format_string (tree format, location
>>>  /* The format_warning_at_substring function is not used here in a way
>>>     that makes using attribute format viable.  Suppress the warning.  */
>>>
>>> -#pragma GCC diagnostic push
>>> -#pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
>>> -
>>
>> The comment above the #pragmas should be removed too.
> 
> Done below.
> 
>>>  /* For convenience and brevity.  */
>>>
>>>  static bool
>>>    (* const fmtwarn) (const substring_loc &, location_t,
>>>  		     const char *, int, const char *, ...)
>>> +  ATTRIBUTE_GCC_DIAG (5, 6)
>>>    = format_warning_at_substring;
>>
>> So add
>>
>>   static bool
>>     (* const fmtwarn_n) (const substring_loc &, location_t,
>>   		         const char *, int, unsigned HOST_WIDE_INT,
>>                          const char *, const char*, ...)
>>     ATTRIBUTE_GCC_DIAG (5, 6)
> 
> That would have to be ATTRIBUTE_GCC_DIAG (6, 8) ATTRIBUTE_GCC_DIAG (7, 8)
> 
>>     = format_warning_at_substring_n;
>>
>> and use the pointer instead?
> 
> As I said, that doesn't really work; it works for -Wformat, but doesn't
> work with exgettext.  Which is why for fmtwarn one has to use
>   fmtwarn (......, G_("foobarbaz"), ...);
> instead of
>   fmtwarn (......, "foobarbaz", ...);
> even when it is not conditional.
> 
> What we can do though is just duplicate the
> format_warning_at_substring and format_warning_at_substring_n functions
> (which are short and simple enough) as static under the shorter names.
> This way we can also get rid of the G_( ... ) wrapping of non-conditional
> fmtwarn arguments.
> 
> I've verified generated gcc.pot is the same between previous and this
> patch, except for line numbers in the comments, and if I comment out
> the (int) casts from dir.len in both one of fmtwarn and one of fmtwarn_n
> calls, gcc properly warns.  Ok for trunk if it passes bootstrap/regtest?
> 
> 2018-03-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* substring-locations.h (format_warning_va): Formatting fix for
> 	ATTRIBUTE_GCC_DIAG.
> 	(format_warning_at_substring): Fix up ATTRIBUTE_GCC_DIAG second
> 	argument.
> 	(format_warning_n_va, format_warning_at_substring_n): New prototypes.
> 	* substring-locations.c: Include intl.h.
> 	(format_warning_va): Turned into small wrapper around
> 	format_warning_n_va, renamed to ...
> 	(format_warning_n_va): ... this, add N and PLURAL_GMSGID arguments,
> 	rename GMSGID to SINGULAR_GMSGID, if SINGULAR_GMSGID != PLURAL_GMSGID,
> 	use ngettext.
> 	(format_warning_at_substring_n): New function.
> 	* gimple-ssa-sprintf.c: Remove GCC diagnostic ignored pragma.
> 	(fmtwarn): Add ATTRIBUTE_GCC_DIAG.  Turn into a copy of
> 	format_warning_at_substring with just a shorter name instead of
> 	const function pointer.
> 	(fmtwarn_n): New function.
> 	(maybe_warn, format_directive, parse_directive): Use fmtwarn_n where
> 	appropriate, get rid of all the fmtstr temporaries, move conditionals
> 	with G_() wrapped string literals directly into fmtwarn arguments,
> 	cast dir.len to (int), formatting fixes.
OK.
jeff