Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-31 Thread Jacek Caban
On 10/31/2018 03:32 AM, Liu Hao wrote:
> 在 2018/10/31 2:01, Jacek Caban 写道:
>> Wine has a test for that with a comment saying that it's supported since
>> Vista:
>> https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/msvcrt/tests/printf.c#l291
>>
>>
>> I changed the test to accept only platforms properly handling %lld and
>> running it on Testbot machines shows that it's indeed supported sine Vista:
>> https://testbot.winehq.org/JobDetails.pl?Key=43687
>>
>>
> This is a valuable asset. If XP and 2003 are out of interest we could
> just use the `ll` modifier cosistently.

Yes, that's tempting and something to consider at some point. XP is
probably is generally still of (limited) interest, but maybe that
doesn't apply to inttypes.h. After all inttypes.h is a C99 thing and if
you want C99 on top of msvcrt.dll, you should use mingw stdio... Still,
I'd say that my patch is safer regression-wise, so I'd go with it.

Cheers,
Jacek


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-31 Thread Liu Hao
在 2018/10/31 21:53, Earnie via Mingw-w64-public 写道:
> Is it possible to allow a skip for the test if the test machine doesn't
> support that test?  Or is that what Jacek did?
> 

As far as I can see hijacking the test was only a means of discovering
platforms whose MSVCRT.DLL has a `printf()` that handles `%lld`
properly. The test is part of Wine, not of mingw-w64.

-- 
Best regards,
LH_Mouse



signature.asc
Description: OpenPGP digital signature
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-31 Thread Earnie via Mingw-w64-public
On 10/30/2018 10:32 PM, Liu Hao wrote:
> 在 2018/10/31 2:01, Jacek Caban 写道:
>> Wine has a test for that with a comment saying that it's supported since
>> Vista:
>> https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/msvcrt/tests/printf.c#l291
>>
>>
>> I changed the test to accept only platforms properly handling %lld and
>> running it on Testbot machines shows that it's indeed supported sine Vista:
>> https://testbot.winehq.org/JobDetails.pl?Key=43687
>>
>>
> This is a valuable asset. If XP and 2003 are out of interest we could
> just use the `ll` modifier cosistently.

Is it possible to allow a skip for the test if the test machine doesn't
support that test?  Or is that what Jacek did?

-- 
Earnie


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-30 Thread Liu Hao
在 2018/10/31 2:01, Jacek Caban 写道:
> Wine has a test for that with a comment saying that it's supported since
> Vista:
> https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/msvcrt/tests/printf.c#l291
> 
> 
> I changed the test to accept only platforms properly handling %lld and
> running it on Testbot machines shows that it's indeed supported sine Vista:
> https://testbot.winehq.org/JobDetails.pl?Key=43687
> 
>
This is a valuable asset. If XP and 2003 are out of interest we could
just use the `ll` modifier cosistently.


-- 
Best regards,
LH_Mouse


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-30 Thread Jacek Caban

On 30/10/2018 18:45, Mateusz wrote:


W dniu 30.10.2018 o 17:04, Liu Hao pisze:

在 2018/10/30 23:48, Mateusz 写道:

Liu Hao example is the explanation.
printf("value = %" PRIx64 "\n", value);
should print 64-bit int 'value' regardless stdio.h is included or not.



Hmmm on my Windows 7 Professional [Version 6.1.7601 x64], the `printf()`
function from MSVCRT actually recognizes `%llx` and `%lld`.

Is maintaining `%I64d` really necessary?

On my Win10, 7 and Vista also %lld works. Strange... Maybe somebody with WinXP 
will try?



Wine has a test for that with a comment saying that it's supported since 
Vista:

https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/msvcrt/tests/printf.c#l291

I changed the test to accept only platforms properly handling %lld and 
running it on Testbot machines shows that it's indeed supported sine Vista:

https://testbot.winehq.org/JobDetails.pl?Key=43687

Jacek


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-30 Thread Mateusz
W dniu 30.10.2018 o 17:04, Liu Hao pisze:
> 在 2018/10/30 23:48, Mateusz 写道:
>> Liu Hao example is the explanation.
>> printf("value = %" PRIx64 "\n", value);
>> should print 64-bit int 'value' regardless stdio.h is included or not.
>>
>>
> 
> Hmmm on my Windows 7 Professional [Version 6.1.7601 x64], the `printf()`
> function from MSVCRT actually recognizes `%llx` and `%lld`.
> 
> Is maintaining `%I64d` really necessary?

On my Win10, 7 and Vista also %lld works. Strange... Maybe somebody with WinXP 
will try?

But if in t.c first line is #include  we have
$ gcc -D__USE_MINGW_ANSI_STDIO= t.c -o t.exe
In file included from t.c:1:0:
f:\msys\m32-494\i686-w64-mingw32\include\stdio.h:234:27: error: #if with no 
expression
 #if __USE_MINGW_ANSI_STDIO
   ^
f:\msys\m32-494\i686-w64-mingw32\include\stdio.h:740:65: error: operator '||' 
has no right operand
 #if !defined (__USE_MINGW_ANSI_STDIO) || __USE_MINGW_ANSI_STDIO == 0
 ^
f:\msys\m32-494\i686-w64-mingw32\include\stdio.h:857:27: error: #if with no 
expression
 #if __USE_MINGW_ANSI_STDIO
   ^
