On Wed, Apr 2, 2025 at 2:04 AM Peter Eisentraut <[email protected]> wrote:
> On 31.03.25 08:28, Kyotaro Horiguchi wrote:
> > I hadn’t paid much attention to this before, but I happened to check
> > how this behaves on Windows, and it seems that with VS2022, PRId64
> > expands to "%lld". As a result, I suspect the gettext message catalog
> > won't match these messages correctly.
>
> I think this is working correctly.  Gettext has a built-in mechanism to
> translate the %<PRI...> back to the appropriate  %lld or %ld.  See also
> <https://www.gnu.org/software/gettext/manual/html_node/c_002dformat.html>.

Interesting report though.  Commit 962da900 assumed that our in-tree
printf implementation still needed to understand that %I64 stuff in
case it came to us from system headers, but it looks like it
disappeared with MSVCRT:

1. I checked with CI (VS 2019).  puts(PRId64) prints out "lld".
2. MinGW's inttypes.h[1] only uses "I64" et al if you build against MSVCRT.

So I think we should delete that stuff.  Attached.

I worried that GNU gettext() might still know about %I64 somewhere,
but it just expands the macros to whatever inttypes.h defines[2].
Good.

We don't even test -Dnls on the Windows CI task, so the fact that it
passes there doesn't mean much (if our tests would even pick up
<PRI*64> expansion failure, not sure).  We should probably do
something about that and/or its absence from the build farm.  We're
effectively counting on the EDB packaging team or end users to tell us
if we break localisation on this platform.

I was also curious to know if the nearby floating point formatting
kludge added by commit f1885386 was still needed today.  CI passes
without it, and the standard is pretty clear: "The exponent always
contains at least two digits, and only as many more digits as
necessary to represent the exponent".  I didn't look too closely at
the fine print, but that text was already present in C89 so I guess
MSVCRT just failed to conform on that point.

[1] 
https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-headers/crt/inttypes.h
[2] 
https://github.com/autotools-mirror/gettext/blob/637b208fbe13f1c306f19d4f31c21fec7e9986d2/gettext-runtime/intl/loadmsgcat.c#L473
From 213d845611e5b357b6ff8af84f5365d96c32f51b Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Wed, 19 Nov 2025 13:45:27 +1300
Subject: [PATCH 1/2] Drop support for MSVCRT's %I64 format strings.

MSVCRT predated C99 and invented non-standard placeholders for 64-bit
numbers, and then later used them in standard macros when C99
<inttypes.h> arrived.  Those macros just use %lld etc when building with
UCRT, so there should be no way for our interposed sprintf.c code to
receive the pre-standard kind.  Time to drop the code that parses them.

That code was in fact already dead when commit 962da900 landed, as we'd
officially disclaimed MSVCRT support a couple of weeks earlier in commit
1758d424, but patch development overlapped and the history hadn't been
investigated.
---
 src/port/snprintf.c | 44 --------------------------------------------
 1 file changed, 44 deletions(-)

diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index d7f18b42d19..dded6c3f65b 100644
--- a/src/port/snprintf.c
+++ b/src/port/snprintf.c
@@ -557,28 +557,6 @@ nextch2:
 					fmtpos = accum;
 				accum = 0;
 				goto nextch2;
-#ifdef WIN32
-			case 'I':
-				/* Windows PRI*{32,64,PTR} size */
-				if (format[0] == '3' && format[1] == '2')
-					format += 2;
-				else if (format[0] == '6' && format[1] == '4')
-				{
-					format += 2;
-					longlongflag = 1;
-				}
-				else
-				{
-#if SIZEOF_VOID_P == SIZEOF_LONG
-					longflag = 1;
-#elif SIZEOF_VOID_P == SIZEOF_LONG_LONG
-					longlongflag = 1;
-#else
-#error "cannot find integer type of the same size as intptr_t"
-#endif
-				}
-				goto nextch2;
-#endif
 			case 'l':
 				if (longflag)
 					longlongflag = 1;
@@ -842,28 +820,6 @@ nextch1:
 					fmtpos = accum;
 				accum = 0;
 				goto nextch1;
-#ifdef WIN32
-			case 'I':
-				/* Windows PRI*{32,64,PTR} size */
-				if (format[0] == '3' && format[1] == '2')
-					format += 2;
-				else if (format[0] == '6' && format[1] == '4')
-				{
-					format += 2;
-					longlongflag = 1;
-				}
-				else
-				{
-#if SIZEOF_VOID_P == SIZEOF_LONG
-					longflag = 1;
-#elif SIZEOF_VOID_P == SIZEOF_LONG_LONG
-					longlongflag = 1;
-#else
-#error "cannot find integer type of the same size as intptr_t"
-#endif
-				}
-				goto nextch1;
-#endif
 			case 'l':
 				if (longflag)
 					longlongflag = 1;
-- 
2.51.2

From 368a43f169a92c9ec172f229a650f7dd6acf97a1 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Wed, 19 Nov 2025 14:25:42 +1300
Subject: [PATCH 2/2] Drop support for MSVCRT's float formatting quirk.

Commit f1885386 added code to remove an unnecessary leading zero from
the exponent in a float formatted by the system snprintf().  The C
standard doesn't allow unnecessary digits beyond two, and the tests pass
without this kludge on modern UCRT-based Windows systems.
---
 src/port/snprintf.c | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index dded6c3f65b..6541182df6d 100644
--- a/src/port/snprintf.c
+++ b/src/port/snprintf.c
@@ -1205,22 +1205,6 @@ fmtfloat(double value, char type, int forcesign, int leftjust,
 		}
 		if (vallen < 0)
 			goto fail;
-
-		/*
-		 * Windows, alone among our supported platforms, likes to emit
-		 * three-digit exponent fields even when two digits would do.  Hack
-		 * such results to look like the way everyone else does it.
-		 */
-#ifdef WIN32
-		if (vallen >= 6 &&
-			convert[vallen - 5] == 'e' &&
-			convert[vallen - 3] == '0')
-		{
-			convert[vallen - 3] = convert[vallen - 2];
-			convert[vallen - 2] = convert[vallen - 1];
-			vallen--;
-		}
-#endif
 	}
 
 	padlen = compute_padlen(minlen, vallen + zeropadlen, leftjust);
@@ -1336,17 +1320,6 @@ pg_strfromd(char *str, size_t count, int precision, double value)
 				target.failed = true;
 				goto fail;
 			}
-
-#ifdef WIN32
-			if (vallen >= 6 &&
-				convert[vallen - 5] == 'e' &&
-				convert[vallen - 3] == '0')
-			{
-				convert[vallen - 3] = convert[vallen - 2];
-				convert[vallen - 2] = convert[vallen - 1];
-				vallen--;
-			}
-#endif
 		}
 	}
 
-- 
2.51.2

Reply via email to