I feel like this is the type of change where there's not much
discussion to be had. And the only way to resolve it is to use some
voting to gauge community opinion.
So my suggestion is for people to respond with -1, -0.5, +-0, +0.5, or
+1 to indicate support against/for the change.
I'll start: +1
Attached is a patch that removes -Wdeclaration-after-statement in the
codebase. This is mainly to be able to add it to the commitfest, to
hopefully get a decent amount of responses.
From 105a4aaff2d142b40d8b093f67597d6340f66b68 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <jelte.fennema@microsoft.com>
Date: Mon, 29 Jan 2024 14:34:37 +0100
Subject: [PATCH v1] Remove declaration-after-statement requirement
This disables the declaration-after-statement requirement, there's no
need for it anymore now that we don't require C89 support in any of the
backbranches.
---
configure | 47 -------------------------------
configure.ac | 8 ------
meson.build | 9 ------
src/Makefile.global.in | 1 -
src/common/Makefile | 7 -----
src/common/meson.build | 14 ++-------
src/include/c.h | 7 ++---
src/makefiles/meson.build | 2 --
src/pl/plperl/plperl_system.h | 6 ++--
src/pl/plpython/plpython_system.h | 5 ++--
10 files changed, 10 insertions(+), 96 deletions(-)
diff --git a/configure b/configure
index 2a1ee251f2d..0459285c184 100755
--- a/configure
+++ b/configure
@@ -738,7 +738,6 @@ CXXFLAGS_SL_MODULE
CFLAGS_SL_MODULE
CFLAGS_VECTORIZE
CFLAGS_UNROLL_LOOPS
-PERMIT_DECLARATION_AFTER_STATEMENT
LLVM_BINPATH
LLVM_CXXFLAGS
LLVM_CFLAGS
@@ -5378,52 +5377,6 @@ if test "$GCC" = yes -a "$ICC" = no; then
# These work in some but not all gcc versions
save_CFLAGS=$CFLAGS
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wdeclaration-after-statement, for CFLAGS" >&5
-$as_echo_n "checking whether ${CC} supports -Wdeclaration-after-statement, for CFLAGS... " >&6; }
-if ${pgac_cv_prog_CC_cflags__Wdeclaration_after_statement+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- pgac_save_CFLAGS=$CFLAGS
-pgac_save_CC=$CC
-CC=${CC}
-CFLAGS="${CFLAGS} -Wdeclaration-after-statement"
-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__Wdeclaration_after_statement=yes
-else
- pgac_cv_prog_CC_cflags__Wdeclaration_after_statement=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__Wdeclaration_after_statement" >&5
-$as_echo "$pgac_cv_prog_CC_cflags__Wdeclaration_after_statement" >&6; }
-if test x"$pgac_cv_prog_CC_cflags__Wdeclaration_after_statement" = x"yes"; then
- CFLAGS="${CFLAGS} -Wdeclaration-after-statement"
-fi
-
-
- # -Wdeclaration-after-statement isn't applicable for C++. Specific C files
- # disable it, so AC_SUBST the negative form.
- PERMIT_DECLARATION_AFTER_STATEMENT=
- if test x"$save_CFLAGS" != x"$CFLAGS"; then
- PERMIT_DECLARATION_AFTER_STATEMENT=-Wno-declaration-after-statement
- fi
-
# Really don't want VLAs to be used in our dialect of C
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Werror=vla, for CFLAGS" >&5
diff --git a/configure.ac b/configure.ac
index 52fd7af4468..481ddaa7381 100644
--- a/configure.ac
+++ b/configure.ac
@@ -509,14 +509,6 @@ if test "$GCC" = yes -a "$ICC" = no; then
CXXFLAGS="-Wall -Wpointer-arith"
# These work in some but not all gcc versions
save_CFLAGS=$CFLAGS
- PGAC_PROG_CC_CFLAGS_OPT([-Wdeclaration-after-statement])
- # -Wdeclaration-after-statement isn't applicable for C++. Specific C files
- # disable it, so AC_SUBST the negative form.
- PERMIT_DECLARATION_AFTER_STATEMENT=
- if test x"$save_CFLAGS" != x"$CFLAGS"; then
- PERMIT_DECLARATION_AFTER_STATEMENT=-Wno-declaration-after-statement
- fi
- AC_SUBST(PERMIT_DECLARATION_AFTER_STATEMENT)
# Really don't want VLAs to be used in our dialect of C
PGAC_PROG_CC_CFLAGS_OPT([-Werror=vla])
# On macOS, complain about usage of symbols newer than the deployment target
diff --git a/meson.build b/meson.build
index 8ed51b6aae8..38ab42150c9 100644
--- a/meson.build
+++ b/meson.build
@@ -1835,15 +1835,6 @@ if llvm.found()
cxxflags_warn += cpp.get_supported_arguments(common_warning_flags)
endif
-# A few places with imported code get a pass on -Wdeclaration-after-statement, remember
-# the result for them
-cflags_no_decl_after_statement = []
-if cc.has_argument('-Wdeclaration-after-statement')
- cflags_warn += '-Wdeclaration-after-statement'
- cflags_no_decl_after_statement += '-Wno-declaration-after-statement'
-endif
-
-
# The following tests want to suppress various unhelpful warnings by adding
# -Wno-foo switches. But gcc won't complain about unrecognized -Wno-foo
# switches, so we have to test for the positive form and if that works,
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 8b3f8c24e08..e674729739e 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -263,7 +263,6 @@ CXXFLAGS_SL_MODULE = @CXXFLAGS_SL_MODULE@
CFLAGS_UNROLL_LOOPS = @CFLAGS_UNROLL_LOOPS@
CFLAGS_VECTORIZE = @CFLAGS_VECTORIZE@
CFLAGS_CRC = @CFLAGS_CRC@
-PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@
CXXFLAGS = @CXXFLAGS@
LLVM_CPPFLAGS = @LLVM_CPPFLAGS@
diff --git a/src/common/Makefile b/src/common/Makefile
index 2ba5069dca4..4b092a5178b 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -187,13 +187,6 @@ kwlist_d.h: $(top_srcdir)/src/include/parser/kwlist.h $(GEN_KEYWORDLIST_DEPS)
# that you don't get broken parsing code, even in a non-enable-depend build.
keywords.o keywords_shlib.o keywords_srv.o: kwlist_d.h
-# The code imported from Ryu gets a pass on declaration-after-statement,
-# in order to keep it more closely aligned with its upstream.
-RYU_FILES = d2s.o f2s.o
-RYU_OBJS = $(RYU_FILES) $(RYU_FILES:%.o=%_shlib.o) $(RYU_FILES:%.o=%_srv.o)
-
-$(RYU_OBJS): CFLAGS += $(PERMIT_DECLARATION_AFTER_STATEMENT)
-
clean distclean:
rm -f libpgcommon.a libpgcommon_shlib.a libpgcommon_srv.a
rm -f $(OBJS_FRONTEND) $(OBJS_SHLIB) $(OBJS_SRV)
diff --git a/src/common/meson.build b/src/common/meson.build
index 4eb16024cb2..ec4666e2018 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -2,6 +2,7 @@
common_sources = files(
'archive.c',
+ 'd2s.c',
'base64.c',
'binaryheap.c',
'blkreftable.c',
@@ -10,6 +11,7 @@ common_sources = files(
'controldata_utils.c',
'encnames.c',
'exec.c',
+ 'f2s.c',
'file_perm.c',
'file_utils.c',
'hashfn.c',
@@ -63,16 +65,6 @@ common_kwlist = custom_target('kwlist',
generated_sources += common_kwlist
common_sources += common_kwlist
-# The code imported from Ryu gets a pass on declaration-after-statement,
-# in order to keep it more closely aligned with its upstream.
-ryu_sources = files(
- 'd2s.c',
- 'f2s.c',
-)
-ryu_cflags = []
-
-ryu_cflags += cflags_no_decl_after_statement
-
config_info_sources = files('config_info.c',)
config_info_cflags = [
'-DVAL_CC="@0@"'.format(var_cc),
@@ -88,11 +80,9 @@ config_info_cflags = [
# Some files need to be built with different cflags. The different sets are
# defined here.
common_cflags = {
- 'ryu': ryu_cflags,
'config_info': config_info_cflags,
}
common_sources_cflags = {
- 'ryu': ryu_sources,
'config_info': config_info_sources
}
diff --git a/src/include/c.h b/src/include/c.h
index 2e3ea206e1c..c1b7e628887 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -903,10 +903,9 @@ extern void ExceptionalCondition(const char *conditionName,
* C11 has _Static_assert(), and most C99 compilers already support that. For
* portability, we wrap it into StaticAssertDecl(). _Static_assert() is a
* "declaration", and so it must be placed where for example a variable
- * declaration would be valid. As long as we compile with
- * -Wno-declaration-after-statement, that also means it cannot be placed after
- * statements in a function. Macros StaticAssertStmt() and StaticAssertExpr()
- * make it safe to use as a statement or in an expression, respectively.
+ * declaration would be valid. Macros StaticAssertStmt() and
+ * StaticAssertExpr() make it safe to use as a statement or in an expression,
+ * respectively.
*
* For compilers without _Static_assert(), we fall back on a kluge that
* assumes the compiler will complain about a negative width for a struct
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index b0f4178b3d9..4955ace1464 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -96,8 +96,6 @@ pgxs_kv = {
'CFLAGS_SL': var_cflags_sl,
'CFLAGS_SL_MODULE': ' '.join(cflags_mod),
'CXXFLAGS_SL_MODULE': ' '.join(cxxflags_mod),
- 'PERMIT_DECLARATION_AFTER_STATEMENT':
- ' '.join(cflags_no_decl_after_statement),
'CFLAGS_CRC': ' '.join(cflags_crc),
'CFLAGS_UNROLL_LOOPS': ' '.join(unroll_loops_cflags),
diff --git a/src/pl/plperl/plperl_system.h b/src/pl/plperl/plperl_system.h
index 48f6575e513..4bac332d460 100644
--- a/src/pl/plperl/plperl_system.h
+++ b/src/pl/plperl/plperl_system.h
@@ -19,9 +19,9 @@
/*
* Newer versions of the perl headers trigger a lot of warnings with our
- * preferred compiler flags (at least -Wdeclaration-after-statement,
- * -Wshadow=compatible-local are known to be problematic). The system_header
- * pragma hides warnings from within the rest of this file, if supported.
+ * preferred compiler flags (at least Wshadow=compatible-local is known to be
+ * problematic). The system_header pragma hides warnings from within the rest
+ * of this file, if supported.
*/
#ifdef HAVE_PRAGMA_GCC_SYSTEM_HEADER
#pragma GCC system_header
diff --git a/src/pl/plpython/plpython_system.h b/src/pl/plpython/plpython_system.h
index 036d339bf83..29c91d4f751 100644
--- a/src/pl/plpython/plpython_system.h
+++ b/src/pl/plpython/plpython_system.h
@@ -19,9 +19,8 @@
/*
* Newer versions of the Python headers trigger a lot of warnings with our
- * preferred compiler flags (at least -Wdeclaration-after-statement is known
- * to be problematic). The system_header pragma hides warnings from within
- * the rest of this file, if supported.
+ * preferred compiler flags. The system_header pragma hides warnings from
+ * within the rest of this file, if supported.
*/
#ifdef HAVE_PRAGMA_GCC_SYSTEM_HEADER
#pragma GCC system_header
base-commit: 6a1ea02c491d16474a6214603dce40b5b122d4d1
--
2.34.1