f:\msys\m32-494\i686-w64-mingw32\include\stdio.h:1195:65: error: operator '||' 
has no right operand
 #if !defined (__USE_MINGW_ANSI_STDIO) || __USE_MINGW_ANSI_STDIO == 0
 ^



___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-30 Thread Mateusz
W dniu 30.10.2018 o 13:59, Jacek Caban pisze:
> On 10/28/2018 10:28 PM, Mateusz wrote:
>> W dniu 28.10.2018 o 16:07, Jacek Caban pisze:
>>>
>>> On 28/10/2018 09:39, Liu Hao wrote:
 在 2018/10/27 19:52, Jacek Caban 写道:
> I don't see what makes __USE_MINGW_ANSI_STDIO a correct solution.
> Firefox builds use ucrt, which is sufficient for handling stdio (and
> official winapi+clang-cl builds use that anyway).
>
 Oh I didn't know that had migrated to UCRT.

> I took a look at this and I think we want a more radical solution. Right
> now we have _mingw_print_push.h/_mingw_print_pop.h hack that has no
> clean purpose to me. I did some git history checking and when it was
> introduced, it was important for __USE_MINGW_ANSI_STDIO because that was
> where printf/scanf functions were #defined. It was later removed and the
> only thing that's left there is a bunch of PRI* macros. Instead of
> defining and redefining them, we may just as well define them correctly
> in the first place.
>
> That's what the attached patch does. It also uses the same for ucrt and
> mingw stdio, leaving hackish values only for affected builds. This fixes
> Firefox issue and makes mingw stdio cleaner. It's not yet tested, I'm
> mostly sending it for comments now.
>
 Yes those macros `scanf()` and `printf()` etc. were removed in
 d4e56ae45d4f92598f7b393340efca93f3deff38 and now those functions are
 implemented by conditional inline definitions. If no other inline
 functions [1] in our headers rely on the old format specifiers, the
 header <_mingw_print_push.h> is consequenctly unneeded thereafter.

 The circumstances of <_mingw_print_pop.h> are a bit more complex. It has
 to restore `PRId64` etc. to the correct strings that correspond to what
 `printf()` etc., after the inclusion of , will eventually
 call. Thus at the moment <_mingw_print_pop.h> actually checks for
 inclusion of  or  (there was an old thread about this
 [2]), otherwise the following code may not produce the expected result:

 ```c
 #define __USE_MINGW_ANSI_STDIO  1
 #include 
 #include 

 // `PRIx64` is defined as `I64x` here, which will co-operate with
 // `printf()` from MSVCRT rather than the inline one in .
>>> In my proposed patch, it will be properly defined to llx in inttypes.h 
>>> (note that the patch makes inttypes.h aware of __USE_MINGW_ANSI_STDIO). By 
>>> defining it to the right value in the first place, we don't need to 
>>> redefine it later.
>> Now if it is defined __USE_MINGW_ANSI_STDIO and stdio.h is NOT included, it 
>> is I64x, with your patch it is changed to llx (that not works with 
>> msvcrt.dll without miracles from stdio.h).
> 
> Yes, but what's wrong with that? Doesn't that make more sense? What's
> the sense of depending on strio.h presence? I think that not depending
> on it will give more consistent results. And I suspect that current
> behaviour is a side effect of a hack and not really intended feature and
> as such I don't see a reason to preserve it.

Liu Hao example is the explanation.
printf("value = %" PRIx64 "\n", value);
should print 64-bit int 'value' regardless stdio.h is included or not.


>> Maybe it is simpler to leave *_pop.h and *_push.h like it is/was and change 
>> only condition from
>> #if defined(__USE_MINGW_ANSI_STDIO)
>> to
>> #if defined(__USE_MINGW_ANSI_STDIO) || __MSVCRT_VERSION__ >= 0x1400
>> for llx inttypes.
>>
>> I've attached patch proposition (and I removed the case when 
>> __USE_MINGW_ANSI_STDIO defined as 0 is like not defined -- it's too 
>> complicated).
> 
> Why do you want to remove support for __USE_MINGW_ANSI_STDIO? I think
> it's good to have a way for users to explicitly specify that they don't
> want mingw stdio.
> 
> Jacek

Now I see it, thanks!

I was wrongly thinking that if __USE_MINGW_ANSI_STDIO is not defined it is 
ms_printf, if it is defined it is mingw_printf.

Now I see that if __USE_MINGW_ANSI_STDIO is not defined it is fuzzy logic 
(_GNU_SOURCE and so on), if it is defined as 0 it should be ms_printf, if it is 
defined != 0 it should be mingw_printf. I will prepare new patch.

Regards,
Mateusz



___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-30 Thread Jacek Caban
On 10/28/2018 10:28 PM, Mateusz wrote:
> W dniu 28.10.2018 o 16:07, Jacek Caban pisze:
>>
>> On 28/10/2018 09:39, Liu Hao wrote:
>>> 在 2018/10/27 19:52, Jacek Caban 写道:
 I don't see what makes __USE_MINGW_ANSI_STDIO a correct solution.
 Firefox builds use ucrt, which is sufficient for handling stdio (and
 official winapi+clang-cl builds use that anyway).

>>> Oh I didn't know that had migrated to UCRT.
>>>
 I took a look at this and I think we want a more radical solution. Right
 now we have _mingw_print_push.h/_mingw_print_pop.h hack that has no
 clean purpose to me. I did some git history checking and when it was
 introduced, it was important for __USE_MINGW_ANSI_STDIO because that was
 where printf/scanf functions were #defined. It was later removed and the
 only thing that's left there is a bunch of PRI* macros. Instead of
 defining and redefining them, we may just as well define them correctly
 in the first place.

 That's what the attached patch does. It also uses the same for ucrt and
 mingw stdio, leaving hackish values only for affected builds. This fixes
 Firefox issue and makes mingw stdio cleaner. It's not yet tested, I'm
 mostly sending it for comments now.

>>> Yes those macros `scanf()` and `printf()` etc. were removed in
>>> d4e56ae45d4f92598f7b393340efca93f3deff38 and now those functions are
>>> implemented by conditional inline definitions. If no other inline
>>> functions [1] in our headers rely on the old format specifiers, the
>>> header <_mingw_print_push.h> is consequenctly unneeded thereafter.
>>>
>>> The circumstances of <_mingw_print_pop.h> are a bit more complex. It has
>>> to restore `PRId64` etc. to the correct strings that correspond to what
>>> `printf()` etc., after the inclusion of , will eventually
>>> call. Thus at the moment <_mingw_print_pop.h> actually checks for
>>> inclusion of  or  (there was an old thread about this
>>> [2]), otherwise the following code may not produce the expected result:
>>>
>>> ```c
>>> #define __USE_MINGW_ANSI_STDIO  1
>>> #include 
>>> #include 
>>>
>>> // `PRIx64` is defined as `I64x` here, which will co-operate with
>>> // `printf()` from MSVCRT rather than the inline one in .
>> In my proposed patch, it will be properly defined to llx in inttypes.h (note 
>> that the patch makes inttypes.h aware of __USE_MINGW_ANSI_STDIO). By 
>> defining it to the right value in the first place, we don't need to redefine 
>> it later.
> Now if it is defined __USE_MINGW_ANSI_STDIO and stdio.h is NOT included, it 
> is I64x, with your patch it is changed to llx (that not works with msvcrt.dll 
> without miracles from stdio.h).

