From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 493 invoked by alias); 19 Feb 2008 12:58:36 -0000 Received: (qmail 481 invoked by uid 22791); 19 Feb 2008 12:58:35 -0000 X-Spam-Check-By: sourceware.org Received: from outdoor.onevision.de (HELO outdoor.onevision.de) (212.77.172.51) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 19 Feb 2008 12:58:14 +0000 Received: from sanders.onevision.de (moonrace [212.77.172.62]) by outdoor.onevision.de (8.13.7/8.13.7/ROSCH/DDB) with ESMTP id m1JCw4gH012228; Tue, 19 Feb 2008 13:58:09 +0100 In-Reply-To: To: "Joseph S. Myers" Cc: Danny Smith , GCC Patches , NightStrike Subject: Re: Ping - old patch from April - mingw support for I32/I64 MS printf formatters to c-format.c MIME-Version: 1.0 X-Mailer: Lotus Notes Release 7.0.1 January 17, 2006 Message-ID: From: Kai Tietz Date: Tue, 19 Feb 2008 13:30:00 -0000 Content-Type: text/plain; charset="US-ASCII" X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2008-02/txt/msg00766.txt.bz2 "Joseph S. Myers" 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.