Hi,
On 2022-10-06 13:00:41 +1300, David Rowley wrote:
> Here's a patch which (I think) fixes the ones I missed.
Yep, does the trick for me.
I attached a patch to add -Wshadow=compatible-local to our set of warnings.
> diff --git a/contrib/hstore/hstore.h b/contrib/hstore/hstore.h
> index 4713e6ea7a..897af244a4 100644
> --- a/contrib/hstore/hstore.h
> +++ b/contrib/hstore/hstore.h
> @@ -128,15 +128,15 @@ typedef struct
> /* finalize a newly-constructed hstore */
> #define HS_FINALIZE(hsp_,count_,buf_,ptr_)
> \
> do {
> \
> - int buflen = (ptr_) - (buf_);
> \
> + int _buflen = (ptr_) - (buf_);
> \
Not pretty. Given that HS_FINALIZE already has multiple-eval hazards, perhaps
we could just remove the local?
> --- a/src/interfaces/libpq/fe-secure-gssapi.c
> +++ b/src/interfaces/libpq/fe-secure-gssapi.c
> @@ -135,11 +135,11 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
> */
> if (PqGSSSendLength)
> {
> - ssize_t ret;
> + ssize_t retval;
That looks like it could easily lead to confusion further down the
line. Wouldn't the better fix here be to remove the inner variable?
> --- a/src/pl/plpython/plpy_exec.c
> +++ b/src/pl/plpython/plpy_exec.c
> @@ -375,11 +375,11 @@ PLy_exec_trigger(FunctionCallInfo fcinfo, PLyProcedure
> *proc)
> rv = NULL;
> else if (pg_strcasecmp(srv, "MODIFY") == 0)
> {
> - TriggerData *tdata = (TriggerData *)
> fcinfo->context;
> + TriggerData *trigdata = (TriggerData *)
> fcinfo->context;
>
> - if (TRIGGER_FIRED_BY_INSERT(tdata->tg_event) ||
> -
> TRIGGER_FIRED_BY_UPDATE(tdata->tg_event))
> - rv = PLy_modify_tuple(proc, plargs,
> tdata, rv);
> + if (TRIGGER_FIRED_BY_INSERT(trigdata->tg_event)
> ||
> +
> TRIGGER_FIRED_BY_UPDATE(trigdata->tg_event))
> + rv = PLy_modify_tuple(proc, plargs,
> trigdata, rv);
> else
> ereport(WARNING,
> (errmsg("PL/Python
> trigger function returned \"MODIFY\" in a DELETE trigger -- ignored")));
This doesn't strike me as a good fix either. Isn't the inner tdata exactly
the same as the outer tdata?
tdata = (TriggerData *) fcinfo->context;
...
TriggerData *trigdata = (TriggerData *)
fcinfo->context;
> --- a/src/test/modules/test_integerset/test_integerset.c
> +++ b/src/test/modules/test_integerset/test_integerset.c
> @@ -585,26 +585,26 @@ test_huge_distances(void)
This is one of the cases where our insistence on -Wdeclaration-after-statement
really makes this unnecessary ugly... Declaring x at the start of the function
just makes this harder to read.
Anyway, this isn't important code, and your fix seem ok.
Greetings,
Andres Freund
diff --git i/configure w/configure
index 1caca21b625..a5a03f6cec3 100755
--- i/configure
+++ w/configure
@@ -5852,6 +5852,97 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wcast_function_type" = x"yes"; then
fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wshadow=compatible-local, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wshadow=compatible-local, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wshadow_compatible_local+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wshadow=compatible-local"
+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__Wshadow_compatible_local=yes
+else
+ pgac_cv_prog_CC_cflags__Wshadow_compatible_local=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__Wshadow_compatible_local" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wshadow_compatible_local" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wshadow_compatible_local" = x"yes"; then
+ CFLAGS="${CFLAGS} -Wshadow=compatible-local"
+fi
+
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Wshadow=compatible-local, for CXXFLAGS" >&5
+$as_echo_n "checking whether ${CXX} supports -Wshadow=compatible-local, for CXXFLAGS... " >&6; }
+if ${pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CXXFLAGS=$CXXFLAGS
+pgac_save_CXX=$CXX
+CXX=${CXX}
+CXXFLAGS="${CXXFLAGS} -Wshadow=compatible-local"
+ac_save_cxx_werror_flag=$ac_cxx_werror_flag
+ac_cxx_werror_flag=yes
+ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+ pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local=yes
+else
+ pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+ac_cxx_werror_flag=$ac_save_cxx_werror_flag
+CXXFLAGS="$pgac_save_CXXFLAGS"
+CXX="$pgac_save_CXX"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local" >&5
+$as_echo "$pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local" >&6; }
+if test x"$pgac_cv_prog_CXX_cxxflags__Wshadow_compatible_local" = x"yes"; then
+ CXXFLAGS="${CXXFLAGS} -Wshadow=compatible-local"
+fi
+
+
# This was included in -Wall/-Wformat in older GCC versions
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wformat-security, for CFLAGS" >&5
diff --git i/configure.ac w/configure.ac
index 10fa55dd154..c696566a7ff 100644
--- i/configure.ac
+++ w/configure.ac
@@ -508,6 +508,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CXX_CFLAGS_OPT([-Wimplicit-fallthrough=3])
PGAC_PROG_CC_CFLAGS_OPT([-Wcast-function-type])
PGAC_PROG_CXX_CFLAGS_OPT([-Wcast-function-type])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wshadow=compatible-local])
+ PGAC_PROG_CXX_CFLAGS_OPT([-Wshadow=compatible-local])
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
PGAC_PROG_CXX_CFLAGS_OPT([-Wformat-security])
diff --git i/meson.build w/meson.build
index 25a6fa941cc..ec6c45d39b9 100644
--- i/meson.build
+++ w/meson.build
@@ -1708,6 +1708,7 @@ common_warning_flags = [
'-Wmissing-format-attribute',
'-Wimplicit-fallthrough=3',
'-Wcast-function-type',
+ '-Wshadow=compatible-local',
# This was included in -Wall/-Wformat in older GCC versions
'-Wformat-security',
]