Yes, but what's wrong with that? Doesn't that make more sense? What's
the sense of depending on strio.h presence? I think that not depending
on it will give more consistent results. And I suspect that current
behaviour is a side effect of a hack and not really intended feature and
as such I don't see a reason to preserve it.

>
> Maybe it is simpler to leave *_pop.h and *_push.h like it is/was and change 
> only condition from
> #if defined(__USE_MINGW_ANSI_STDIO)
> to
> #if defined(__USE_MINGW_ANSI_STDIO) || __MSVCRT_VERSION__ >= 0x1400
> for llx inttypes.
>
> I've attached patch proposition (and I removed the case when 
> __USE_MINGW_ANSI_STDIO defined as 0 is like not defined -- it's too 
> complicated).

Why do you want to remove support for __USE_MINGW_ANSI_STDIO? I think
it's good to have a way for users to explicitly specify that they don't
want mingw stdio.

Jacek


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-29 Thread Martin Storsjö

On Mon, 29 Oct 2018, Liu Hao wrote:


在 2018/10/29 5:28, Mateusz 写道:

Now if it is defined __USE_MINGW_ANSI_STDIO and stdio.h is NOT included, it is 
I64x, with your patch it is changed to llx (that not works with msvcrt.dll 
without miracles from stdio.h).



Yes this was what I mentioned in my previous mail.

The condition for C99 format specifiers which we check at the moment, in
<_mingw_print_pop.h>, is:

-#if defined(__USE_MINGW_ANSI_STDIO) && (defined(_INC_STDIO) || 
defined(_WSTDIO_DEFINED)) && ((__USE_MINGW_ANSI_STDIO + 0) != 0)

while in the proposed patch, in , it is changed to:

+#if __MSVCRT_VERSION__ >= 0x1400 || (defined(__USE_MINGW_ANSI_STDIO) && 
((__USE_MINGW_ANSI_STDIO + 0) != 0))


So I bet it would miss something.



Maybe it is simpler to leave *_pop.h and *_push.h like it is/was and change 
only condition from
#if defined(__USE_MINGW_ANSI_STDIO)
to
#if defined(__USE_MINGW_ANSI_STDIO) || __MSVCRT_VERSION__ >= 0x1400
for llx inttypes.

I've attached patch proposition (and I removed the case when 
__USE_MINGW_ANSI_STDIO defined as 0 is like not defined -- it's too 
complicated).




I don't know how many people define `__USE_MINGW_ANSI_STDIO` as zero. If
it does matter, there is still no need to check for
`defined(__USE_MINGW_ANSI_STDIO)`, as all undefined identifiers are
replaced with zeroes implicitly [1], so `#if __USE_MINGW_ANSI_STDIO`
will suffice - even the `+ 0` and `!= 0` parts are superfluous.


I would prefer not to rely on this in public installed headers, as it 
would start triggering warnings if people build code with -Wundef (unless 
such warnings are ignored for system headers, but I'd rather not rely on 
that).


// Martin

___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-28 Thread Liu Hao
在 2018/10/29 5:28, Mateusz 写道:
> Now if it is defined __USE_MINGW_ANSI_STDIO and stdio.h is NOT included, it 
> is I64x, with your patch it is changed to llx (that not works with msvcrt.dll 
> without miracles from stdio.h).
> 

