Hi Joseph, > On Wed, 5 Dec 2007, NightStrike wrote: > > > On 12/4/07, Joseph S. Myers wrote: > > > On Tue, 4 Dec 2007, NightStrike wrote: > > > > > > > http://gcc.gnu.org/ml/gcc-patches/2007-04/msg00767.html > > > > > > > > This old thread is in need of review to help the x86_64-pc-mingw32 > > > > port of gcc. It's been about 8 months so far. Any takers to review > > > > the patch? > > > > > > If there is a patch with full testcases, please point to it - that patch > > > appears to be one without testcases, and so not ready for review. > > > > I think Kai was waiting until your review before making the testcases. > > Can you at least evaluate what's there already? > > No, in the original discussions leading to that patch we already discussed > the general nature of what the feature should look like at the language > level, and assessing whether the patch meets that requires testcases (and > changes to existing testcases) that provide proper coverage of the new and > existing features and leave all the format checking tests passing on both > MinGW and non-MinGW targets. > > If after reviewing testcases and code I suspect problems in areas not > covered by the tests I build the compiler with a patch when reviewing it > and try my own tests, but the basic tests to provide thorough coverage of > the patch are the responsibility of the patch submitter. This (and > diagnostic-related patches in general) is the sort of patch I review > primarily by examining the testcases to see if I agree with the > diagnostics asserted to be present or absent and that the testcases appear > to cover the feature as described, and only secondarily if the testcases > appear correct and adequate by examining the implementation. > > I made clear on 25 April > that I was > awaiting testsuite changes. I do not think there is any open question > regarding the principle of the feature (where partial patches might be of > value in assessing how worthwhile a feature is bearing in mind the cost of > the implementation); what needs review is the quality of an > implementation, and that needs a complete patch submission. I added updated the patch against current head version of gcc and add a test-case for the common case that the system (crt) version of formatter is used. For mingw targets there is no %lld formatter existing. Instead MS functions are using %I64d and %I64d. This patch additionally allows the user to define a alternative formatter beside the system one for i386 targets. The default formatters are named "gnu_*" and the MS specific "ms_*". This patch is tested for *-pc-mingw32 (using MS %I32 and %I64) and for i686-pc-cygwin (for the gnu variant). ChangeLog: 2007-12-18 Kai Tietz * gcc/doc/extend.texi: Extent user description. * gcc/doc/tm.texi: (TARGET_OVERRIDES_FORMAT_ATTRIBUTES, TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT): New. * gcc/c-format.c: (replace_formatter_name_to_system_name): New method to map target format attributes as system. (decode_format_attr): Translate system format attribute names. (format_types_orig): Prefix gnu attributes by "gnu_". (ARGET_OVERRIDES_FORMAT_ATTRIBUTES): New extern for target specific system format attribute override. (gnu_target_overrides_format_attributes): Default overrides for gnu. * gcc/c-format.h: (target_ovr_attr): New type for system attribute mapping. * gcc/config/i386/mingw32.h: (TARGET_OVERRIDES_FORMAT_ATTRIBUTES, TARGET_OVERRIDES_FORMAT_ATTRIBUTES_COUNT, TARGET_FORMAT_TYPES, TARGET_N_FORMAT_TYPES): New. * gcc/config/i386/i386.c: Add ms-formatter definitions. * gcc/testsuite/gcc.dg/sys_formatter.c: New test case. Cheers, i.A. Kai Tietz | (\_/) This is Bunny. Copy and paste Bunny | (='.'=) into your signature to help him gain | (")_(") world domination. ------------------------------------------------------------------------------------------ OneVision Software Entwicklungs GmbH & Co. KG Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050 Handelsregister: HRA 6744, Amtsgericht Regensburg Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: Ulrike Döhler, Manuela Kluger