Re: Ping - old patch from April - mingw support for I32/I64 MS printf formatters to c-format.c

["Joseph S. Myers" <joseph@codesourcery.com>]

On Mon, 4 Feb 2008, Kai Tietz wrote:

> Thank you for your advice. May just by consistancy we should'nt declare 
> C99 functions for c90 in format.h. But the %Y is not present for mingw at 
> all. Therefore I prefer to disable them for these targets completely.

In that case you'll need to disable c99-printf-3.c for those targets as 
well; this patch version still has the bogus changes there.

"formatter" is nowhere used in the existing code to refer to kinds of 
formats; I suggest just using "format" throughout the patch.

>         * testsuite/gcc.dg/format/sys_formatter.c: New.

Does not belong in gcc/; you appear to have a properly located ChangeLog 
entry for this file below.

> @@ -1776,7 +1782,22 @@ check_format_info_main (format_check_res
>        if (fli)
>  	{
>  	  while (fli->name != 0 && fli->name[0] != *format_chars)
> -	    fli++;
> +	    {
> +	      if (fli->name[0] == '\0')
[...]

If this patch chunk is changing the meaning of some structure in 
c-format.h, the comments on that structure need updating to describe the 
new semantics implemented by this new code.

> +/* Description of gnu specific format attributes reflected to
> +   system format attribute types, as printf, scanf, strftime, and
> +   strfmon.  */

Not clear English, especially "reflected".  I think something like

/* Attributes such as "printf" are equivalent to those such as
   "gnu_printf" unless this is overridden by a target.  */

would be a clearer description of the purpose of this array.

> +const target_ovr_attr gnu_target_overrides_format_attributes[] =

I think this can be static.

> +/* Translate to unified attribute name. This is used in decode_format_type and
> +   decode_format_attr. In attr_name the user specified argument is passed. It
> +   returns the unified formatter name from TARGET_OVERRIDES_FORMAT_ATTRIBUTES
> +   or the attr_name passed to this function, if there is no matching entry.  */
> +static const char *
> +replace_formatter_name_to_system_name (const char *attr_name)

"replace ... to" isn't idiomatic English; "convert" might be better than 
"replace" here.

> +/* A helper function to compare the target override format attribute tattr_name
> +   and the user format attribute attr_name. The underscore variant of attr_name
> +   is ignore for compare. I returns for equal attribute the value one, otherwise
> +   zero.  */

/* Return true if TATTR_NAME and ATTR_NAME are the same format attribute,
   counting "name" and "__name__" as the same, false otherwise.  */

> +static int

Should return bool rather than int.

> +/* For none gnu style formatter types we need to compare the none gnu formatter
> +   and a gnu style formatter style to compare in kind.   This is done via the
> +   array TARGET_OVERRIDES_FORMAT_ATTRIBUTES.  The argument custom_type specifiers
> +   the index of the formatter. The argument def_type specifies the gnu style
> +   formatter index to be compare with. If the kind is equal it returns true,
> +   otherwise false.  */
> +static int

What should this return for comparing each pair of strftime, gnu_strftime, 
ms_strftime?  I can't tell from the comment.

Actually, I don't think we need this function.  The comparisons against 
specific types that you change to call this function are used for two 
things: processing GCC-internal formats, and giving the error "strftime 
formats cannot format arguments".  For the former, there will be no 
target-specific variants; these formats are the same everywhere.  For the 
latter, it would be better to replace the hardcoded check for 
strftime_format_type with a check for the format not setting 
FMT_FLAG_ARG_CONVERT in its flags; that's the right condition to check.  
The diagnostic should name the particular attribute in use, whether 
"strftime", "gnu_strftime" or "ms_strftime" (that is, the attribute in the 
user's source code, not any translated version of it).

> +/* Structure describing the target specific to be override formatter
> +   attributes, e.g. printf, scanf, etc.  This allows to support different
> +   runtime-library specific formatter attributes to co-exist and defining
> +   a default system version.
> +   This type is used for the pointer variable TARGET_OVERRIDES_FORMAT_ATTRIBUTES
> +   refers to.  */

/* Structure describing how format attributes such as "printf" are
   interpreted as "gnu_printf" or "ms_printf" on a particular system.
   TARGET_OVERRIDES_FORMAT_ATTRIBUTES is used to specify target-specific
   defaults.  */

> +/* Be aware to keep these values in synch with enum format_type from c-format.c.  */

Perhaps that enum should simply move to this header and avoid the separate 
list?

-- 
Joseph S. Myers
joseph@codesourcery.com