Yes this was what I mentioned in my previous mail.

The condition for C99 format specifiers which we check at the moment, in
<_mingw_print_pop.h>, is:
> -#if defined(__USE_MINGW_ANSI_STDIO) && (defined(_INC_STDIO) || 
> defined(_WSTDIO_DEFINED)) && ((__USE_MINGW_ANSI_STDIO + 0) != 0)
while in the proposed patch, in , it is changed to:
> +#if __MSVCRT_VERSION__ >= 0x1400 || (defined(__USE_MINGW_ANSI_STDIO) && 
> ((__USE_MINGW_ANSI_STDIO + 0) != 0))

So I bet it would miss something.


> Maybe it is simpler to leave *_pop.h and *_push.h like it is/was and change 
> only condition from
> #if defined(__USE_MINGW_ANSI_STDIO)
> to
> #if defined(__USE_MINGW_ANSI_STDIO) || __MSVCRT_VERSION__ >= 0x1400
> for llx inttypes.
> 
> I've attached patch proposition (and I removed the case when 
> __USE_MINGW_ANSI_STDIO defined as 0 is like not defined -- it's too 
> complicated).
> 
>

I don't know how many people define `__USE_MINGW_ANSI_STDIO` as zero. If
it does matter, there is still no need to check for
`defined(__USE_MINGW_ANSI_STDIO)`, as all undefined identifiers are
replaced with zeroes implicitly [1], so `#if __USE_MINGW_ANSI_STDIO`
will suffice - even the `+ 0` and `!= 0` parts are superfluous.


[1] https://en.cppreference.com/w/cpp/preprocessor/conditional


-- 
Best regards,
LH_Mouse



signature.asc
Description: OpenPGP digital signature
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-28 Thread Mateusz
W dniu 27.10.2018 o 13:46, Vincent Torri pisze:
> On Fri, Oct 26, 2018 at 11:58 PM Mateusz  wrote:
>>
>> W dniu 26.10.2018 o 22:35, Tom Ritter pisze:
>>> This is not really a MinGW problem, but MinGW does diverge from other
>>> compilers and it caused Firefox to crash.
>>>
>>> MinGW defines a lot of I64[foo] format specifiers in inttypes.h.
>>> clang and clang-cl don't use I64[foo] they use ll[foo]. (I64[foo] is
>>> valid according to Microsoft. MinGW mentions "MS runtime does not yet
>>> understand C9x standard "ll"" but at some point they started
>>> supporting ll[foo].  And as I mentioned, that's what clang[-cl] uses.
>>>
>>> Mozilla has our own implementation of printf that does the format
>>> specifier parsing. We don't support I64[foo]. So using it caused data
>>> corruption and general bad behavior. Switching to ll[foo] fixed it.
>>>
>>> I have a patch here:
>>> https://hg.mozilla.org/try/raw-file/eaae7782a1dd/build/build-clang/mingw-int.patch
>>
>> You can try to define
>> __USE_MINGW_ANSI_STDIO
>> instead of patching mingw-w64
> 
> i've mesure a significant performance loss when using printf
> functions with __USE_MINGW_ANSI_STDIO cmopared to the ones in
> msvcrt.dll
> 
> is it normal ?
> 
> Vincent Torri

Yes, it is normal.

We could try to speed up mingw_printf -- if you have some examples when the 
speed loss is significat please share (it was recently speed up for some cases 
in mingw_wprintf family -- wide char version).

Regards,
Mateusz



