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

[Kai Tietz <Kai.Tietz@onevision.com>]

"Joseph S. Myers" <joseph@codesourcery.com> wrote on 19.02.2008 13:28:31:

> On Tue, 19 Feb 2008, Kai Tietz wrote:
> 
> > > "formatter" is nowhere used in the existing code to refer to kinds 
of 
> > > formats; I suggest just using "format" throughout the patch.
> > 
> > 
> > It is renamed to sys_format.c, fine?
> 
> I mean everywhere: in all the function names, not just in one file name.
> 
> > > > +/* 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).
> > 
> > IMHO, this method is necessary to be able to use the existing argument 

> > checking routines and generalize the existing code.
> > If we have user defined formatters extensions, we need to get their 
gnu 
> > style variant. So we can check, if the attribute has a formatter, 
needs 
> > additional arguments, and if the formatter can be NULL, and so on.
> 
> I still can't tell what the definition is for what is considered "equal" 

> or not, and in particular what the answers should be for each pair of 
the 
> three attributes I listed above.
I treated the enums simply as kinds, not as index positions. What is a bit 
wired here in fact. I agree, that this is no fast and good solution as it 
is.

> The code should not be comparing against particular enum values.  It 
> should:
> 
> * Get a structure with information about the format (possibly converting 

> "strftime" to "gnu_strftime" or "ms_strftime" in the process, for 
> example).
> 
> * Look up information in that structure.  This information will tell 
> everything about format strings, additional arguments etc..
> 
> The only exception might be for the GCC-internal types, where you know 
> there will never be system-specific variants.

What's about extending the attribute definition structure by a kind member 
and compare instead of index position, there kind? So this function is no 
longer necessary and we need just in the c-format.h enum values defining 
kinds.

typedef enum {
 attr_kind_printf=0,
 attr_kind_scanf,
 ...
} attr_kind;

And we can remove the doubled enum list.

 Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.