I have committed all of the fixes that I had previously posted, but
before actually activating the warning option, I found another small
hiccup with the Bison files.
Before Bison 3.4, the generated parser implementation files run afoul of
-Wmissing-variable-declarations (in spite of commit ab61c40bfa2) because
declarations for yylval and possibly yylloc are missing. The generated
header files contain an extern declaration, but the implementation files
don't include the header files. Since Bison 3.4, the generated
implementation files automatically include the generated header files,
so then it works.
To make this work with older Bison versions as well, I made a patch to
include the generated header file from the .y file.
(With older Bison versions, the generated implementation file contains
effectively a copy of the header file pasted in, so including the header
file is redundant. But we know this works anyway because the core
grammar uses this arrangement already.)
From a4eadd43c9e0300bd6027c0d4d59f5eec11cc577 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 26 Jul 2024 10:17:53 +0200
Subject: [PATCH v3 1/2] Include bison header files into implementation files
Before Bison 3.4, the generated parser implementation files run afoul
of -Wmissing-variable-declarations (in spite of commit ab61c40bfa2)
because declarations for yylval and possibly yylloc are missing. The
generated header files contain an extern declaration, but the
implementation files don't include the header files. Since Bison 3.4,
the generated implementation files automatically include the generated
header files, so then it works.
To make this work with older Bison versions as well, include the
generated header file from the .y file.
(With older Bison versions, the generated implementation file contains
effectively a copy of the header file pasted in, so including the
header file is redundant. But we know this works anyway because the
core grammar uses this arrangement already.)
Discussion:
https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c9...@eisentraut.org
---
contrib/cube/cubeparse.y | 8 +++++---
contrib/seg/segparse.y | 1 +
src/backend/bootstrap/bootparse.y | 1 +
src/backend/replication/repl_gram.y | 1 +
src/backend/replication/syncrep_gram.y | 2 ++
src/interfaces/ecpg/preproc/ecpg.header | 1 +
src/pl/plpgsql/src/pl_gram.y | 1 +
src/test/isolation/specparse.y | 1 +
8 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/contrib/cube/cubeparse.y b/contrib/cube/cubeparse.y
index fd56d0e1628..52622875cbb 100644
--- a/contrib/cube/cubeparse.y
+++ b/contrib/cube/cubeparse.y
@@ -11,13 +11,15 @@
#include "utils/float.h"
#include "varatt.h"
+/* All grammar constructs return strings */
+#define YYSTYPE char *
+
+#include "cubeparse.h"
+
/* silence -Wmissing-variable-declarations */
extern int cube_yychar;
extern int cube_yynerrs;
-/* All grammar constructs return strings */
-#define YYSTYPE char *
-
/*
* Bison doesn't allocate anything that needs to live across parser calls,
* so we can easily have it use palloc instead of malloc. This prevents
diff --git a/contrib/seg/segparse.y b/contrib/seg/segparse.y
index 729d4b6390b..efa5fb749cf 100644
--- a/contrib/seg/segparse.y
+++ b/contrib/seg/segparse.y
@@ -12,6 +12,7 @@
#include "utils/float.h"
#include "segdata.h"
+#include "segparse.h"
/* silence -Wmissing-variable-declarations */
extern int seg_yychar;
diff --git a/src/backend/bootstrap/bootparse.y
b/src/backend/bootstrap/bootparse.y
index 58e0878dc8d..73a7592fb71 100644
--- a/src/backend/bootstrap/bootparse.y
+++ b/src/backend/bootstrap/bootparse.y
@@ -32,6 +32,7 @@
#include "nodes/makefuncs.h"
#include "utils/memutils.h"
+#include "bootparse.h"
/* silence -Wmissing-variable-declarations */
extern int boot_yychar;
diff --git a/src/backend/replication/repl_gram.y
b/src/backend/replication/repl_gram.y
index c46ca395263..06daa954813 100644
--- a/src/backend/replication/repl_gram.y
+++ b/src/backend/replication/repl_gram.y
@@ -22,6 +22,7 @@
#include "replication/walsender.h"
#include "replication/walsender_private.h"
+#include "repl_gram.h"
/* silence -Wmissing-variable-declarations */
extern int replication_yychar;
diff --git a/src/backend/replication/syncrep_gram.y
b/src/backend/replication/syncrep_gram.y
index 5ce4f1bfe73..e4d9962226c 100644
--- a/src/backend/replication/syncrep_gram.y
+++ b/src/backend/replication/syncrep_gram.y
@@ -17,6 +17,8 @@
#include "nodes/pg_list.h"
#include "replication/syncrep.h"
+#include "syncrep_gram.h"
+
/* Result of parsing is returned in one of these two variables */
SyncRepConfigData *syncrep_parse_result;
char *syncrep_parse_error_msg;
diff --git a/src/interfaces/ecpg/preproc/ecpg.header
b/src/interfaces/ecpg/preproc/ecpg.header
index 571b92f6434..3790a601d1a 100644
--- a/src/interfaces/ecpg/preproc/ecpg.header
+++ b/src/interfaces/ecpg/preproc/ecpg.header
@@ -5,6 +5,7 @@
#include "postgres_fe.h"
#include "preproc_extern.h"
+#include "preproc.h"
#include "ecpg_config.h"
#include <unistd.h>
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index 0671ff78722..8182ce28aa1 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -26,6 +26,7 @@
#include "plpgsql.h"
+#include "pl_gram.h"
/* silence -Wmissing-variable-declarations */
extern int plpgsql_yychar;
diff --git a/src/test/isolation/specparse.y b/src/test/isolation/specparse.y
index 282a7504556..788069d1ba5 100644
--- a/src/test/isolation/specparse.y
+++ b/src/test/isolation/specparse.y
@@ -13,6 +13,7 @@
#include "postgres_fe.h"
#include "isolationtester.h"
+#include "specparse.h"
/* silence -Wmissing-variable-declarations */
extern int spec_yychar;
base-commit: c7301c3b6fe2feaf96d52cbf35a85ac6b95374dc
--
2.45.2
From 395d35c28d5c4dbd20bd5e4a64fc20a70d3d7a5d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 26 Jul 2024 09:26:18 +0200
Subject: [PATCH v3 2/2] Add -Wmissing-variable-declarations to the standard
compilation flags
This warning flag detects global variables not declared in header
files. This is similar to what -Wmissing-prototypes does for
functions. (More correctly, it is similar to what
-Wmissing-declarations does for functions, but -Wmissing-prototypes is
a superset of that in C.)
This flag is new in GCC 14. Clang has supported it for a while.
Several recent commits have cleaned up warnings triggered by this, so
it should now be clean.
Reviewed-by: Andres Freund <and...@anarazel.de>
Discussion:
https://www.postgresql.org/message-id/flat/e0a62134-83da-4ba4-8cdb-ceb0111c9...@eisentraut.org
---
configure | 49 +++++++++++++++++++++++
configure.ac | 9 +++++
meson.build | 10 +++++
src/Makefile.global.in | 1 +
src/interfaces/ecpg/test/Makefile.regress | 2 +-
src/interfaces/ecpg/test/meson.build | 1 +
src/makefiles/meson.build | 2 +
src/tools/pg_bsd_indent/Makefile | 2 +
src/tools/pg_bsd_indent/meson.build | 1 +
9 files changed, 76 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 062d40e1ab2..c2b220e8cac 100755
--- a/configure
+++ b/configure
@@ -741,6 +741,7 @@ CXXFLAGS_SL_MODULE
CFLAGS_SL_MODULE
CFLAGS_VECTORIZE
CFLAGS_UNROLL_LOOPS
+PERMIT_MISSING_VARIABLE_DECLARATIONS
PERMIT_DECLARATION_AFTER_STATEMENT
LLVM_BINPATH
LLVM_CXXFLAGS
@@ -6080,6 +6081,54 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wformat_security"
= x"yes"; then
fi
+ # gcc 14+, clang for a while
+ # (Supported in C++ by clang but not gcc. For consistency, omit in C++.)
+ save_CFLAGS=$CFLAGS
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports
-Wmissing-variable-declarations, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wmissing-variable-declarations,
for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wmissing_variable_declarations+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wmissing-variable-declarations"
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_prog_CC_cflags__Wmissing_variable_declarations=yes
+else
+ pgac_cv_prog_CC_cflags__Wmissing_variable_declarations=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+CFLAGS="$pgac_save_CFLAGS"
+CC="$pgac_save_CC"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result:
$pgac_cv_prog_CC_cflags__Wmissing_variable_declarations" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wmissing_variable_declarations" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wmissing_variable_declarations" = x"yes";
then
+ CFLAGS="${CFLAGS} -Wmissing-variable-declarations"
+fi
+
+
+ PERMIT_MISSING_VARIABLE_DECLARATIONS=
+ if test x"$save_CFLAGS" != x"$CFLAGS"; then
+ PERMIT_MISSING_VARIABLE_DECLARATIONS=-Wno-missing-variable-declarations
+ fi
+
# Disable strict-aliasing rules; needed for gcc 3.3+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports
-fno-strict-aliasing, for CFLAGS" >&5
diff --git a/configure.ac b/configure.ac
index ef56226156a..48917718240 100644
--- a/configure.ac
+++ b/configure.ac
@@ -530,6 +530,15 @@ if test "$GCC" = yes -a "$ICC" = no; then
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
PGAC_PROG_CXX_CFLAGS_OPT([-Wformat-security])
+ # gcc 14+, clang for a while
+ # (Supported in C++ by clang but not gcc. For consistency, omit in C++.)
+ save_CFLAGS=$CFLAGS
+ PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-variable-declarations])
+ PERMIT_MISSING_VARIABLE_DECLARATIONS=
+ if test x"$save_CFLAGS" != x"$CFLAGS"; then
+ PERMIT_MISSING_VARIABLE_DECLARATIONS=-Wno-missing-variable-declarations
+ fi
+ AC_SUBST(PERMIT_MISSING_VARIABLE_DECLARATIONS)
# Disable strict-aliasing rules; needed for gcc 3.3+
PGAC_PROG_CC_CFLAGS_OPT([-fno-strict-aliasing])
PGAC_PROG_CXX_CFLAGS_OPT([-fno-strict-aliasing])
diff --git a/meson.build b/meson.build
index efde3a28cc9..733f8be47f7 100644
--- a/meson.build
+++ b/meson.build
@@ -1971,6 +1971,16 @@ if cc.has_argument('-Wdeclaration-after-statement')
cflags_no_decl_after_statement += '-Wno-declaration-after-statement'
endif
+# Some code is not clean for -Wmissing-variable-declarations, so we
+# make the "no" option available. Also, while clang supports this
+# option for C++, gcc does not, so for consistency, leave it off for
+# C++.
+cflags_no_missing_var_decls = []
+if cc.has_argument('-Wmissing-variable-declarations')
+ cflags_warn += '-Wmissing-variable-declarations'
+ cflags_no_missing_var_decls += '-Wno-missing-variable-declarations'
+endif
+
# The following tests want to suppress various unhelpful warnings by adding
# -Wno-foo switches. But gcc won't complain about unrecognized -Wno-foo
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 83b91fe9167..42f50b49761 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -266,6 +266,7 @@ CFLAGS_POPCNT = @CFLAGS_POPCNT@
CFLAGS_CRC = @CFLAGS_CRC@
CFLAGS_XSAVE = @CFLAGS_XSAVE@
PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@
+PERMIT_MISSING_VARIABLE_DECLARATIONS = @PERMIT_MISSING_VARIABLE_DECLARATIONS@
CXXFLAGS = @CXXFLAGS@
LLVM_CPPFLAGS = @LLVM_CPPFLAGS@
diff --git a/src/interfaces/ecpg/test/Makefile.regress
b/src/interfaces/ecpg/test/Makefile.regress
index b0647cd2c5f..9bf0efa40b9 100644
--- a/src/interfaces/ecpg/test/Makefile.regress
+++ b/src/interfaces/ecpg/test/Makefile.regress
@@ -3,7 +3,7 @@
override CPPFLAGS := -I../../include
-I$(top_srcdir)/src/interfaces/ecpg/include \
-I$(libpq_srcdir) $(CPPFLAGS)
-override CFLAGS += $(PTHREAD_CFLAGS)
+override CFLAGS += $(PTHREAD_CFLAGS) $(PERMIT_MISSING_VARIABLE_DECLARATIONS)
LDFLAGS_INTERNAL += -L../../ecpglib -lecpg -L../../pgtypeslib -lpgtypes
$(libpq)
diff --git a/src/interfaces/ecpg/test/meson.build
b/src/interfaces/ecpg/test/meson.build
index c1e508ccc82..8fd284071f2 100644
--- a/src/interfaces/ecpg/test/meson.build
+++ b/src/interfaces/ecpg/test/meson.build
@@ -27,6 +27,7 @@ testprep_targets += pg_regress_ecpg
# create .c files and executables from .pgc files
ecpg_test_exec_kw = {
+ 'c_args': cflags_no_missing_var_decls,
'dependencies': [frontend_code, libpq],
'include_directories': [ecpg_inc],
'link_with': [ecpglib_so, ecpg_compat_so, ecpg_pgtypes_so],
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index 5618050b306..850e9275845 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -98,6 +98,8 @@ pgxs_kv = {
'CXXFLAGS_SL_MODULE': ' '.join(cxxflags_mod),
'PERMIT_DECLARATION_AFTER_STATEMENT':
' '.join(cflags_no_decl_after_statement),
+ 'PERMIT_MISSING_VARIABLE_DECLARATIONS':
+ ' '.join(cflags_no_missing_var_decls),
'CFLAGS_CRC': ' '.join(cflags_crc),
'CFLAGS_POPCNT': ' '.join(cflags_popcnt),
diff --git a/src/tools/pg_bsd_indent/Makefile b/src/tools/pg_bsd_indent/Makefile
index d922013e40b..f721dfb0d19 100644
--- a/src/tools/pg_bsd_indent/Makefile
+++ b/src/tools/pg_bsd_indent/Makefile
@@ -25,6 +25,8 @@ OBJS = \
parse.o \
pr_comment.o
+$(OBJS): CFLAGS += $(PERMIT_MISSING_VARIABLE_DECLARATIONS)
+
all: pg_bsd_indent
pg_bsd_indent: $(OBJS) | submake-libpgport
diff --git a/src/tools/pg_bsd_indent/meson.build
b/src/tools/pg_bsd_indent/meson.build
index 4387c47740e..87ed4292975 100644
--- a/src/tools/pg_bsd_indent/meson.build
+++ b/src/tools/pg_bsd_indent/meson.build
@@ -18,6 +18,7 @@ endif
pg_bsd_indent = executable('pg_bsd_indent',
pg_bsd_indent_sources,
+ c_args: cflags_no_missing_var_decls,
dependencies: [frontend_code],
include_directories: include_directories('.'),
kwargs: default_bin_args + {
--
2.45.2