___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-28 Thread Mateusz
W dniu 28.10.2018 o 16:07, Jacek Caban pisze:
> 
> 
> On 28/10/2018 09:39, Liu Hao wrote:
>> 在 2018/10/27 19:52, Jacek Caban 写道:
>>> I don't see what makes __USE_MINGW_ANSI_STDIO a correct solution.
>>> Firefox builds use ucrt, which is sufficient for handling stdio (and
>>> official winapi+clang-cl builds use that anyway).
>>>
>> Oh I didn't know that had migrated to UCRT.
>>
>>> I took a look at this and I think we want a more radical solution. Right
>>> now we have _mingw_print_push.h/_mingw_print_pop.h hack that has no
>>> clean purpose to me. I did some git history checking and when it was
>>> introduced, it was important for __USE_MINGW_ANSI_STDIO because that was
>>> where printf/scanf functions were #defined. It was later removed and the
>>> only thing that's left there is a bunch of PRI* macros. Instead of
>>> defining and redefining them, we may just as well define them correctly
>>> in the first place.
>>>
>>> That's what the attached patch does. It also uses the same for ucrt and
>>> mingw stdio, leaving hackish values only for affected builds. This fixes
>>> Firefox issue and makes mingw stdio cleaner. It's not yet tested, I'm
>>> mostly sending it for comments now.
>>>
>> Yes those macros `scanf()` and `printf()` etc. were removed in
>> d4e56ae45d4f92598f7b393340efca93f3deff38 and now those functions are
>> implemented by conditional inline definitions. If no other inline
>> functions [1] in our headers rely on the old format specifiers, the
>> header <_mingw_print_push.h> is consequenctly unneeded thereafter.
>>
>> The circumstances of <_mingw_print_pop.h> are a bit more complex. It has
>> to restore `PRId64` etc. to the correct strings that correspond to what
>> `printf()` etc., after the inclusion of , will eventually
>> call. Thus at the moment <_mingw_print_pop.h> actually checks for
>> inclusion of  or  (there was an old thread about this
>> [2]), otherwise the following code may not produce the expected result:
>>
>> ```c
>> #define __USE_MINGW_ANSI_STDIO  1
>> #include 
>> #include 
>>
>> // `PRIx64` is defined as `I64x` here, which will co-operate with
>> // `printf()` from MSVCRT rather than the inline one in .
> 
> In my proposed patch, it will be properly defined to llx in inttypes.h (note 
> that the patch makes inttypes.h aware of __USE_MINGW_ANSI_STDIO). By defining 
> it to the right value in the first place, we don't need to redefine it later.

Now if it is defined __USE_MINGW_ANSI_STDIO and stdio.h is NOT included, it is 
I64x, with your patch it is changed to llx (that not works with msvcrt.dll 
without miracles from stdio.h).

Maybe it is simpler to leave *_pop.h and *_push.h like it is/was and change 
only condition from
#if defined(__USE_MINGW_ANSI_STDIO)
to
#if defined(__USE_MINGW_ANSI_STDIO) || __MSVCRT_VERSION__ >= 0x1400
for llx inttypes.

I've attached patch proposition (and I removed the case when 
__USE_MINGW_ANSI_STDIO defined as 0 is like not defined -- it's too 
complicated).

Regards,
Mateusz
From 956ce6c208de2bf8bc67772839543449e0f08f5e Mon Sep 17 00:00:00 2001
From: Mateusz 
Date: Sun, 28 Oct 2018 19:48:06 +0100
Subject: [PATCH] use ANSI inttypes for ucrt builds

Signed-off-by: Mateusz 
---
 mingw-w64-headers/crt/_mingw_print_pop.h  | 4 ++--
 mingw-w64-headers/crt/_mingw_print_push.h | 4 ++--
 mingw-w64-headers/crt/stdio.h | 8 
 mingw-w64-headers/crt/stdlib.h| 2 +-
 mingw-w64-headers/crt/wchar.h | 6 +++---
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/mingw-w64-headers/crt/_mingw_print_pop.h 
b/mingw-w64-headers/crt/_mingw_print_pop.h
index 046b6203..e711cd28 100644
--- a/mingw-w64-headers/crt/_mingw_print_pop.h
+++ b/mingw-w64-headers/crt/_mingw_print_pop.h
@@ -5,7 +5,7 @@
  */
 
 /* Define __mingw_ macros.  */
-#if defined(__USE_MINGW_ANSI_STDIO) && (defined(_INC_STDIO) || 
defined(_WSTDIO_DEFINED)) && ((__USE_MINGW_ANSI_STDIO + 0) != 0)
+#if (defined(__USE_MINGW_ANSI_STDIO) && (defined(_INC_STDIO) || 
defined(_WSTDIO_DEFINED))) || __MSVCRT_VERSION__ >= 0x1400
 
 /* Redefine to GNU specific PRI... and SCN... macros.  */
 #if defined(_INTTYPES_H_) && defined(PRId64)
@@ -133,4 +133,4 @@
 #endif /* _WIN64 */
 #endif /* defined(_INTTYPES_H_) && defined(PRId64) */
 
-#endif /* defined(__USE_MINGW_ANSI_STDIO) && defined(_INC_STDIO) && 
__USE_MINGW_ANSI_STDIO != 0 */
+#endif /* (defined(__USE_MINGW_ANSI_STDIO) && defined(_INC_STDIO)) || 
__MSVCRT_VERSION__ >= 0x1400 */
diff --git a/mingw-w64-headers/crt/_mingw_print_push.h 
b/mingw-w64-headers/crt/_mingw_print_push.h
index e0da9d00..7431c041 100644
--- a/mingw-w64-headers/crt/_mingw_print_push.h
+++ b/mingw-w64-headers/crt/_mingw_print_push.h
@@ -5,7 +5,7 @@
  */
 
 /* Undefine __mingw_ macros.  */
-#if defined(__USE_MINGW_ANSI_STDIO) && ((__USE_MINGW_ANSI_STDIO + 0) != 0)
+#if defined(__USE_MINGW_ANSI_STDIO) || __MSVCRT_VERSION__ >= 0x1400
 
 /* Redefine to MS specific PRI... and SCN... macros.  */
 #if defined(_INTTYPES_H_) && defined(PRId64)
@@ 

Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-28 Thread Jacek Caban



On 28/10/2018 09:39, Liu Hao wrote:

在 2018/10/27 19:52, Jacek Caban 写道:

I don't see what makes __USE_MINGW_ANSI_STDIO a correct solution.
Firefox builds use ucrt, which is sufficient for handling stdio (and
official winapi+clang-cl builds use that anyway).


Oh I didn't know that had migrated to UCRT.


I took a look at this and I think we want a more radical solution. Right
now we have _mingw_print_push.h/_mingw_print_pop.h hack that has no
clean purpose to me. I did some git history checking and when it was
introduced, it was important for __USE_MINGW_ANSI_STDIO because that was
where printf/scanf functions were #defined. It was later removed and the
only thing that's left there is a bunch of PRI* macros. Instead of
defining and redefining them, we may just as well define them correctly
in the first place.

That's what the attached patch does. It also uses the same for ucrt and
mingw stdio, leaving hackish values only for affected builds. This fixes
Firefox issue and makes mingw stdio cleaner. It's not yet tested, I'm
mostly sending it for comments now.


Yes those macros `scanf()` and `printf()` etc. were removed in
d4e56ae45d4f92598f7b393340efca93f3deff38 and now those functions are
implemented by conditional inline definitions. If no other inline
functions [1] in our headers rely on the old format specifiers, the
header <_mingw_print_push.h> is consequenctly unneeded thereafter.

The circumstances of <_mingw_print_pop.h> are a bit more complex. It has
to restore `PRId64` etc. to the correct strings that correspond to what
`printf()` etc., after the inclusion of , will eventually
call. Thus at the moment <_mingw_print_pop.h> actually checks for
inclusion of  or  (there was an old thread about this
[2]), otherwise the following code may not produce the expected result:

```c
#define __USE_MINGW_ANSI_STDIO  1
#include 
#include 

// `PRIx64` is defined as `I64x` here, which will co-operate with
// `printf()` from MSVCRT rather than the inline one in .


In my proposed patch, it will be properly defined to llx in inttypes.h 
(note that the patch makes inttypes.h aware of __USE_MINGW_ANSI_STDIO). 
By defining it to the right value in the first place, we don't need to 
redefine it later.


Thanks,
Jacek


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-28 Thread Liu Hao
在 2018/10/27 19:52, Jacek Caban 写道:
> I don't see what makes __USE_MINGW_ANSI_STDIO a correct solution.
> Firefox builds use ucrt, which is sufficient for handling stdio (and
> official winapi+clang-cl builds use that anyway).
> 

Oh I didn't know that had migrated to UCRT.

> I took a look at this and I think we want a more radical solution. Right
> now we have _mingw_print_push.h/_mingw_print_pop.h hack that has no
> clean purpose to me. I did some git history checking and when it was
> introduced, it was important for __USE_MINGW_ANSI_STDIO because that was
> where printf/scanf functions were #defined. It was later removed and the
> only thing that's left there is a bunch of PRI* macros. Instead of
> defining and redefining them, we may just as well define them correctly
> in the first place.
> 
> That's what the attached patch does. It also uses the same for ucrt and
> mingw stdio, leaving hackish values only for affected builds. This fixes
> Firefox issue and makes mingw stdio cleaner. It's not yet tested, I'm
> mostly sending it for comments now.
> 

Yes those macros `scanf()` and `printf()` etc. were removed in
d4e56ae45d4f92598f7b393340efca93f3deff38 and now those functions are
implemented by conditional inline definitions. If no other inline
functions [1] in our headers rely on the old format specifiers, the
header <_mingw_print_push.h> is consequenctly unneeded thereafter.

The circumstances of <_mingw_print_pop.h> are a bit more complex. It has
to restore `PRId64` etc. to the correct strings that correspond to what
`printf()` etc., after the inclusion of , will eventually
call. Thus at the moment <_mingw_print_pop.h> actually checks for
inclusion of  or  (there was an old thread about this
[2]), otherwise the following code may not produce the expected result:

```c
#define __USE_MINGW_ANSI_STDIO  1
#include 
#include 

// `PRIx64` is defined as `I64x` here, which will co-operate with
// `printf()` from MSVCRT rather than the inline one in .
extern int printf(const char *, ...);

int main(void)
  {
uint64_t value = 0x123456789abcdef0;
printf("value = %" PRIx64 "\n", value);
  }
```



[1] The functions in  concerned me. Now I believe they don't
suffer from this change as they only ever call Microsoft variants whose
names begin with an underscore e.g. `_vsnprintf()`.
[2]
https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/1471206950.20170625161106%40serebryakov.spb.ru/#msg35911673

-- 
Best regards,
LH_Mouse



signature.asc
Description: OpenPGP digital signature
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-27 Thread Jacek Caban

On 27/10/2018 07:57, Liu Hao wrote:


在 2018/10/27 上午5:57, Mateusz 写道:

W dniu 26.10.2018 o 22:35, Tom Ritter pisze:

This is not really a MinGW problem, but MinGW does diverge from other
compilers and it caused Firefox to crash.

MinGW defines a lot of I64[foo] format specifiers in inttypes.h.
clang and clang-cl don't use I64[foo] they use ll[foo]. (I64[foo] is
valid according to Microsoft. MinGW mentions "MS runtime does not yet
understand C9x standard "ll"" but at some point they started
supporting ll[foo].  And as I mentioned, that's what clang[-cl] uses.

Mozilla has our own implementation of printf that does the format
specifier parsing. We don't support I64[foo]. So using it caused data
corruption and general bad behavior. Switching to ll[foo] fixed it.

I have a patch here:
https://hg.mozilla.org/try/raw-file/eaae7782a1dd/build/build-clang/mingw-int.patch

You can try to define
__USE_MINGW_ANSI_STDIO
instead of patching mingw-w64



It is the correct solution to define `__USE_MINGW_ANSI_STDIO` when
compiling Firefox, just like what libstdc++ does.


I don't see what makes __USE_MINGW_ANSI_STDIO a correct solution. 
Firefox builds use ucrt, which is sufficient for handling stdio (and 
official winapi+clang-cl builds use that anyway).



