public inbox for [email protected]  
help / color / mirror / Atom feed
some extra warnings from MSVC
3+ messages / 3 participants
[nested] [flat]

* some extra warnings from MSVC
@ 2026-04-11 10:37 Peter Eisentraut <[email protected]>
  2026-04-11 14:09 ` Re: some extra warnings from MSVC Tom Lane <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Peter Eisentraut @ 2026-04-11 10:37 UTC (permalink / raw)
  To: pgsql-hackers

I ran the MSVC build with the warning level set one level higher than 
the default (that is, meson -Dwarning_level=2, which actually maps to 
what MSVC considers its level 3), and it gave two new warnings that 
should be fixed:

../src/backend/utils/error/elog.c(1255): warning C4013: 'wchar2char' 
undefined; assuming extern returning int

../src/backend/utils/hash/dynahash.c(1767): warning C4334: '<<': result 
of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)

These are both in code that is new in PG19.

The first one is from commit 65707ed9afc (Add backtrace support for 
Windows).  This would be an error in gcc (from C99 on); it's kind of 
incredible that MSVC doesn't even warn about this by default.  I propose 
to add this warning category to the default set.

(Second thought: For consistency, make this an error, with '/we4013' 
instead of '/w24013'.)

The second one is from commit 13b935cd521 (Change dynahash.c and 
hsearch.h to use int64 instead of long).  I don't have a patch here to 
include this in the default warning set, mainly because it doesn't 
appear to map to any gcc warning option, but maybe we should add it 
anyway, since it can catch this kind of 4-byte-long-on-Windows issue.

From b0345430ae1bae1cea7092fde562aa137ac8b0c9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 11 Apr 2026 12:23:23 +0200
Subject: [PATCH 1/3] MSVC: Add warning for missing function declaration

---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index be97e986e5d..240d0869af9 100644
--- a/meson.build
+++ b/meson.build
@@ -2328,6 +2328,7 @@ if cc.get_id() == 'msvc'
     '/wd4244', # conversion from 'type1' to 'type2', possible loss of data
 
     # Additional warnings to enable:
+    '/w24013', # 'function' undefined; assuming extern returning int' [like -Wimplicit-function-declaration]
     '/w24062', # enumerator 'identifier' in switch of enum 'enumeration' is not handled [like -Wswitch]
     '/w24102', # unreferenced label [like -Wunused-label]
   ]
-- 
2.53.0


From 7e5031524cc7f981e856cdd4726a36966b19da61 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 11 Apr 2026 12:23:23 +0200
Subject: [PATCH 2/3] MSVC: Add missing include

../src/backend/utils/error/elog.c(1255): warning C4013: 'wchar2char' undefined; assuming extern returning int
---
 src/backend/utils/error/elog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index c270c62e213..ad960336e8d 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -86,6 +86,7 @@
 #include "tcop/tcopprot.h"
 #include "utils/guc_hooks.h"
 #include "utils/memutils.h"
+#include "utils/pg_locale.h"
 #include "utils/ps_status.h"
 #include "utils/varlena.h"
 
-- 
2.53.0


From c57160dd0d6cc4bc0ebcac3b58b89428c090da6c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 11 Apr 2026 12:23:23 +0200
Subject: [PATCH 3/3] Fix 64-bit shifting in dynahash.c

The switch from long to int64 in commit 13b935cd521 was incomplete.
It was shifting the constant 1L, which is not always 64 bit.  Fix by
using an explicit int64 constant.

MSVC warning:

../src/backend/utils/hash/dynahash.c(1767): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)
---
 src/backend/utils/hash/dynahash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 20610f96e7b..4596e5c7476 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -1764,7 +1764,7 @@ static int64
 next_pow2_int64(int64 num)
 {
 	/* my_log2's internal range check is sufficient */
-	return 1L << my_log2(num);
+	return (int64) 1 << my_log2(num);
 }
 
 /* calculate first power of 2 >= num, bounded to what will fit in an int */
-- 
2.53.0



Attachments:

  [text/plain] 0001-MSVC-Add-warning-for-missing-function-declaration.patch (851B, 2-0001-MSVC-Add-warning-for-missing-function-declaration.patch)
  download | inline diff:
From b0345430ae1bae1cea7092fde562aa137ac8b0c9 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 11 Apr 2026 12:23:23 +0200
Subject: [PATCH 1/3] MSVC: Add warning for missing function declaration

---
 meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meson.build b/meson.build
index be97e986e5d..240d0869af9 100644
--- a/meson.build
+++ b/meson.build
@@ -2328,6 +2328,7 @@ if cc.get_id() == 'msvc'
     '/wd4244', # conversion from 'type1' to 'type2', possible loss of data
 
     # Additional warnings to enable:
+    '/w24013', # 'function' undefined; assuming extern returning int' [like -Wimplicit-function-declaration]
     '/w24062', # enumerator 'identifier' in switch of enum 'enumeration' is not handled [like -Wswitch]
     '/w24102', # unreferenced label [like -Wunused-label]
   ]
-- 
2.53.0



  [text/plain] 0002-MSVC-Add-missing-include.patch (784B, 3-0002-MSVC-Add-missing-include.patch)
  download | inline diff:
From 7e5031524cc7f981e856cdd4726a36966b19da61 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 11 Apr 2026 12:23:23 +0200
Subject: [PATCH 2/3] MSVC: Add missing include

../src/backend/utils/error/elog.c(1255): warning C4013: 'wchar2char' undefined; assuming extern returning int
---
 src/backend/utils/error/elog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index c270c62e213..ad960336e8d 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -86,6 +86,7 @@
 #include "tcop/tcopprot.h"
 #include "utils/guc_hooks.h"
 #include "utils/memutils.h"
+#include "utils/pg_locale.h"
 #include "utils/ps_status.h"
 #include "utils/varlena.h"
 
-- 
2.53.0



  [text/plain] 0003-Fix-64-bit-shifting-in-dynahash.c.patch (1.1K, 4-0003-Fix-64-bit-shifting-in-dynahash.c.patch)
  download | inline diff:
From c57160dd0d6cc4bc0ebcac3b58b89428c090da6c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
Date: Sat, 11 Apr 2026 12:23:23 +0200
Subject: [PATCH 3/3] Fix 64-bit shifting in dynahash.c

The switch from long to int64 in commit 13b935cd521 was incomplete.
It was shifting the constant 1L, which is not always 64 bit.  Fix by
using an explicit int64 constant.

MSVC warning:

../src/backend/utils/hash/dynahash.c(1767): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?)
---
 src/backend/utils/hash/dynahash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 20610f96e7b..4596e5c7476 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -1764,7 +1764,7 @@ static int64
 next_pow2_int64(int64 num)
 {
 	/* my_log2's internal range check is sufficient */
-	return 1L << my_log2(num);
+	return (int64) 1 << my_log2(num);
 }
 
 /* calculate first power of 2 >= num, bounded to what will fit in an int */
-- 
2.53.0



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: some extra warnings from MSVC
  2026-04-11 10:37 some extra warnings from MSVC Peter Eisentraut <[email protected]>
@ 2026-04-11 14:09 ` Tom Lane <[email protected]>
  2026-04-19 22:18   ` Re: some extra warnings from MSVC Michael Paquier <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Tom Lane @ 2026-04-11 14:09 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers

Peter Eisentraut <[email protected]> writes:
> The first one is from commit 65707ed9afc (Add backtrace support for 
> Windows).  This would be an error in gcc (from C99 on); it's kind of 
> incredible that MSVC doesn't even warn about this by default.  I propose 
> to add this warning category to the default set.
> (Second thought: For consistency, make this an error, with '/we4013' 
> instead of '/w24013'.)

+1 for making it an error.

> The second one is from commit 13b935cd521 (Change dynahash.c and 
> hsearch.h to use int64 instead of long).  I don't have a patch here to 
> include this in the default warning set, mainly because it doesn't 
> appear to map to any gcc warning option, but maybe we should add it 
> anyway, since it can catch this kind of 4-byte-long-on-Windows issue.

I think it'd be a good idea to warn even if we can't make gcc do that.
I think Windows is the only 64-bit platform we deal with where long
is just 32 bits, so covering the case in MSVC will expose bugs we
would not notice otherwise.

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: some extra warnings from MSVC
  2026-04-11 10:37 some extra warnings from MSVC Peter Eisentraut <[email protected]>
  2026-04-11 14:09 ` Re: some extra warnings from MSVC Tom Lane <[email protected]>
@ 2026-04-19 22:18   ` Michael Paquier <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Michael Paquier @ 2026-04-19 22:18 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; pgsql-hackers

On Sat, Apr 11, 2026 at 10:09:58AM -0400, Tom Lane wrote:
> I think it'd be a good idea to warn even if we can't make gcc do that.
> I think Windows is the only 64-bit platform we deal with where long
> is just 32 bits, so covering the case in MSVC will expose bugs we
> would not notice otherwise.

Interesting issue.  I did not notice this thread until 9018c7d37bb4.
Please do not hesitate to add me in CC in the future, as 13b935cd521
has my fingerprints.  Thanks for fixing the issue.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, 2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2026-04-19 22:18 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-11 10:37 some extra warnings from MSVC Peter Eisentraut <[email protected]>
2026-04-11 14:09 ` Tom Lane <[email protected]>
2026-04-19 22:18   ` Michael Paquier <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox