headerscheck warnings with late-model gcc
Using gcc 15.1.1 (from Fedora 42) I see these warnings that
didn't appear with older gcc:
$ src/tools/pginclude/headerscheck
In file included from /tmp/headerscheck.xp0AI5/test.c:2:
./src/common/kwlist_d.h:1163:23: warning: no previous declaration for 'ScanKeywords' [-Wmissing-variable-declarations]
1163 | const ScanKeywordList ScanKeywords = {
| ^~~~~~~~~~~~
In file included from /tmp/headerscheck.xp0AI5/test.c:2:
./src/interfaces/ecpg/test/preproc/strings.h:1:13: warning: no previous declaration for 's1' [-Wmissing-variable-declarations]
1 | char *s1,
| ^~
./src/interfaces/ecpg/test/preproc/strings.h:2:21: warning: no previous declaration for 's2' [-Wmissing-variable-declarations]
2 | *s2,
| ^~
./src/interfaces/ecpg/test/preproc/strings.h:3:21: warning: no previous declaration for 's3' [-Wmissing-variable-declarations]
3 | *s3,
| ^~
./src/interfaces/ecpg/test/preproc/strings.h:4:21: warning: no previous declaration for 's4' [-Wmissing-variable-declarations]
4 | *s4,
| ^~
./src/interfaces/ecpg/test/preproc/strings.h:5:21: warning: no previous declaration for 's5' [-Wmissing-variable-declarations]
5 | *s5,
| ^~
./src/interfaces/ecpg/test/preproc/strings.h:6:21: warning: no previous declaration for 's6' [-Wmissing-variable-declarations]
6 | *s6,
| ^~
./src/interfaces/ecpg/test/preproc/strings.h:7:21: warning: no previous declaration for 's7' [-Wmissing-variable-declarations]
7 | *s7,
| ^~
./src/interfaces/ecpg/test/preproc/strings.h:8:21: warning: no previous declaration for 's8' [-Wmissing-variable-declarations]
8 | *s8;
| ^~
While we could possibly get away with making headerscheck ignore that
ecpg test header, it seems unwise to skip kwlist_d.h. So I propose
the attached patch, which I've confirmed silences these warnings.
Curiously, no such complaints appear with cpluspluscheck (which is
using g++ 15.1.1). I don't really understand why not: why would
they have turned on -Wmissing-variable-declarations by default
for C but not C++? But anyway, since there doesn't seem to be
any C++ compatibility issue here, I think it's sufficient to fix
this in master and not back-patch.
regards, tom lane
Attachments:
silence-Wmissing-variable-declarations.patchtext/x-diff; charset=us-ascii; name=silence-Wmissing-variable-declarations.patchDownload
diff --git a/src/interfaces/ecpg/test/expected/preproc-strings.c b/src/interfaces/ecpg/test/expected/preproc-strings.c
index a26817968de..55859b624eb 100644
--- a/src/interfaces/ecpg/test/expected/preproc-strings.c
+++ b/src/interfaces/ecpg/test/expected/preproc-strings.c
@@ -18,6 +18,16 @@
#line 3 "strings.pgc"
/* exec sql begin declare section */
#line 1 "strings.h"
+/* This extern silences headerscheck warnings with some gcc versions */
+
+
+
+
+
+
+
+
+
@@ -29,7 +39,10 @@
#line 5 "strings.pgc"
-#line 1 "strings.h"
+#line 2 "strings.h"
+ extern char * s1 , * s2 , * s3 , * s4 , * s5 , * s6 , * s7 , * s8 ;
+
+#line 11 "strings.h"
char * s1 , * s2 , * s3 , * s4 , * s5 , * s6 , * s7 , * s8 ;
/* exec sql end declare section */
#line 5 "strings.pgc"
diff --git a/src/interfaces/ecpg/test/preproc/strings.h b/src/interfaces/ecpg/test/preproc/strings.h
index edb5be5339e..71581d6f94a 100644
--- a/src/interfaces/ecpg/test/preproc/strings.h
+++ b/src/interfaces/ecpg/test/preproc/strings.h
@@ -1,3 +1,13 @@
+/* This extern silences headerscheck warnings with some gcc versions */
+extern char *s1,
+ *s2,
+ *s3,
+ *s4,
+ *s5,
+ *s6,
+ *s7,
+ *s8;
+
char *s1,
*s2,
*s3,
diff --git a/src/tools/gen_keywordlist.pl b/src/tools/gen_keywordlist.pl
index 6ec83ff33f9..8825b4476ac 100644
--- a/src/tools/gen_keywordlist.pl
+++ b/src/tools/gen_keywordlist.pl
@@ -169,7 +169,16 @@ printf $kwdef qq|static %s\n|, $f;
# Emit the struct that wraps all this lookup info into one variable.
-printf $kwdef "static " if !$extern;
+if ($extern)
+{
+ # This redundant extern declaration is needed to silence headerscheck
+ # warnings with some gcc versions.
+ printf $kwdef "extern const ScanKeywordList %s;\n\n", $varname;
+}
+else
+{
+ printf $kwdef "static ";
+}
printf $kwdef "const ScanKeywordList %s = {\n", $varname;
printf $kwdef qq|\t%s_kw_string,\n|, $varname;
printf $kwdef qq|\t%s_kw_offsets,\n|, $varname;
On 05.08.25 20:09, Tom Lane wrote:
Curiously, no such complaints appear with cpluspluscheck (which is
using g++ 15.1.1). I don't really understand why not: why would
they have turned on -Wmissing-variable-declarations by default
for C but not C++? But anyway, since there doesn't seem to be
any C++ compatibility issue here, I think it's sufficient to fix
this in master and not back-patch.
-Wmissing-variable-declarations is added by us as of PG18 (commit
66188912566). It's available since gcc 14 and doesn't exist for C++.
Peter Eisentraut <peter@eisentraut.org> writes:
On 05.08.25 20:09, Tom Lane wrote:
Curiously, no such complaints appear with cpluspluscheck (which is
using g++ 15.1.1). I don't really understand why not ...
-Wmissing-variable-declarations is added by us as of PG18 (commit
66188912566). It's available since gcc 14 and doesn't exist for C++.
Oh! Okay, that explains the lack of messages, but it still seems
like an odd omission.
regards, tom lane
On 06.08.25 21:11, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
On 05.08.25 20:09, Tom Lane wrote:
Curiously, no such complaints appear with cpluspluscheck (which is
using g++ 15.1.1). I don't really understand why not ...-Wmissing-variable-declarations is added by us as of PG18 (commit
66188912566). It's available since gcc 14 and doesn't exist for C++.Oh! Okay, that explains the lack of messages, but it still seems
like an odd omission.
Yeah, this is pretty much my fault for not checking this for the above
commit.
I've been having a hard time getting headerscheck to work reliably in my
environment, so I ended up relying on CI, which doesn't have new-enough
compilers yet (and/or doesn't run it everywhere; the clang on the
FreeBSD task might have caught it (but we also don't have this
integrated with meson yet)).
Attached are three patches to fix some unrelated problems with
headerscheck in my environment.
The fourth one is to fix the ecpg issue; I think we can ignore it under
the "code fragment" category.
kwlist_d.h doesn't show up in my run, probably because I'm using a
separate build directory, which headerscheck doesn't handle? Another
thing to fix. But I guess it would also fall under the code fragment
category?
But the code fragment exception is also faulty, because we plausibly do
want to check that file for C++ compatibility, just not necessarily as a
standalone file. Not sure how to cover all these bases at once.
Attachments:
0001-headerscheck-Use-ICU_CFLAGS.patchtext/plain; charset=UTF-8; name=0001-headerscheck-Use-ICU_CFLAGS.patchDownload
From f40b5a065d96b3bca7c1e535cee81c89ee202c9e Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 7 Aug 2025 15:55:04 +0200
Subject: [PATCH 1/4] headerscheck: Use ICU_CFLAGS
Otherwise, headerscheck will fail if the ICU headers are in a location
not reached by the normal CFLAGS/CPPFLAGS:
../src/include/utils/pg_locale.h:21:10: fatal error: unicode/ucol.h: No such file or directory
---
src/tools/pginclude/headerscheck | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 9e86d049362..30afa967305 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -44,6 +44,7 @@ CXXFLAGS=${CXXFLAGS:- -fsyntax-only -Wall}
MGLOB="$builddir/src/Makefile.global"
CPPFLAGS=`sed -n 's/^CPPFLAGS[ ]*=[ ]*//p' "$MGLOB"`
CFLAGS=`sed -n 's/^CFLAGS[ ]*=[ ]*//p' "$MGLOB"`
+ICU_CFLAGS=`sed -n 's/^ICU_CFLAGS[ ]*=[ ]*//p' "$MGLOB"`
CC=`sed -n 's/^CC[ ]*=[ ]*//p' "$MGLOB"`
CXX=`sed -n 's/^CXX[ ]*=[ ]*//p' "$MGLOB"`
PG_SYSROOT=`sed -n 's/^PG_SYSROOT[ ]*=[ ]*//p' "$MGLOB"`
@@ -64,11 +65,11 @@ if $cplusplus; then
-I*|-D*) CXXPPFLAGS="$CXXPPFLAGS $flag";;
esac
done
- COMPILER_FLAGS="$CXXPPFLAGS $CXXFLAGS"
+ COMPILER_FLAGS="$CXXPPFLAGS $CXXFLAGS $ICU_CFLAGS"
else
ext=c
COMPILER=${CC:-gcc}
- COMPILER_FLAGS="$CPPFLAGS $CFLAGS"
+ COMPILER_FLAGS="$CPPFLAGS $CFLAGS $ICU_CFLAGS"
fi
# Create temp directory.
--
2.50.1
0002-headerscheck-Ignore-llvmjit_backport.h.patchtext/plain; charset=UTF-8; name=0002-headerscheck-Ignore-llvmjit_backport.h.patchDownload
From eedfa689ce1a940b1187bcbd29d65437999052d6 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 7 Aug 2025 15:57:12 +0200
Subject: [PATCH 2/4] headerscheck: Ignore llvmjit_backport.h
This requires --with-llvm, which is not always active. (So perhaps in
a similar category as sepgsql.h.)
XXX alternative: Only check if --with-llvm is enabled?
---
src/tools/pginclude/headerscheck | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 30afa967305..e984a441edb 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -172,6 +172,9 @@ do
# SectionMemoryManager.h is C++
test "$f" = src/include/jit/SectionMemoryManager.h && continue
+ # XXX depends on --with-llvm
+ test "$f" = src/include/jit/llvmjit_backport.h && continue
+
# ppport.h is not under our control, so we can't make it standalone.
test "$f" = src/pl/plperl/ppport.h && continue
--
2.50.1
0003-headerscheck-Ignore-Windows-specific-header.patchtext/plain; charset=UTF-8; name=0003-headerscheck-Ignore-Windows-specific-header.patchDownload
From 7ea0c526f41fcc30951f1a25acaedb65f340eeed Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 7 Aug 2025 15:58:34 +0200
Subject: [PATCH 3/4] headerscheck: Ignore Windows-specific header
Ignore src/include/port/win32/sys/resource.h. At least on macOS,
including this results in warnings and errors because of duplication
with system headers:
../src/include/port/win32/sys/resource.h:10:9: warning: 'RUSAGE_CHILDREN' redefined
../src/include/port/win32/sys/resource.h:16:1: error: redefinition of struct or union 'struct rusage'
Since we are also not checking similar system-replacement headers for
Windows, it makes sense to exclude this one, too.
---
src/tools/pginclude/headerscheck | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index e984a441edb..850bf293ae5 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -98,7 +98,7 @@ do
# Additional Windows-specific headers.
test "$f" = src/include/port/win32_port.h && continue
test "$f" = src/include/port/win32/netdb.h && continue
- $cplusplus && test "$f" = src/include/port/win32/sys/resource.h && continue
+ test "$f" = src/include/port/win32/sys/resource.h && continue
test "$f" = src/include/port/win32/sys/socket.h && continue
test "$f" = src/include/port/win32_msvc/dirent.h && continue
test "$f" = src/include/port/win32_msvc/utime.h && continue
--
2.50.1
0004-headerscheck-Ignore-ecpg-test-header.patchtext/plain; charset=UTF-8; name=0004-headerscheck-Ignore-ecpg-test-header.patchDownload
From 00e0e2694cdce7d5d0760fb35d3ea15551c94e10 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 7 Aug 2025 16:03:17 +0200
Subject: [PATCH 4/4] headerscheck: Ignore ecpg test header
This fails headerscheck because of
-Wmissing-variable-declarations (somewhat recently added by commit
66188912566). Ignore it, since it's just a code fragment, not a real
header file.
---
src/tools/pginclude/headerscheck | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/tools/pginclude/headerscheck b/src/tools/pginclude/headerscheck
index 850bf293ae5..5ca3db9deb0 100755
--- a/src/tools/pginclude/headerscheck
+++ b/src/tools/pginclude/headerscheck
@@ -182,6 +182,8 @@ do
test "$f" = src/interfaces/ecpg/test/regression.h && continue
# printf_hack.h produces "unused function" warnings.
test "$f" = src/interfaces/ecpg/test/printf_hack.h && continue
+ # code fragment
+ test "$f" = src/interfaces/ecpg/test/preproc/strings.h && continue
if $cplusplus; then
# pg_trace.h and utils/probes.h can include sys/sdt.h from SystemTap,
--
2.50.1
Peter Eisentraut <peter@eisentraut.org> writes:
Attached are three patches to fix some unrelated problems with
headerscheck in my environment.
0001 seems fine; it's an oversight that I'd not noticed because
ICU_CFLAGS is empty in my usage.
Don't like 0002 as-is. I'd be okay with skipping that header if
--with-llvm isn't given. However, we already tell users that
they'd better configure --with-perl and --with-python, so maybe
just add --with-llvm to that list?
0003: +1, I noticed that too yesterday.
0004: I prefer the solution I exhibited yesterday, ie add
externs to those headers.
kwlist_d.h doesn't show up in my run, probably because I'm using a
separate build directory, which headerscheck doesn't handle? Another
thing to fix.
Yeah, as it stands headerscheck is really only meant for in-tree
builds. We will definitely have to do something about that to make
it handle built headers in meson builds, and I guess it'd be nice if
VPATH works that way as well. One idea could be to redefine it as
searching the installation include-file tree instead of the source
tree, so that the build process washes out of the matter.
regards, tom lane
Hi,
On 2025-08-07 10:58:56 -0400, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
kwlist_d.h doesn't show up in my run, probably because I'm using a
separate build directory, which headerscheck doesn't handle? Another
thing to fix.Yeah, as it stands headerscheck is really only meant for in-tree
builds. We will definitely have to do something about that to make
it handle built headers in meson builds, and I guess it'd be nice if
VPATH works that way as well. One idea could be to redefine it as
searching the installation include-file tree instead of the source
tree, so that the build process washes out of the matter.
One thing I dislike rather intensely about cpluspluscheck/headerscheck is that
they're abominally slow and thus can't just be executed as part of normal
compile-test-edit cycles. That's obviously due to a) not being incremental b)
not being parallel.
The reason I bring that up here is that the fix for that would be to move the
iteration over all headers from headerscheck to the build system, which then
could perform the check only if headers changed and in parallel. That'd
obviously also address VPATH / meson builds.
The challenge with that is the buildsystem needs to be aware of the list of
headers. However, if we had that awareness, we'd gain two additional benefits:
1) The compile_commands.json that meson generates would provide the
information necessary to compile headers, which would allow editors to do
auto-completion etc for header files.
2) We'd be able to install headers with meson using an explicit list, rather
than the install_subdir(), which has issues with editor files as mentioned
in src/include/meson.build
It's possible to do this by globing for files at configure time, but that
wouldn't detect adding new headers (which would need to trigger a
re-configure). Whether that's an issue worth caring about I'm a bit on the
fence about.
Greetings,
Andres Freund
On 07.08.25 16:58, Tom Lane wrote:
Peter Eisentraut <peter@eisentraut.org> writes:
Attached are three patches to fix some unrelated problems with
headerscheck in my environment.0001 seems fine; it's an oversight that I'd not noticed because
ICU_CFLAGS is empty in my usage.
committed
Don't like 0002 as-is. I'd be okay with skipping that header if
--with-llvm isn't given. However, we already tell users that
they'd better configure --with-perl and --with-python, so maybe
just add --with-llvm to that list?
Yes, just added documentation.
0003: +1, I noticed that too yesterday.
committed
0004: I prefer the solution I exhibited yesterday, ie add
externs to those headers.
That solution seems fine, too.
This comment should be clarified:
+ # This redundant extern declaration is needed to silence headerscheck
+ # warnings with some gcc versions.You could just write
# silence -Wmissing-variable-declarations
which also appears elsewhere in the code.
Peter Eisentraut <peter@eisentraut.org> writes:
On 07.08.25 16:58, Tom Lane wrote:
0004: I prefer the solution I exhibited yesterday, ie add
externs to those headers.
That solution seems fine, too.
Great.
This comment should be clarified: + # This redundant extern declaration is needed to silence headerscheck + # warnings with some gcc versions. You could just write # silence -Wmissing-variable-declarations which also appears elsewhere in the code.
I compromised on writing "redundant declaration to silence
-Wmissing-variable-declarations", and pushed it.
regards, tom lane