As for the patch:  The name `DEFINE_TWO_VERSION` is not descriptive for
this macro's purpose, and not suitable within a standard library header
because it doesn't start with an underscore. A better name would look
like `__MINGW_SELECT_C99_STDIO_FORMAT`.



I took a look at this and I think we want a more radical solution. Right 
now we have _mingw_print_push.h/_mingw_print_pop.h hack that has no 
clean purpose to me. I did some git history checking and when it was 
introduced, it was important for __USE_MINGW_ANSI_STDIO because that was 
where printf/scanf functions were #defined. It was later removed and the 
only thing that's left there is a bunch of PRI* macros. Instead of 
defining and redefining them, we may just as well define them correctly 
in the first place.


That's what the attached patch does. It also uses the same for ucrt and 
mingw stdio, leaving hackish values only for affected builds. This fixes 
Firefox issue and makes mingw stdio cleaner. It's not yet tested, I'm 
mostly sending it for comments now.


Cheers,
Jacek
commit 3060d249c397feffe6e00dbad6be64879e97294b
Author: Jacek Caban 
Date:   Sat Oct 27 13:35:38 2018 +0200

inttypes.h: Take into account __USE_MINGW_ANSI_STDIO and msvcrt version 
instead of depending on _mingw_print_p*.h headers.

diff --git a/mingw-w64-headers/configure.ac b/mingw-w64-headers/configure.ac
index d83037e8..dda8a629 100644
--- a/mingw-w64-headers/configure.ac
+++ b/mingw-w64-headers/configure.ac
@@ -39,7 +39,7 @@ AM_CONDITIONAL([HAVE_WIDL],[AS_VAR_TEST_SET([WIDL])])
 
 # Checks for header files.
 
-BASEHEAD_LIST="crt/_bsd_types.h crt/_cygwin.h crt/_mingw.h crt/_mingw_mac.h 
crt/_mingw_print_push.h crt/_mingw_print_pop.h crt/_mingw_secapi.h 
crt/_mingw_unicode.h crt/_timeval.h crt/crtdefs.h crt/excpt.h crt/intrin.h 
crt/vadefs.h crt/tchar.h "$srcdir/include/*.h
+BASEHEAD_LIST="crt/_bsd_types.h crt/_cygwin.h crt/_mingw.h crt/_mingw_mac.h 
crt/_mingw_secapi.h crt/_mingw_unicode.h crt/_timeval.h crt/crtdefs.h 
crt/excpt.h crt/intrin.h crt/vadefs.h crt/tchar.h "$srcdir/include/*.h
 SECHEAD_LIST="$srcdir/crt/sec_api/stralign_s.h"
 for i in c dlg h16 hxx rh ver; do
   BASEHEAD_LIST="$BASEHEAD_LIST "$srcdir/include/*.$i
diff --git a/mingw-w64-headers/crt/_mingw_print_pop.h 
b/mingw-w64-headers/crt/_mingw_print_pop.h
deleted file mode 100644
index 046b6203..
--- a/mingw-w64-headers/crt/_mingw_print_pop.h
+++ /dev/null
@@ -1,136 +0,0 @@
-/**
- * This file has no copyright assigned and is placed in the Public Domain.
- * This file is part of the mingw-w64 runtime package.
- * No warranty is given; refer to the file DISCLAIMER.PD within this package.
- */
-
-/* Define __mingw_ macros.  */
-#if defined(__USE_MINGW_ANSI_STDIO) && (defined(_INC_STDIO) || 
defined(_WSTDIO_DEFINED)) && ((__USE_MINGW_ANSI_STDIO + 0) != 0)
-
-/* Redefine to GNU specific PRI... and SCN... macros.  */
-#if defined(_INTTYPES_H_) && defined(PRId64)
-#undef PRId64
-#undef PRIdLEAST64
-#undef PRIdFAST64
-#undef PRIdMAX
-#undef PRIi64
-#undef PRIiLEAST64
-#undef PRIiFAST64
-#undef PRIiMAX
-#undef PRIo64
-#undef PRIoLEAST64
-#undef PRIoFAST64
-#undef PRIoMAX
-#undef PRIu64
-#undef PRIuLEAST64
-#undef PRIuFAST64
-#undef PRIuMAX
-#undef PRIx64
-#undef PRIxLEAST64
-#undef PRIxFAST64
-#undef PRIxMAX
-#undef PRIX64
-#undef PRIXLEAST64
-#undef PRIXFAST64
-#undef PRIXMAX
-
-#undef SCNd64
-#undef SCNdLEAST64
-#undef SCNdFAST64
-#undef SCNdMAX
-#undef SCNi64
-#undef SCNiLEAST64
-#undef SCNiFAST64
-#undef SCNiMAX
-#undef SCNo64
-#undef SCNoLEAST64
-#undef SCNoFAST64
-#undef SCNoMAX
-#undef SCNx64
-#undef SCNxLEAST64
-#undef SCNxFAST64
-#undef SCNxMAX
-#undef SCNu64
-#undef SCNuLEAST64
-#undef SCNuFAST64
-#undef SCNuMAX
-
-#ifdef _WIN64
-#undef PRIdPTR

Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-27 Thread Vincent Torri
On Fri, Oct 26, 2018 at 11:58 PM Mateusz  wrote:
>
> W dniu 26.10.2018 o 22:35, Tom Ritter pisze:
> > This is not really a MinGW problem, but MinGW does diverge from other
> > compilers and it caused Firefox to crash.
> >
> > MinGW defines a lot of I64[foo] format specifiers in inttypes.h.
> > clang and clang-cl don't use I64[foo] they use ll[foo]. (I64[foo] is
> > valid according to Microsoft. MinGW mentions "MS runtime does not yet
> > understand C9x standard "ll"" but at some point they started
> > supporting ll[foo].  And as I mentioned, that's what clang[-cl] uses.
> >
> > Mozilla has our own implementation of printf that does the format
> > specifier parsing. We don't support I64[foo]. So using it caused data
> > corruption and general bad behavior. Switching to ll[foo] fixed it.
> >
> > I have a patch here:
> > https://hg.mozilla.org/try/raw-file/eaae7782a1dd/build/build-clang/mingw-int.patch
>
> You can try to define
> __USE_MINGW_ANSI_STDIO
> instead of patching mingw-w64

i've mesure a significant performance loss when using printf
functions with __USE_MINGW_ANSI_STDIO cmopared to the ones in
msvcrt.dll

is it normal ?

Vincent Torri


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-27 Thread Liu Hao
在 2018/10/27 上午5:57, Mateusz 写道:
> W dniu 26.10.2018 o 22:35, Tom Ritter pisze:
>> This is not really a MinGW problem, but MinGW does diverge from other
>> compilers and it caused Firefox to crash.
>>
>> MinGW defines a lot of I64[foo] format specifiers in inttypes.h.
>> clang and clang-cl don't use I64[foo] they use ll[foo]. (I64[foo] is
>> valid according to Microsoft. MinGW mentions "MS runtime does not yet
>> understand C9x standard "ll"" but at some point they started
>> supporting ll[foo].  And as I mentioned, that's what clang[-cl] uses.
>>
>> Mozilla has our own implementation of printf that does the format
>> specifier parsing. We don't support I64[foo]. So using it caused data
>> corruption and general bad behavior. Switching to ll[foo] fixed it.
>>
>> I have a patch here:
>> https://hg.mozilla.org/try/raw-file/eaae7782a1dd/build/build-clang/mingw-int.patch
> 
> You can try to define
> __USE_MINGW_ANSI_STDIO
> instead of patching mingw-w64
> 
> 

It is the correct solution to define `__USE_MINGW_ANSI_STDIO` when
compiling Firefox, just like what libstdc++ does.

As for the patch:  The name `DEFINE_TWO_VERSION` is not descriptive for
this macro's purpose, and not suitable within a standard library header
because it doesn't start with an underscore. A better name would look
like `__MINGW_SELECT_C99_STDIO_FORMAT`.


-- 
Best regards,
LH_Mouse



signature.asc
Description: OpenPGP digital signature
___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


Re: [Mingw-w64-public] inttypes Format Specifiers

2018-10-26 Thread Mateusz
W dniu 26.10.2018 o 22:35, Tom Ritter pisze:
> This is not really a MinGW problem, but MinGW does diverge from other
> compilers and it caused Firefox to crash.
> 
> MinGW defines a lot of I64[foo] format specifiers in inttypes.h.
> clang and clang-cl don't use I64[foo] they use ll[foo]. (I64[foo] is
> valid according to Microsoft. MinGW mentions "MS runtime does not yet
> understand C9x standard "ll"" but at some point they started
> supporting ll[foo].  And as I mentioned, that's what clang[-cl] uses.
> 
> Mozilla has our own implementation of printf that does the format
> specifier parsing. We don't support I64[foo]. So using it caused data
> corruption and general bad behavior. Switching to ll[foo] fixed it.
> 
> I have a patch here:
> https://hg.mozilla.org/try/raw-file/eaae7782a1dd/build/build-clang/mingw-int.patch

You can try to define
__USE_MINGW_ANSI_STDIO
instead of patching mingw-w64



___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public


[Mingw-w64-public] inttypes Format Specifiers

2018-10-26 Thread Tom Ritter
This is not really a MinGW problem, but MinGW does diverge from other
compilers and it caused Firefox to crash.

MinGW defines a lot of I64[foo] format specifiers in inttypes.h.
clang and clang-cl don't use I64[foo] they use ll[foo]. (I64[foo] is
valid according to Microsoft. MinGW mentions "MS runtime does not yet
understand C9x standard "ll"" but at some point they started
supporting ll[foo].  And as I mentioned, that's what clang[-cl] uses.

Mozilla has our own implementation of printf that does the format
specifier parsing. We don't support I64[foo]. So using it caused data
corruption and general bad behavior. Switching to ll[foo] fixed it.

I have a patch here:
https://hg.mozilla.org/try/raw-file/eaae7782a1dd/build/build-clang/mingw-int.patch

(Try run for Firefox is here if anyone is interested:
https://treeherder.mozilla.org/#/jobs?repo=try=5db7504d12cf2e5181f2fcd46e09df37e0e80159=208082989
)

It may be desirable to take this patch, even though I64[foo] is valid,
just because ll[foo] is much more common and it will unify behavior
across compilers.  (It would also be helpful for me; but we can
support local patches to MinGW if you don't want to upstream this.)

I64[foo] -> ll[foo] is not the only difference. The following
illustrates other differences:
https://ritter.vg/misc/transient/format-specifiers-diff.html

-tom


___
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public