pgsql: Support FETCH FIRST WITH TIES
Support FETCH FIRST WITH TIES
WITH TIES is an option to the FETCH FIRST N ROWS clause (the SQL
standard's spelling of LIMIT), where you additionally get rows that
compare equal to the last of those N rows by the columns in the
mandatory ORDER BY clause.
There was a proposal by Andrew Gierth to implement this functionality in
a more powerful way that would yield more features, but the other patch
had not been finished at this time, so we decided to use this one for
now in the spirit of incremental development.
Author: Surafel Temesgen <surafel3000@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Discussion: /messages/by-id/CALAY4q9ky7rD_A4vf=FVQvCGngm3LOes-ky0J6euMrg=_Se+ag@mail.gmail.com
Discussion: /messages/by-id/87o8wvz253.fsf@news-spur.riddles.org.uk
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/357889eb17bb9c9336c4f324ceb1651da616fe57
Modified Files
--------------
doc/src/sgml/ref/select.sgml | 15 +--
src/backend/catalog/sql_features.txt | 2 +-
src/backend/executor/nodeLimit.c | 160 +++++++++++++++++++++++++++---
src/backend/nodes/copyfuncs.c | 7 ++
src/backend/nodes/equalfuncs.c | 2 +
src/backend/nodes/outfuncs.c | 7 ++
src/backend/nodes/readfuncs.c | 6 ++
src/backend/optimizer/plan/createplan.c | 45 ++++++++-
src/backend/optimizer/plan/planner.c | 1 +
src/backend/optimizer/util/pathnode.c | 2 +
src/backend/parser/analyze.c | 21 ++--
src/backend/parser/gram.y | 117 +++++++++++++++++-----
src/backend/parser/parse_clause.c | 15 ++-
src/backend/utils/adt/ruleutils.c | 27 ++++--
src/include/catalog/catversion.h | 2 +-
src/include/nodes/execnodes.h | 5 +
src/include/nodes/nodes.h | 13 +++
src/include/nodes/parsenodes.h | 2 +
src/include/nodes/pathnodes.h | 1 +
src/include/nodes/plannodes.h | 5 +
src/include/optimizer/pathnode.h | 1 +
src/include/optimizer/planmain.h | 5 +-
src/include/parser/parse_clause.h | 3 +-
src/test/regress/expected/limit.out | 167 ++++++++++++++++++++++++++++++++
src/test/regress/sql/limit.sql | 48 +++++++++
25 files changed, 610 insertions(+), 69 deletions(-)
Hi Álvaro,
On Tue, Apr 7, 2020 at 10:28 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Support FETCH FIRST WITH TIES
WITH TIES is an option to the FETCH FIRST N ROWS clause (the SQL
standard's spelling of LIMIT), where you additionally get rows that
compare equal to the last of those N rows by the columns in the
mandatory ORDER BY clause.There was a proposal by Andrew Gierth to implement this functionality in
a more powerful way that would yield more features, but the other patch
had not been finished at this time, so we decided to use this one for
now in the spirit of incremental development.Author: Surafel Temesgen <surafel3000@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Discussion: /messages/by-id/CALAY4q9ky7rD_A4vf=FVQvCGngm3LOes-ky0J6euMrg=_Se+ag@mail.gmail.com
Discussion: /messages/by-id/87o8wvz253.fsf@news-spur.riddles.org.ukBranch
------
masterDetails
-------
https://git.postgresql.org/pg/commitdiff/357889eb17bb9c9336c4f324ceb1651da616fe57Modified Files
--------------
doc/src/sgml/ref/select.sgml | 15 +--
src/backend/catalog/sql_features.txt | 2 +-
src/backend/executor/nodeLimit.c | 160 +++++++++++++++++++++++++++---
src/backend/nodes/copyfuncs.c | 7 ++
src/backend/nodes/equalfuncs.c | 2 +
src/backend/nodes/outfuncs.c | 7 ++
src/backend/nodes/readfuncs.c | 6 ++
src/backend/optimizer/plan/createplan.c | 45 ++++++++-
src/backend/optimizer/plan/planner.c | 1 +
src/backend/optimizer/util/pathnode.c | 2 +
src/backend/parser/analyze.c | 21 ++--
src/backend/parser/gram.y | 117 +++++++++++++++++-----
src/backend/parser/parse_clause.c | 15 ++-
src/backend/utils/adt/ruleutils.c | 27 ++++--
src/include/catalog/catversion.h | 2 +-
src/include/nodes/execnodes.h | 5 +
src/include/nodes/nodes.h | 13 +++
src/include/nodes/parsenodes.h | 2 +
src/include/nodes/pathnodes.h | 1 +
src/include/nodes/plannodes.h | 5 +
src/include/optimizer/pathnode.h | 1 +
src/include/optimizer/planmain.h | 5 +-
src/include/parser/parse_clause.h | 3 +-
src/test/regress/expected/limit.out | 167 ++++++++++++++++++++++++++++++++
src/test/regress/sql/limit.sql | 48 +++++++++
25 files changed, 610 insertions(+), 69 deletions(-)
FTR I now get the following when compiling with -Wimplicit-fallthrough -Werror:
nodeLimit.c: In function ‘ExecLimit’:
nodeLimit.c:136:7: error: this statement may fall through
[-Werror=implicit-fallthrough=]
136 | if (ScanDirectionIsForward(direction))
| ^
nodeLimit.c:216:3: note: here
216 | case LIMIT_WINDOWEND_TIES:
| ^~~~
It seems that this fall-through comment:
[...]
else
{
node->lstate = LIMIT_WINDOWEND_TIES;
/* fall-through */
}
[...]
Is not recognized by my compiler (gcc (Gentoo 9.3.0 p1) 9.3.0). If
that's something we should fix, PFA a naive patch for that.
Attachments:
fix_ties_warning.difftext/x-patch; charset=US-ASCII; name=fix_ties_warning.diffDownload
diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index 12cfc5177b..99d33fbc21 100644
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -212,6 +212,8 @@ ExecLimit(PlanState *pstate)
node->position--;
break;
}
+ Assert(node->lstate == LIMIT_WINDOWEND_TIES);
+ /* fall-through */
case LIMIT_WINDOWEND_TIES:
if (ScanDirectionIsForward(direction))
Julien Rouhaud <rjuju123@gmail.com> writes:
On Tue, Apr 7, 2020 at 10:28 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Support FETCH FIRST WITH TIES
FTR I now get the following when compiling with -Wimplicit-fallthrough -Werror:
Yeah, assorted buildfarm animals are mentioning that too. I wonder
if we should add that to the default warning options selected by
configure? I don't remember if that's been discussed before.
It seems that this fall-through comment:
/* fall-through */
Is not recognized by my compiler (gcc (Gentoo 9.3.0 p1) 9.3.0). If
that's something we should fix, PFA a naive patch for that.
Hmm, I feel like this logic is baroque enough to need more of a rewrite
than that :-(. But not sure exactly what would be better, so your
patch seems reasonable for now. The comments could use some help too.
regards, tom lane
On Sat, Apr 11, 2020 at 02:47:34PM -0400, Tom Lane wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
On Tue, Apr 7, 2020 at 10:28 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Support FETCH FIRST WITH TIES
FTR I now get the following when compiling with -Wimplicit-fallthrough -Werror:
Yeah, assorted buildfarm animals are mentioning that too. I wonder
if we should add that to the default warning options selected by
configure? I don't remember if that's been discussed before.
I'm all for it. It seems like a trap easy to catch up early, and we do want to
enforce it anyway. I'm attaching a simple patch for that if needed, hopefully
with the correct autoconf version.
It seems that this fall-through comment:
/* fall-through */
Is not recognized by my compiler (gcc (Gentoo 9.3.0 p1) 9.3.0). If
that's something we should fix, PFA a naive patch for that.Hmm, I feel like this logic is baroque enough to need more of a rewrite
than that :-(. But not sure exactly what would be better, so your
patch seems reasonable for now. The comments could use some help too.
Yes I just checked the state machine to make sure that the fallthrough was
expected, but the comments are now way better, thanks!
Attachments:
v1-implicit-fallthrough.difftext/plain; charset=us-asciiDownload
diff --git a/configure b/configure
index 83abe872aa..739b30d5fa 100755
--- a/configure
+++ b/configure
@@ -5552,6 +5552,97 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wmissing_format_attribute" = x"yes"; then
fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wimplicit-fallthrough, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wimplicit-fallthrough, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wimplicit_fallthrough+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wimplicit-fallthrough"
+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__Wimplicit_fallthrough=yes
+else
+ pgac_cv_prog_CC_cflags__Wimplicit_fallthrough=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__Wimplicit_fallthrough" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wimplicit_fallthrough" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wimplicit_fallthrough" = x"yes"; then
+ CFLAGS="${CFLAGS} -Wimplicit-fallthrough"
+fi
+
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Wimplicit-fallthrough, for CXXFLAGS" >&5
+$as_echo_n "checking whether ${CXX} supports -Wimplicit-fallthrough, for CXXFLAGS... " >&6; }
+if ${pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CXXFLAGS=$CXXFLAGS
+pgac_save_CXX=$CXX
+CXX=${CXX}
+CXXFLAGS="${CXXFLAGS} -Wimplicit-fallthrough"
+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__Wimplicit_fallthrough=yes
+else
+ pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough=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__Wimplicit_fallthrough" >&5
+$as_echo "$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough" >&6; }
+if test x"$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough" = x"yes"; then
+ CXXFLAGS="${CXXFLAGS} -Wimplicit-fallthrough"
+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 a/configure.in b/configure.in
index ecdf172396..49816a83dc 100644
--- a/configure.in
+++ b/configure.in
@@ -496,6 +496,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CXX_CFLAGS_OPT([-Wendif-labels])
PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-format-attribute])
PGAC_PROG_CXX_CFLAGS_OPT([-Wmissing-format-attribute])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wimplicit-fallthrough])
+ PGAC_PROG_CXX_CFLAGS_OPT([-Wimplicit-fallthrough])
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
PGAC_PROG_CXX_CFLAGS_OPT([-Wformat-security])
Julien Rouhaud <rjuju123@gmail.com> writes:
On Sat, Apr 11, 2020 at 02:47:34PM -0400, Tom Lane wrote:
Yeah, assorted buildfarm animals are mentioning that too. I wonder
if we should add that to the default warning options selected by
configure? I don't remember if that's been discussed before.
I'm all for it. It seems like a trap easy to catch up early, and we do want to
enforce it anyway. I'm attaching a simple patch for that if needed, hopefully
with the correct autoconf version.
Poking around in the archives, it seems like the only previous formal
proposal to add -Wimplicit-fallthrough was in the context of a much
more aggressive proposal to make a lot of non-Wall warnings into
errors [1]/messages/by-id/B9FB1155-B39D-43C9-A7E6-B67E1C59E4CE@gmail.com, which people did not like.
-Wimplicit-fallthrough does have some issues, eg it seems that it's
applied at a stage where gcc hasn't yet figured out that elog(ERROR)
doesn't return, so you need to add breaks after those. But we had
sort of agreed that we could have it on-by-default in one relevant
discussion [2]/messages/by-id/E1fDenm-0000C8-IJ@gemulon.postgresql.org, and then failed to pull the trigger.
If we do this, I suggest we use -Wimplicit-fallthrough=4, which
uses a more-restrictive-than-default definition of how a "fallthrough"
comment can be spelled. The default, per the gcc manual, is
* -Wimplicit-fallthrough=3 case sensitively matches one of the
following regular expressions:
*<"-fallthrough">
*<"@fallthrough@">
*<"lint -fallthrough[ \t]*">
*<"[ \t.!]*(ELSE,? |INTENTIONAL(LY)? )?FALL(S |
|-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)?">
*<"[ \t.!]*(Else,? |Intentional(ly)? )?Fall((s |
|-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)?">
*<"[ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )?fall(s |
|-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?">
which to my eyes is not exactly encouraging a project-standard style
for these, plus it seems like it might accept some things we'd rather
it didn't. The only more-restrictive alternative, short of disabling
the comments altogether, is
* -Wimplicit-fallthrough=4 case sensitively matches one of the
following regular expressions:
*<"-fallthrough">
*<"@fallthrough@">
*<"lint -fallthrough[ \t]*">
*<"[ \t]*FALLTHR(OUGH|U)[ \t]*">
Thoughts?
regards, tom lane
[1]: /messages/by-id/B9FB1155-B39D-43C9-A7E6-B67E1C59E4CE@gmail.com
[2]: /messages/by-id/E1fDenm-0000C8-IJ@gemulon.postgresql.org
On Apr 12, 2020, at 7:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Poking around in the archives, it seems like the only previous formal
proposal to add -Wimplicit-fallthrough was in the context of a much
more aggressive proposal to make a lot of non-Wall warnings into
errors [1], which people did not like.
That was from me.
The only more-restrictive alternative, short of disabling
the comments altogether, is* -Wimplicit-fallthrough=4 case sensitively matches one of the
following regular expressions:*<"-fallthrough">
*<"@fallthrough@">
*<"lint -fallthrough[ \t]*">
*<"[ \t]*FALLTHR(OUGH|U)[ \t]*">Thoughts?
Naturally, I'm +1 for this.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Sun, Apr 12, 2020 at 5:25 PM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
On Apr 12, 2020, at 7:55 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Poking around in the archives, it seems like the only previous formal
proposal to add -Wimplicit-fallthrough was in the context of a much
more aggressive proposal to make a lot of non-Wall warnings into
errors [1], which people did not like.That was from me.
The only more-restrictive alternative, short of disabling
the comments altogether, is* -Wimplicit-fallthrough=4 case sensitively matches one of the
following regular expressions:*<"-fallthrough">
*<"@fallthrough@">
*<"lint -fallthrough[ \t]*">
*<"[ \t]*FALLTHR(OUGH|U)[ \t]*">Thoughts?
Naturally, I'm +1 for this.
+1 too, obviously.
Attachments:
v2-implicit-fallthrough.difftext/x-patch; charset=US-ASCII; name=v2-implicit-fallthrough.diffDownload
diff --git a/configure b/configure
index 83abe872aa..4dfe4fcfe1 100755
--- a/configure
+++ b/configure
@@ -5552,6 +5552,97 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wmissing_format_attribute" = x"yes"; then
fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wimplicit-fallthrough=4, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wimplicit-fallthrough=4, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wimplicit-fallthrough=4"
+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__Wimplicit_fallthrough_4=yes
+else
+ pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4=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__Wimplicit_fallthrough_4" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4" = x"yes"; then
+ CFLAGS="${CFLAGS} -Wimplicit-fallthrough=4"
+fi
+
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Wimplicit-fallthrough=4, for CXXFLAGS" >&5
+$as_echo_n "checking whether ${CXX} supports -Wimplicit-fallthrough=4, for CXXFLAGS... " >&6; }
+if ${pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CXXFLAGS=$CXXFLAGS
+pgac_save_CXX=$CXX
+CXX=${CXX}
+CXXFLAGS="${CXXFLAGS} -Wimplicit-fallthrough=4"
+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__Wimplicit_fallthrough_4=yes
+else
+ pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4=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__Wimplicit_fallthrough_4" >&5
+$as_echo "$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4" >&6; }
+if test x"$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4" = x"yes"; then
+ CXXFLAGS="${CXXFLAGS} -Wimplicit-fallthrough=4"
+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 a/configure.in b/configure.in
index ecdf172396..94fba67261 100644
--- a/configure.in
+++ b/configure.in
@@ -496,6 +496,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CXX_CFLAGS_OPT([-Wendif-labels])
PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-format-attribute])
PGAC_PROG_CXX_CFLAGS_OPT([-Wmissing-format-attribute])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wimplicit-fallthrough=4])
+ PGAC_PROG_CXX_CFLAGS_OPT([-Wimplicit-fallthrough=4])
# This was included in -Wall/-Wformat in older GCC versions
PGAC_PROG_CC_CFLAGS_OPT([-Wformat-security])
PGAC_PROG_CXX_CFLAGS_OPT([-Wformat-security])
Do we intend to see this done in the current cycle?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Do we intend to see this done in the current cycle?
I don't have an objection to doing it now. It's just a new compiler
warning option, it shouldn't be able to break any code. (Plus there's
plenty of time to revert, if somehow it causes a problem.)
regards, tom lane
On 2020-Apr-12, Tom Lane wrote:
The only more-restrictive alternative, short of disabling
the comments altogether, is* -Wimplicit-fallthrough=4 case sensitively matches one of the
following regular expressions:*<"-fallthrough">
*<"@fallthrough@">
*<"lint -fallthrough[ \t]*">
*<"[ \t]*FALLTHR(OUGH|U)[ \t]*">Thoughts?
This doesn't allow whitespace between "fall" and "through", which means
we generate 217 such warnings currently. Or we can just use
-Wimplicit-fallthrough=3, which does allow whitespace (among other
detritus).
For my own reference, the manual is at
https://gcc.gnu.org/onlinedocs/gcc-8.3.0/gcc/Warning-Options.html#index-Wimplicit-fallthrough
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-May-06, Alvaro Herrera wrote:
This doesn't allow whitespace between "fall" and "through", which means
we generate 217 such warnings currently. Or we can just use
-Wimplicit-fallthrough=3, which does allow whitespace (among other
detritus).
If we're OK with patching all those places, I volunteer to do so. Any
objections? Or I can keep it at level 3, which can be done with minimal
patching.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2020-May-06, Alvaro Herrera wrote:
This doesn't allow whitespace between "fall" and "through", which means
we generate 217 such warnings currently. Or we can just use
-Wimplicit-fallthrough=3, which does allow whitespace (among other
detritus).
If we're OK with patching all those places, I volunteer to do so. Any
objections? Or I can keep it at level 3, which can be done with minimal
patching.
If we're gonna do it at all, I think we should go for the level 4
warnings. Level 3's idea of a fallthrough comment is too liberal
for my tastes...
regards, tom lane
On Mon, May 11, 2020 at 6:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2020-May-06, Alvaro Herrera wrote:
This doesn't allow whitespace between "fall" and "through", which means
we generate 217 such warnings currently. Or we can just use
-Wimplicit-fallthrough=3, which does allow whitespace (among other
detritus).If we're OK with patching all those places, I volunteer to do so. Any
objections? Or I can keep it at level 3, which can be done with minimal
patching.If we're gonna do it at all, I think we should go for the level 4
warnings. Level 3's idea of a fallthrough comment is too liberal
for my tastes...
Here's a patch that also takes care of cleaning all warning due to the
level 4 setting (at least the one I got with my other config options).
I went with "FALLTHRU" as this seemed more used.
Note that timezone/zic.c would also have to be changed.
Attachments:
v3-implicit-fallthrough.difftext/x-patch; charset=US-ASCII; name=v3-implicit-fallthrough.diffDownload
diff --git a/configure b/configure
index 83abe872aa..4dfe4fcfe1 100755
--- a/configure
+++ b/configure
@@ -5552,6 +5552,97 @@ if test x"$pgac_cv_prog_CXX_cxxflags__Wmissing_format_attribute" = x"yes"; then
fi
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wimplicit-fallthrough=4, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Wimplicit-fallthrough=4, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Wimplicit-fallthrough=4"
+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__Wimplicit_fallthrough_4=yes
+else
+ pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4=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__Wimplicit_fallthrough_4" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Wimplicit_fallthrough_4" = x"yes"; then
+ CFLAGS="${CFLAGS} -Wimplicit-fallthrough=4"
+fi
+
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Wimplicit-fallthrough=4, for CXXFLAGS" >&5
+$as_echo_n "checking whether ${CXX} supports -Wimplicit-fallthrough=4, for CXXFLAGS... " >&6; }
+if ${pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CXXFLAGS=$CXXFLAGS
+pgac_save_CXX=$CXX
+CXX=${CXX}
+CXXFLAGS="${CXXFLAGS} -Wimplicit-fallthrough=4"
+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__Wimplicit_fallthrough_4=yes
+else
+ pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4=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__Wimplicit_fallthrough_4" >&5
+$as_echo "$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4" >&6; }
+if test x"$pgac_cv_prog_CXX_cxxflags__Wimplicit_fallthrough_4" = x"yes"; then
+ CXXFLAGS="${CXXFLAGS} -Wimplicit-fallthrough=4"
+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 a/configure.in b/configure.in
index ecdf172396..94fba67261 100644
--- a/configure.in
+++ b/configure.in
@@ -496,6 +496,8 @@ if test "$GCC" = yes -a "$ICC" = no; then
PGAC_PROG_CXX_CFLAGS_OPT([-Wendif-labels])
PGAC_PROG_CC_CFLAGS_OPT([-Wmissing-format-attribute])
PGAC_PROG_CXX_CFLAGS_OPT([-Wmissing-format-attribute])
+ PGAC_PROG_CC_CFLAGS_OPT([-Wimplicit-fallthrough=4])
+ PGAC_PROG_CXX_CFLAGS_OPT([-Wimplicit-fallthrough=4])
# 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 a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 56b35622f1..85f823e2fd 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -814,7 +814,7 @@ heapam_relation_copy_for_cluster(Relation OldHeap, Relation NewHeap,
break;
case HEAPTUPLE_RECENTLY_DEAD:
*tups_recently_dead += 1;
- /* fall through */
+ /* FALLTHRU */
case HEAPTUPLE_LIVE:
/* Live or recently dead, must copy it */
isdead = false;
diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 5565e6fc19..a43d5b46db 100644
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -615,7 +615,7 @@ findDependentObjects(const ObjectAddress *object,
break;
/* Otherwise, treat this like an internal dependency */
- /* FALL THRU */
+ /* FALLTHRU */
case DEPENDENCY_INTERNAL:
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index a518b552b3..e30ac35365 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -12467,7 +12467,7 @@ ATExecChangeOwner(Oid relationOid, Oid newOwnerId, bool recursing, LOCKMODE lock
case RELKIND_TOASTVALUE:
if (recursing)
break;
- /* FALL THRU */
+ /* FALLTHRU */
default:
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index ed551ab73a..9c0a8d098d 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -3877,7 +3877,7 @@ AfterTriggerExecute(EState *estate,
trig_tuple_slot2))
elog(ERROR, "failed to fetch tuple2 for AFTER trigger");
}
- /* fall through */
+ /* FALLTHRU */
case AFTER_TRIGGER_FDW_REUSE:
/*
diff --git a/src/backend/executor/nodeHash.c b/src/backend/executor/nodeHash.c
index 5da13ada72..98656a1245 100644
--- a/src/backend/executor/nodeHash.c
+++ b/src/backend/executor/nodeHash.c
@@ -256,7 +256,7 @@ MultiExecParallelHash(HashState *node)
* way, wait for everyone to arrive here so we can proceed.
*/
BarrierArriveAndWait(build_barrier, WAIT_EVENT_HASH_BUILD_ALLOCATING);
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_BUILD_HASHING_INNER:
@@ -1181,13 +1181,13 @@ ExecParallelHashIncreaseNumBatches(HashJoinTable hashtable)
/* All other participants just flush their tuples to disk. */
ExecParallelHashCloseBatchAccessors(hashtable);
}
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_GROW_BATCHES_ALLOCATING:
/* Wait for the above to be finished. */
BarrierArriveAndWait(&pstate->grow_batches_barrier,
WAIT_EVENT_HASH_GROW_BATCHES_ALLOCATING);
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_GROW_BATCHES_REPARTITIONING:
/* Make sure that we have the current dimensions and buckets. */
@@ -1200,7 +1200,7 @@ ExecParallelHashIncreaseNumBatches(HashJoinTable hashtable)
/* Wait for the above to be finished. */
BarrierArriveAndWait(&pstate->grow_batches_barrier,
WAIT_EVENT_HASH_GROW_BATCHES_REPARTITIONING);
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_GROW_BATCHES_DECIDING:
@@ -1255,7 +1255,7 @@ ExecParallelHashIncreaseNumBatches(HashJoinTable hashtable)
dsa_free(hashtable->area, pstate->old_batches);
pstate->old_batches = InvalidDsaPointer;
}
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_GROW_BATCHES_FINISHING:
/* Wait for the above to complete. */
@@ -1533,13 +1533,13 @@ ExecParallelHashIncreaseNumBuckets(HashJoinTable hashtable)
/* Clear the flag. */
pstate->growth = PHJ_GROWTH_OK;
}
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_GROW_BUCKETS_ALLOCATING:
/* Wait for the above to complete. */
BarrierArriveAndWait(&pstate->grow_buckets_barrier,
WAIT_EVENT_HASH_GROW_BUCKETS_ALLOCATING);
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_GROW_BUCKETS_REINSERTING:
/* Reinsert all tuples into the hash table. */
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index cc8edacdd0..f5fd0c801a 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -340,7 +340,7 @@ ExecHashJoinImpl(PlanState *pstate, bool parallel)
else
node->hj_JoinState = HJ_NEED_NEW_OUTER;
- /* FALL THRU */
+ /* FALLTHRU*/
case HJ_NEED_NEW_OUTER:
@@ -413,7 +413,7 @@ ExecHashJoinImpl(PlanState *pstate, bool parallel)
/* OK, let's scan the bucket for matches */
node->hj_JoinState = HJ_SCAN_BUCKET;
- /* FALL THRU */
+ /* FALLTHRU*/
case HJ_SCAN_BUCKET:
@@ -1137,13 +1137,13 @@ ExecParallelHashJoinNewBatch(HashJoinState *hjstate)
if (BarrierArriveAndWait(batch_barrier,
WAIT_EVENT_HASH_BATCH_ELECTING))
ExecParallelHashTableAlloc(hashtable, batchno);
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_BATCH_ALLOCATING:
/* Wait for allocation to complete. */
BarrierArriveAndWait(batch_barrier,
WAIT_EVENT_HASH_BATCH_ALLOCATING);
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_BATCH_LOADING:
/* Start (or join in) loading tuples. */
@@ -1163,7 +1163,7 @@ ExecParallelHashJoinNewBatch(HashJoinState *hjstate)
sts_end_parallel_scan(inner_tuples);
BarrierArriveAndWait(batch_barrier,
WAIT_EVENT_HASH_BATCH_LOADING);
- /* Fall through. */
+ /* FALLTHRU */
case PHJ_BATCH_PROBING:
diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index d85cf7d93e..4368a97768 100644
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -69,7 +69,7 @@ ExecLimit(PlanState *pstate)
*/
recompute_limits(node);
- /* FALL THRU */
+ /* FALLTHRU */
case LIMIT_RESCAN:
@@ -216,7 +216,7 @@ ExecLimit(PlanState *pstate)
}
Assert(node->lstate == LIMIT_WINDOWEND_TIES);
- /* FALL THRU */
+ /* FALLTHRU */
case LIMIT_WINDOWEND_TIES:
if (ScanDirectionIsForward(direction))
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 02b6c3f127..77a5b22c9a 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2123,7 +2123,7 @@ pam_passwd_conv_proc(int num_msg, const struct pam_message **msg,
ereport(LOG,
(errmsg("error from underlying PAM layer: %s",
msg[i]->msg)));
- /* FALL THROUGH */
+ /* FALLTHRU */
case PAM_TEXT_INFO:
/* we don't bother to log TEXT_INFO messages */
if ((reply[i].resp = strdup("")) == NULL)
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index 0c6fe0115a..a16a2ff16b 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -1582,7 +1582,7 @@ find_nonnullable_rels_walker(Node *node, bool top_level)
* the intersection of the sets of nonnullable rels, just as
* for OR. Fall through to share code.
*/
- /* FALL THRU */
+ /* FALLTHRU */
case OR_EXPR:
/*
@@ -1807,7 +1807,7 @@ find_nonnullable_vars_walker(Node *node, bool top_level)
* the intersection of the sets of nonnullable vars, just as
* for OR. Fall through to share code.
*/
- /* FALL THRU */
+ /* FALLTHRU */
case OR_EXPR:
/*
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 75c122fe34..75997fd8a1 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -737,7 +737,7 @@ transformColumnDefinition(CreateStmtContext *cxt, ColumnDef *column)
errmsg("primary key constraints are not supported on foreign tables"),
parser_errposition(cxt->pstate,
constraint->location)));
- /* FALL THRU */
+ /* FALLTHRU */
case CONSTR_UNIQUE:
if (cxt->isforeign)
diff --git a/src/backend/partitioning/partprune.c b/src/backend/partitioning/partprune.c
index eac52e6ec8..e67575ab7a 100644
--- a/src/backend/partitioning/partprune.c
+++ b/src/backend/partitioning/partprune.c
@@ -2571,7 +2571,7 @@ get_matching_list_bounds(PartitionPruneContext *context,
case BTGreaterEqualStrategyNumber:
inclusive = true;
- /* fall through */
+ /* FALLTHRU */
case BTGreaterStrategyNumber:
off = partition_list_bsearch(partsupfunc,
partcollation,
@@ -2606,7 +2606,7 @@ get_matching_list_bounds(PartitionPruneContext *context,
case BTLessEqualStrategyNumber:
inclusive = true;
- /* fall through */
+ /* FALLTHRU */
case BTLessStrategyNumber:
off = partition_list_bsearch(partsupfunc,
partcollation,
@@ -2853,7 +2853,7 @@ get_matching_range_bounds(PartitionPruneContext *context,
case BTGreaterEqualStrategyNumber:
inclusive = true;
- /* fall through */
+ /* FALLTHRU */
case BTGreaterStrategyNumber:
/*
@@ -2934,7 +2934,7 @@ get_matching_range_bounds(PartitionPruneContext *context,
case BTLessEqualStrategyNumber:
inclusive = true;
- /* fall through */
+ /* FALLTHRU */
case BTLessStrategyNumber:
/*
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index e19d5dc1a6..bbe33b05bb 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -5902,19 +5902,19 @@ bgworker_should_start_now(BgWorkerStartTime start_time)
case PM_RUN:
if (start_time == BgWorkerStart_RecoveryFinished)
return true;
- /* fall through */
+ /* FALLTHRU */
case PM_HOT_STANDBY:
if (start_time == BgWorkerStart_ConsistentState)
return true;
- /* fall through */
+ /* FALLTHRU */
case PM_RECOVERY:
case PM_STARTUP:
case PM_INIT:
if (start_time == BgWorkerStart_PostmasterStart)
return true;
- /* fall through */
+ /* FALLTHRU */
}
diff --git a/src/backend/regex/regc_pg_locale.c b/src/backend/regex/regc_pg_locale.c
index 3cc2d4d362..1942bfa1bc 100644
--- a/src/backend/regex/regc_pg_locale.c
+++ b/src/backend/regex/regc_pg_locale.c
@@ -303,7 +303,7 @@ pg_wc_isdigit(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswdigit((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
isdigit((unsigned char) c));
@@ -312,7 +312,7 @@ pg_wc_isdigit(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswdigit_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -339,7 +339,7 @@ pg_wc_isalpha(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswalpha((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
isalpha((unsigned char) c));
@@ -348,7 +348,7 @@ pg_wc_isalpha(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswalpha_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -375,7 +375,7 @@ pg_wc_isalnum(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswalnum((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
isalnum((unsigned char) c));
@@ -384,7 +384,7 @@ pg_wc_isalnum(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswalnum_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -411,7 +411,7 @@ pg_wc_isupper(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswupper((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
isupper((unsigned char) c));
@@ -420,7 +420,7 @@ pg_wc_isupper(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswupper_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -447,7 +447,7 @@ pg_wc_islower(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswlower((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
islower((unsigned char) c));
@@ -456,7 +456,7 @@ pg_wc_islower(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswlower_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -483,7 +483,7 @@ pg_wc_isgraph(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswgraph((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
isgraph((unsigned char) c));
@@ -492,7 +492,7 @@ pg_wc_isgraph(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswgraph_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -519,7 +519,7 @@ pg_wc_isprint(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswprint((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
isprint((unsigned char) c));
@@ -528,7 +528,7 @@ pg_wc_isprint(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswprint_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -555,7 +555,7 @@ pg_wc_ispunct(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswpunct((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
ispunct((unsigned char) c));
@@ -564,7 +564,7 @@ pg_wc_ispunct(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswpunct_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -591,7 +591,7 @@ pg_wc_isspace(pg_wchar c)
case PG_REGEX_LOCALE_WIDE:
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswspace((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
return (c <= (pg_wchar) UCHAR_MAX &&
isspace((unsigned char) c));
@@ -600,7 +600,7 @@ pg_wc_isspace(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return iswspace_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
return (c <= (pg_wchar) UCHAR_MAX &&
@@ -631,7 +631,7 @@ pg_wc_toupper(pg_wchar c)
return pg_ascii_toupper((unsigned char) c);
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return towupper((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
/* force C behavior for ASCII characters, per comments above */
if (c <= (pg_wchar) 127)
@@ -644,7 +644,7 @@ pg_wc_toupper(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return towupper_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
if (c <= (pg_wchar) UCHAR_MAX)
@@ -675,7 +675,7 @@ pg_wc_tolower(pg_wchar c)
return pg_ascii_tolower((unsigned char) c);
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return towlower((wint_t) c);
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE:
/* force C behavior for ASCII characters, per comments above */
if (c <= (pg_wchar) 127)
@@ -688,7 +688,7 @@ pg_wc_tolower(pg_wchar c)
if (sizeof(wchar_t) >= 4 || c <= (pg_wchar) 0xFFFF)
return towlower_l((wint_t) c, pg_regex_locale->info.lt);
#endif
- /* FALL THRU */
+ /* FALLTHRU */
case PG_REGEX_LOCALE_1BYTE_L:
#ifdef HAVE_LOCALE_T
if (c <= (pg_wchar) UCHAR_MAX)
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.c
index 4594cf9509..8f6cb2b05c 100644
--- a/src/backend/replication/logical/reorderbuffer.c
+++ b/src/backend/replication/logical/reorderbuffer.c
@@ -1589,7 +1589,7 @@ ReorderBufferCommit(ReorderBuffer *rb, TransactionId xid,
change = specinsert;
change->action = REORDER_BUFFER_CHANGE_INSERT;
- /* intentionally fall through */
+ /* FALLTHRU */
case REORDER_BUFFER_CHANGE_INSERT:
case REORDER_BUFFER_CHANGE_UPDATE:
case REORDER_BUFFER_CHANGE_DELETE:
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index d69fb90132..8d579992ad 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -215,7 +215,7 @@ WalReceiverMain(void)
case WALRCV_STOPPING:
/* If we've already been requested to stop, don't start up. */
walrcv->walRcvState = WALRCV_STOPPED;
- /* fall through */
+ /* FALLTHRU */
case WALRCV_STOPPED:
SpinLockRelease(&walrcv->mutex);
diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c
index 4afad83539..d1809d7336 100644
--- a/src/backend/replication/walreceiverfuncs.c
+++ b/src/backend/replication/walreceiverfuncs.c
@@ -183,7 +183,7 @@ ShutdownWalRcv(void)
case WALRCV_WAITING:
case WALRCV_RESTARTING:
walrcv->walRcvState = WALRCV_STOPPING;
- /* fall through */
+ /* FALLTHRU */
case WALRCV_STOPPING:
walrcvpid = walrcv->pid;
break;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b1f7f6e2d0..858b0fab44 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1883,7 +1883,7 @@ ExecDropStmt(DropStmt *stmt, bool isTopLevel)
if (stmt->concurrent)
PreventInTransactionBlock(isTopLevel,
"DROP INDEX CONCURRENTLY");
- /* fall through */
+ /* FALLTHRU */
case OBJECT_TABLE:
case OBJECT_SEQUENCE:
diff --git a/src/backend/utils/adt/formatting.c b/src/backend/utils/adt/formatting.c
index aab5802edb..f179c6c47d 100644
--- a/src/backend/utils/adt/formatting.c
+++ b/src/backend/utils/adt/formatting.c
@@ -3425,7 +3425,7 @@ DCH_from_char(FormatNode *node, const char *in, TmFromChar *out,
case DCH_FF5:
case DCH_FF6:
out->ff = n->key->id - DCH_FF1 + 1;
- /* fall through */
+ /* FALLTHRU */
case DCH_US: /* microsecond */
len = from_char_parse_int_len(&out->us, &s,
n->key->id == DCH_US ? 6 :
diff --git a/src/backend/utils/adt/jsonb_util.c b/src/backend/utils/adt/jsonb_util.c
index 04b70c805b..2f14e69fec 100644
--- a/src/backend/utils/adt/jsonb_util.c
+++ b/src/backend/utils/adt/jsonb_util.c
@@ -637,7 +637,7 @@ pushJsonbValueScalar(JsonbParseState **pstate, JsonbIteratorToken seq,
break;
case WJB_END_OBJECT:
uniqueifyJsonbObject(&(*pstate)->contVal);
- /* fall through! */
+ /* FALLTHRU */
case WJB_END_ARRAY:
/* Steps here common to WJB_END_OBJECT case */
Assert(!scalarVal);
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 4caffb5804..bf01df1c09 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -3866,14 +3866,14 @@ timestamp_trunc(PG_FUNCTION_ARGS)
tm->tm_year = ((tm->tm_year + 999) / 1000) * 1000 - 999;
else
tm->tm_year = -((999 - (tm->tm_year - 1)) / 1000) * 1000 + 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_CENTURY:
/* see comments in timestamptz_trunc */
if (tm->tm_year > 0)
tm->tm_year = ((tm->tm_year + 99) / 100) * 100 - 99;
else
tm->tm_year = -((99 - (tm->tm_year - 1)) / 100) * 100 + 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_DECADE:
/* see comments in timestamptz_trunc */
if (val != DTK_MILLENNIUM && val != DTK_CENTURY)
@@ -3883,25 +3883,25 @@ timestamp_trunc(PG_FUNCTION_ARGS)
else
tm->tm_year = -((8 - (tm->tm_year - 1)) / 10) * 10;
}
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_YEAR:
tm->tm_mon = 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_QUARTER:
tm->tm_mon = (3 * ((tm->tm_mon - 1) / 3)) + 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_MONTH:
tm->tm_mday = 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_DAY:
tm->tm_hour = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_HOUR:
tm->tm_min = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_MINUTE:
tm->tm_sec = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_SECOND:
fsec = 0;
break;
@@ -4007,14 +4007,14 @@ timestamptz_trunc_internal(text *units, TimestampTz timestamp, pg_tz *tzp)
tm->tm_year = ((tm->tm_year + 999) / 1000) * 1000 - 999;
else
tm->tm_year = -((999 - (tm->tm_year - 1)) / 1000) * 1000 + 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_CENTURY:
/* truncating to the century? as above: -100, 1, 101... */
if (tm->tm_year > 0)
tm->tm_year = ((tm->tm_year + 99) / 100) * 100 - 99;
else
tm->tm_year = -((99 - (tm->tm_year - 1)) / 100) * 100 + 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_DECADE:
/*
@@ -4028,26 +4028,26 @@ timestamptz_trunc_internal(text *units, TimestampTz timestamp, pg_tz *tzp)
else
tm->tm_year = -((8 - (tm->tm_year - 1)) / 10) * 10;
}
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_YEAR:
tm->tm_mon = 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_QUARTER:
tm->tm_mon = (3 * ((tm->tm_mon - 1) / 3)) + 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_MONTH:
tm->tm_mday = 1;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_DAY:
tm->tm_hour = 0;
redotz = true; /* for all cases >= DAY */
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_HOUR:
tm->tm_min = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_MINUTE:
tm->tm_sec = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_SECOND:
fsec = 0;
break;
@@ -4195,33 +4195,33 @@ interval_trunc(PG_FUNCTION_ARGS)
case DTK_MILLENNIUM:
/* caution: C division may have negative remainder */
tm->tm_year = (tm->tm_year / 1000) * 1000;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_CENTURY:
/* caution: C division may have negative remainder */
tm->tm_year = (tm->tm_year / 100) * 100;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_DECADE:
/* caution: C division may have negative remainder */
tm->tm_year = (tm->tm_year / 10) * 10;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_YEAR:
tm->tm_mon = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_QUARTER:
tm->tm_mon = 3 * (tm->tm_mon / 3);
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_MONTH:
tm->tm_mday = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_DAY:
tm->tm_hour = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_HOUR:
tm->tm_min = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_MINUTE:
tm->tm_sec = 0;
- /* FALL THRU */
+ /* FALLTHRU */
case DTK_SECOND:
fsec = 0;
break;
diff --git a/src/backend/utils/adt/tsginidx.c b/src/backend/utils/adt/tsginidx.c
index 2d656168fc..fc39f1b432 100644
--- a/src/backend/utils/adt/tsginidx.c
+++ b/src/backend/utils/adt/tsginidx.c
@@ -264,7 +264,7 @@ TS_execute_ternary(GinChkVal *gcv, QueryItem *curitem, bool in_phrase)
/* Pass down in_phrase == true in case there's a NOT below */
in_phrase = true;
- /* FALL THRU */
+ /* FALLTHRU */
case OP_AND:
val1 = TS_execute_ternary(gcv, curitem + curitem->qoperator.left,
diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c
index 5b4b9e487f..bfe7608039 100644
--- a/src/backend/utils/hash/dynahash.c
+++ b/src/backend/utils/hash/dynahash.c
@@ -1042,7 +1042,7 @@ hash_search_with_hash_value(HTAB *hashp,
case HASH_ENTER_NULL:
/* ENTER_NULL does not work with palloc-based allocator */
Assert(hashp->alloc != DynaHashAlloc);
- /* FALL THRU */
+ /* FALLTHRU */
case HASH_ENTER:
/* Return existing element if found, else create one */
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index a8e13cacfd..0b114117cf 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -1263,7 +1263,7 @@ pg_utf8_increment(unsigned char *charptr, int length)
charptr[3]++;
break;
}
- /* FALL THRU */
+ /* FALLTHRU */
case 3:
a = charptr[2];
if (a < 0xBF)
@@ -1271,7 +1271,7 @@ pg_utf8_increment(unsigned char *charptr, int length)
charptr[2]++;
break;
}
- /* FALL THRU */
+ /* FALLTHRU */
case 2:
a = charptr[1];
switch (*charptr)
@@ -1291,7 +1291,7 @@ pg_utf8_increment(unsigned char *charptr, int length)
charptr[1]++;
break;
}
- /* FALL THRU */
+ /* FALLTHRU */
case 1:
a = *charptr;
if (a == 0x7F || a == 0xDF || a == 0xEF || a == 0xF4)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 5bdc02fce2..5330275dc9 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -7058,7 +7058,7 @@ set_config_option(const char *name, const char *value,
return 0;
}
/* fall through to process the same as PGC_BACKEND */
- /* FALLTHROUGH */
+ /* FALLTHRU */
case PGC_BACKEND:
if (context == PGC_SIGHUP)
{
@@ -8436,7 +8436,7 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
case VAR_SET_DEFAULT:
if (stmt->is_local)
WarnNoTransactionBlock(isTopLevel, "SET LOCAL");
- /* fall through */
+ /* FALLTHRU */
case VAR_RESET:
if (strcmp(stmt->name, "transaction_isolation") == 0)
WarnNoTransactionBlock(isTopLevel, "RESET TRANSACTION");
diff --git a/src/common/hashfn.c b/src/common/hashfn.c
index 990f18e610..e9a886184f 100644
--- a/src/common/hashfn.c
+++ b/src/common/hashfn.c
@@ -178,13 +178,13 @@ hash_bytes(const unsigned char *k, int keylen)
{
case 11:
c += ((uint32) k[10] << 8);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 24);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += ka[1];
@@ -192,22 +192,22 @@ hash_bytes(const unsigned char *k, int keylen)
break;
case 7:
b += ((uint32) k[6] << 8);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 16);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += ((uint32) k[4] << 24);
- /* fall through */
+ /* FALLTHRU */
case 4:
a += ka[0];
break;
case 3:
a += ((uint32) k[2] << 8);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 16);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += ((uint32) k[0] << 24);
/* case 0: nothing left to add */
@@ -217,13 +217,13 @@ hash_bytes(const unsigned char *k, int keylen)
{
case 11:
c += ((uint32) k[10] << 24);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 8);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += ka[1];
@@ -231,22 +231,22 @@ hash_bytes(const unsigned char *k, int keylen)
break;
case 7:
b += ((uint32) k[6] << 16);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 8);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += k[4];
- /* fall through */
+ /* FALLTHRU */
case 4:
a += ka[0];
break;
case 3:
a += ((uint32) k[2] << 16);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 8);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += k[0];
/* case 0: nothing left to add */
@@ -280,35 +280,35 @@ hash_bytes(const unsigned char *k, int keylen)
{
case 11:
c += ((uint32) k[10] << 8);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 24);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += k[7];
- /* fall through */
+ /* FALLTHRU */
case 7:
b += ((uint32) k[6] << 8);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 16);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += ((uint32) k[4] << 24);
- /* fall through */
+ /* FALLTHRU */
case 4:
a += k[3];
- /* fall through */
+ /* FALLTHRU */
case 3:
a += ((uint32) k[2] << 8);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 16);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += ((uint32) k[0] << 24);
/* case 0: nothing left to add */
@@ -318,35 +318,35 @@ hash_bytes(const unsigned char *k, int keylen)
{
case 11:
c += ((uint32) k[10] << 24);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 8);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += ((uint32) k[7] << 24);
- /* fall through */
+ /* FALLTHRU */
case 7:
b += ((uint32) k[6] << 16);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 8);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += k[4];
- /* fall through */
+ /* FALLTHRU */
case 4:
a += ((uint32) k[3] << 24);
- /* fall through */
+ /* FALLTHRU */
case 3:
a += ((uint32) k[2] << 16);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 8);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += k[0];
/* case 0: nothing left to add */
@@ -417,13 +417,13 @@ hash_bytes_extended(const unsigned char *k, int keylen, uint64 seed)
{
case 11:
c += ((uint32) k[10] << 8);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 24);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += ka[1];
@@ -431,22 +431,22 @@ hash_bytes_extended(const unsigned char *k, int keylen, uint64 seed)
break;
case 7:
b += ((uint32) k[6] << 8);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 16);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += ((uint32) k[4] << 24);
- /* fall through */
+ /* FALLTHRU */
case 4:
a += ka[0];
break;
case 3:
a += ((uint32) k[2] << 8);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 16);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += ((uint32) k[0] << 24);
/* case 0: nothing left to add */
@@ -456,13 +456,13 @@ hash_bytes_extended(const unsigned char *k, int keylen, uint64 seed)
{
case 11:
c += ((uint32) k[10] << 24);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 8);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += ka[1];
@@ -470,22 +470,22 @@ hash_bytes_extended(const unsigned char *k, int keylen, uint64 seed)
break;
case 7:
b += ((uint32) k[6] << 16);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 8);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += k[4];
- /* fall through */
+ /* FALLTHRU */
case 4:
a += ka[0];
break;
case 3:
a += ((uint32) k[2] << 16);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 8);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += k[0];
/* case 0: nothing left to add */
@@ -519,35 +519,35 @@ hash_bytes_extended(const unsigned char *k, int keylen, uint64 seed)
{
case 11:
c += ((uint32) k[10] << 8);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 24);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += k[7];
- /* fall through */
+ /* FALLTHRU */
case 7:
b += ((uint32) k[6] << 8);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 16);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += ((uint32) k[4] << 24);
- /* fall through */
+ /* FALLTHRU */
case 4:
a += k[3];
- /* fall through */
+ /* FALLTHRU */
case 3:
a += ((uint32) k[2] << 8);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 16);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += ((uint32) k[0] << 24);
/* case 0: nothing left to add */
@@ -557,35 +557,35 @@ hash_bytes_extended(const unsigned char *k, int keylen, uint64 seed)
{
case 11:
c += ((uint32) k[10] << 24);
- /* fall through */
+ /* FALLTHRU */
case 10:
c += ((uint32) k[9] << 16);
- /* fall through */
+ /* FALLTHRU */
case 9:
c += ((uint32) k[8] << 8);
- /* fall through */
+ /* FALLTHRU */
case 8:
/* the lowest byte of c is reserved for the length */
b += ((uint32) k[7] << 24);
- /* fall through */
+ /* FALLTHRU */
case 7:
b += ((uint32) k[6] << 16);
- /* fall through */
+ /* FALLTHRU */
case 6:
b += ((uint32) k[5] << 8);
- /* fall through */
+ /* FALLTHRU */
case 5:
b += k[4];
- /* fall through */
+ /* FALLTHRU */
case 4:
a += ((uint32) k[3] << 24);
- /* fall through */
+ /* FALLTHRU */
case 3:
a += ((uint32) k[2] << 16);
- /* fall through */
+ /* FALLTHRU */
case 2:
a += ((uint32) k[1] << 8);
- /* fall through */
+ /* FALLTHRU */
case 1:
a += k[0];
/* case 0: nothing left to add */
diff --git a/src/common/wchar.c b/src/common/wchar.c
index efaf1c155b..943b0adff8 100644
--- a/src/common/wchar.c
+++ b/src/common/wchar.c
@@ -1452,12 +1452,12 @@ pg_utf8_islegal(const unsigned char *source, int length)
a = source[3];
if (a < 0x80 || a > 0xBF)
return false;
- /* FALL THRU */
+ /* FALLTHRU */
case 3:
a = source[2];
if (a < 0x80 || a > 0xBF)
return false;
- /* FALL THRU */
+ /* FALLTHRU */
case 2:
a = source[1];
switch (*source)
@@ -1483,7 +1483,7 @@ pg_utf8_islegal(const unsigned char *source, int length)
return false;
break;
}
- /* FALL THRU */
+ /* FALLTHRU */
case 1:
a = *source;
if (a >= 0x80 && a < 0xC2)
diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c
index 48ba50bbce..31cdb02094 100644
--- a/src/interfaces/ecpg/pgtypeslib/interval.c
+++ b/src/interfaces/ecpg/pgtypeslib/interval.c
@@ -184,7 +184,7 @@ DecodeISO8601Interval(char *str,
continue;
}
/* Else fall through to extended alternative format */
- /* FALLTHROUGH */
+ /* FALLTHRU */
case '-': /* ISO 8601 4.4.3.3 Alternative Format,
* Extended */
if (havefield)
@@ -263,7 +263,7 @@ DecodeISO8601Interval(char *str,
return 0;
}
/* Else fall through to extended alternative format */
- /* FALLTHROUGH */
+ /* FALLTHRU */
case ':': /* ISO 8601 4.4.3.3 Alternative Format,
* Extended */
if (havefield)
@@ -391,7 +391,7 @@ DecodeInterval(char **field, int *ftype, int nf, /* int range, */
tmask = DTK_M(TZ);
break;
}
- /* FALL THROUGH */
+ /* FALLTHRU */
case DTK_DATE:
case DTK_NUMBER:
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index b455b45e96..f69d3c1776 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -375,7 +375,7 @@ retry_masked:
REMEMBER_EPIPE(spinfo, true);
#ifdef ECONNRESET
- /* FALL THRU */
+ /* FALLTHRU */
case ECONNRESET:
#endif
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c
index aeb6c8fefc..0af29b08ed 100644
--- a/src/pl/plpgsql/src/pl_exec.c
+++ b/src/pl/plpgsql/src/pl_exec.c
@@ -3186,7 +3186,7 @@ exec_stmt_return(PLpgSQL_execstate *estate, PLpgSQL_stmt_return *stmt)
/* fulfill promise if needed, then handle like regular var */
plpgsql_fulfill_promise(estate, (PLpgSQL_var *) retvar);
- /* FALL THRU */
+ /* FALLTHRU */
case PLPGSQL_DTYPE_VAR:
{
@@ -3332,7 +3332,7 @@ exec_stmt_return_next(PLpgSQL_execstate *estate,
/* fulfill promise if needed, then handle like regular var */
plpgsql_fulfill_promise(estate, (PLpgSQL_var *) retvar);
- /* FALL THRU */
+ /* FALLTHRU */
case PLPGSQL_DTYPE_VAR:
{
@@ -5423,7 +5423,7 @@ exec_eval_datum(PLpgSQL_execstate *estate,
/* fulfill promise if needed, then handle like regular var */
plpgsql_fulfill_promise(estate, (PLpgSQL_var *) datum);
- /* FALL THRU */
+ /* FALLTHRU */
case PLPGSQL_DTYPE_VAR:
{
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index 83a81033a5..161dbd58b4 100644
--- a/src/port/snprintf.c
+++ b/src/port/snprintf.c
@@ -464,7 +464,7 @@ nextch2:
/* set zero padding if no nonzero digits yet */
if (accum == 0 && !pointflag)
zpad = '0';
- /* FALL THRU */
+ /* FALLTHRU */
case '1':
case '2':
case '3':
diff --git a/src/timezone/zic.c b/src/timezone/zic.c
index c27fb456d0..bc54d5c610 100644
--- a/src/timezone/zic.c
+++ b/src/timezone/zic.c
@@ -1367,19 +1367,19 @@ gethms(char const *string, char const *errstring)
break;
case 8:
ok = '0' <= xr && xr <= '9';
- /* fallthrough */
+ /* FALLTHRU */
case 7:
ok &= ssx == '.';
if (ok && noise)
warning(_("fractional seconds rejected by"
" pre-2018 versions of zic"));
- /* fallthrough */
+ /* FALLTHRU */
case 5:
ok &= mmx == ':';
- /* fallthrough */
+ /* FALLTHRU */
case 3:
ok &= hhx == ':';
- /* fallthrough */
+ /* FALLTHRU */
case 1:
break;
}
On 5/11/20 7:29 AM, Julien Rouhaud wrote:
On Mon, May 11, 2020 at 6:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2020-May-06, Alvaro Herrera wrote:
This doesn't allow whitespace between "fall" and "through", which means
we generate 217 such warnings currently. Or we can just use
-Wimplicit-fallthrough=3, which does allow whitespace (among other
detritus).If we're OK with patching all those places, I volunteer to do so. Any
objections? Or I can keep it at level 3, which can be done with minimal
patching.If we're gonna do it at all, I think we should go for the level 4
warnings. Level 3's idea of a fallthrough comment is too liberal
for my tastes...Here's a patch that also takes care of cleaning all warning due to the
level 4 setting (at least the one I got with my other config options).
I went with "FALLTHRU" as this seemed more used.Note that timezone/zic.c would also have to be changed.
Since this is external code maybe we should leave that at level 3? I
think that should be doable via a Makefile override.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Mon, May 11, 2020 at 2:07 PM Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
On 5/11/20 7:29 AM, Julien Rouhaud wrote:
On Mon, May 11, 2020 at 6:47 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
On 2020-May-06, Alvaro Herrera wrote:
This doesn't allow whitespace between "fall" and "through", which means
we generate 217 such warnings currently. Or we can just use
-Wimplicit-fallthrough=3, which does allow whitespace (among other
detritus).If we're OK with patching all those places, I volunteer to do so. Any
objections? Or I can keep it at level 3, which can be done with minimal
patching.If we're gonna do it at all, I think we should go for the level 4
warnings. Level 3's idea of a fallthrough comment is too liberal
for my tastes...Here's a patch that also takes care of cleaning all warning due to the
level 4 setting (at least the one I got with my other config options).
I went with "FALLTHRU" as this seemed more used.Note that timezone/zic.c would also have to be changed.
Since this is external code maybe we should leave that at level 3? I
think that should be doable via a Makefile override.
Yes that was my concern. The best way I found to avoid changing zic.c
is something like that in src/timezone/Makefile, before the zic
target:
ifneq (,$(filter -Wimplicit-fallthrough=4,$(CFLAGS)))
CFLAGS := $(filter-out -Wimplicit-fallthrough=4,$(CFLAGS))
CFLAGS += '-Wimplicit-fallthrough=3'
endif
Julien Rouhaud <rjuju123@gmail.com> writes:
Note that timezone/zic.c would also have to be changed.
Why? It uses "fallthrough" which is a legal spelling per level 4.
regards, tom lane
On Mon, May 11, 2020 at 3:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
Note that timezone/zic.c would also have to be changed.
Why? It uses "fallthrough" which is a legal spelling per level 4.
GCC documentation mentions [ \t]*FALLTHR(OUGH|U)[ \t]* for level 4
(out of the view other alternatives), which AFAICT is case sensitive
(level 3 has fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?).
Julien Rouhaud <rjuju123@gmail.com> writes:
On Mon, May 11, 2020 at 3:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Why? It uses "fallthrough" which is a legal spelling per level 4.
GCC documentation mentions [ \t]*FALLTHR(OUGH|U)[ \t]* for level 4
(out of the view other alternatives), which AFAICT is case sensitive
(level 3 has fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?).
Oh, I'd missed that that was case sensitive. Ugh --- that seems
unreasonable. Maybe we'd better settle for level 3 after all;
I don't think there's much room to doubt the intentions of a
comment spelled that way.
regards, tom lane
On Mon, May 11, 2020 at 4:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
On Mon, May 11, 2020 at 3:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Why? It uses "fallthrough" which is a legal spelling per level 4.
GCC documentation mentions [ \t]*FALLTHR(OUGH|U)[ \t]* for level 4
(out of the view other alternatives), which AFAICT is case sensitive
(level 3 has fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?).Oh, I'd missed that that was case sensitive. Ugh --- that seems
unreasonable. Maybe we'd better settle for level 3 after all;
I don't think there's much room to doubt the intentions of a
comment spelled that way.
Agreed.
On 2020-May-11, Julien Rouhaud wrote:
On Mon, May 11, 2020 at 4:40 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Julien Rouhaud <rjuju123@gmail.com> writes:
On Mon, May 11, 2020 at 3:41 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Why? It uses "fallthrough" which is a legal spelling per level 4.
GCC documentation mentions [ \t]*FALLTHR(OUGH|U)[ \t]* for level 4
(out of the view other alternatives), which AFAICT is case sensitive
(level 3 has fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)?).Oh, I'd missed that that was case sensitive. Ugh --- that seems
unreasonable. Maybe we'd better settle for level 3 after all;
I don't think there's much room to doubt the intentions of a
comment spelled that way.Agreed.
Pushed, thanks.
I ended up using level 4 and dialling back to 3 for zic.c only
(different make trickery though). I also settled on FALLTHROUGH rather
than FALLTHRU because the latter seems ugly as a spelling to me. I'm
not a fan of the uppercase, but the alternative would be to add a - or
@s.
I get no warnings with this (gcc 8), but ccache seems to save warnings
in one run so that they can be thrown in a later one. I'm not sure what
to make of that, but ccache -d proved that beyond reasonable doubt and
ccache -clear got rid of the lot.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-May-12, Alvaro Herrera wrote:
I get no warnings with this (gcc 8), but ccache seems to save warnings
in one run so that they can be thrown in a later one. I'm not sure what
to make of that, but ccache -d proved that beyond reasonable doubt and
ccache -clear got rid of the lot.
Fixed one straggler in contrib, and while testing it I realized why
ccache doesn't pay attention to the changes I was doing in the file:
ccache compares the *preprocessed* version of the file and only if that
differs from the version that was cached last, ccache sends the new one
to the compiler; and of course these comments are not present in the
preprocessed version, so changing only the comment accomplishes nothing.
You have to touch one byte outside of any comments.
I bet this is going to bite someone ... maybe we'd be better off going
all the way to -Wimplicit-fallthrough=5 and use the
__attribute__((fallthrough)) stuff instead.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I ended up using level 4 and dialling back to 3 for zic.c only
(different make trickery though).
Meh ... if we're going to use level 4, I'm inclined to just change zic.c
to match. It's not like we're using the upstream code verbatim anyway.
We could easily add s/fallthrough/FALLTHROUGH/ to the conversion recipe.
I get no warnings with this (gcc 8), but ccache seems to save warnings
in one run so that they can be thrown in a later one. I'm not sure what
to make of that, but ccache -d proved that beyond reasonable doubt and
ccache -clear got rid of the lot.
Sounds like a ccache bug: maybe it's not accounting for different
fallthrough warning levels. ccache knows a *ton* about gcc options,
so I'd not be surprised if it's doing something special with this one.
regards, tom lane
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Fixed one straggler in contrib, and while testing it I realized why
ccache doesn't pay attention to the changes I was doing in the file:
ccache compares the *preprocessed* version of the file and only if that
differs from the version that was cached last, ccache sends the new one
to the compiler; and of course these comments are not present in the
preprocessed version, so changing only the comment accomplishes nothing.
You have to touch one byte outside of any comments.
Ugh. So the only way ccache could avoid this is to drop the
preprocessed-file comparison check if -Wimplicit-fallthrough is on.
Doesn't really sound like something we'd want to ask them to do.
I bet this is going to bite someone ... maybe we'd be better off going
all the way to -Wimplicit-fallthrough=5 and use the
__attribute__((fallthrough)) stuff instead.
I'm not really in favor of the __attribute__ solution --- seems too
gcc-specific. FALLTHROUGH-type comments are understood by other
sorts of tools besides gcc.
In practice, it doesn't seem like this'll be a huge problem once
we're past the initial fixup stage. We can revisit it later if
that prediction proves wrong, of course.
regards, tom lane
At Tue, 12 May 2020 17:12:51 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Fixed one straggler in contrib, and while testing it I realized why
ccache doesn't pay attention to the changes I was doing in the file:
ccache compares the *preprocessed* version of the file and only if that
differs from the version that was cached last, ccache sends the new one
to the compiler; and of course these comments are not present in the
preprocessed version, so changing only the comment accomplishes nothing.
You have to touch one byte outside of any comments.Ugh. So the only way ccache could avoid this is to drop the
preprocessed-file comparison check if -Wimplicit-fallthrough is on.
Doesn't really sound like something we'd want to ask them to do.I bet this is going to bite someone ... maybe we'd be better off going
all the way to -Wimplicit-fallthrough=5 and use the
__attribute__((fallthrough)) stuff instead.I'm not really in favor of the __attribute__ solution --- seems too
gcc-specific. FALLTHROUGH-type comments are understood by other
sorts of tools besides gcc.In practice, it doesn't seem like this'll be a huge problem once
we're past the initial fixup stage. We can revisit it later if
that prediction proves wrong, of course.
FWIW, I got a warning for jsonpath_gram.c.
jsonpath_gram.c:1026:16: warning: this statement may fall through [-Wimplicit-fallthrough=]
if (*++yyp != '\\')
^
jsonpath_gram.c:1029:11: note: here
default:
^~~~~~~
jsonpath_gram.c:1025
case '\\':
if (*++yyp != '\\')
goto do_not_strip_quotes;
/* Fall through. */
default:
It is generated code by bison.
$ bison --version
bison (GNU Bison) 3.0.4
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Wed, May 13, 2020 at 4:13 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:
At Tue, 12 May 2020 17:12:51 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Fixed one straggler in contrib, and while testing it I realized why
ccache doesn't pay attention to the changes I was doing in the file:
ccache compares the *preprocessed* version of the file and only if that
differs from the version that was cached last, ccache sends the new one
to the compiler; and of course these comments are not present in the
preprocessed version, so changing only the comment accomplishesnothing.
You have to touch one byte outside of any comments.
Ugh. So the only way ccache could avoid this is to drop the
preprocessed-file comparison check if -Wimplicit-fallthrough is on.
Doesn't really sound like something we'd want to ask them to do.I bet this is going to bite someone ... maybe we'd be better off going
all the way to -Wimplicit-fallthrough=5 and use the
__attribute__((fallthrough)) stuff instead.I'm not really in favor of the __attribute__ solution --- seems too
gcc-specific. FALLTHROUGH-type comments are understood by other
sorts of tools besides gcc.In practice, it doesn't seem like this'll be a huge problem once
we're past the initial fixup stage. We can revisit it later if
that prediction proves wrong, of course.FWIW, I got a warning for jsonpath_gram.c.
jsonpath_gram.c:1026:16: warning: this statement may fall through
[-Wimplicit-fallthrough=]
if (*++yyp != '\\')
^
jsonpath_gram.c:1029:11: note: here
default:
^~~~~~~jsonpath_gram.c:1025
case '\\':
if (*++yyp != '\\')
goto do_not_strip_quotes;
/* Fall through. */
default:It is generated code by bison.
$ bison --version
bison (GNU Bison) 3.0.4
I just found this just serval minutes ago. Upgrading your bison to the
latest
version (3.6) is ok. I'd like we have a better way to share this knowledge
through.
I spend ~30 minutes to troubleshooting this issue.
Best Regards
Andy Fan
At Wed, 13 May 2020 16:17:50 +0800, Andy Fan <zhihui.fan1213@gmail.com> wrote in
On Wed, May 13, 2020 at 4:13 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:jsonpath_gram.c:1026:16: warning: this statement may fall through
[-Wimplicit-fallthrough=]
if (*++yyp != '\\')
^
jsonpath_gram.c:1029:11: note: here
default:
...
It is generated code by bison.
$ bison --version
bison (GNU Bison) 3.0.4I just found this just serval minutes ago. Upgrading your bison to the
latest
version (3.6) is ok. I'd like we have a better way to share this knowledge
through.
I spend ~30 minutes to troubleshooting this issue.
Thanks. I'm happy to know that! But AFAICS 3.0.4 is the current
version of bison in AppStream and PowerTools of CentOS8...
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Wed, May 13, 2020 at 12:16 PM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
At Wed, 13 May 2020 16:17:50 +0800, Andy Fan <zhihui.fan1213@gmail.com> wrote in
On Wed, May 13, 2020 at 4:13 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:jsonpath_gram.c:1026:16: warning: this statement may fall through
[-Wimplicit-fallthrough=]
if (*++yyp != '\\')
^
jsonpath_gram.c:1029:11: note: here
default:...
It is generated code by bison.
$ bison --version
bison (GNU Bison) 3.0.4I just found this just serval minutes ago. Upgrading your bison to the
latest
version (3.6) is ok. I'd like we have a better way to share this knowledge
through.
I spend ~30 minutes to troubleshooting this issue.Thanks. I'm happy to know that! But AFAICS 3.0.4 is the current
version of bison in AppStream and PowerTools of CentOS8...
FTR I'm using bison 3.5 and I didn't hit any issue. However that may
be because of ccache, as mentioned by Alvaro.
Andy Fan <zhihui.fan1213@gmail.com> writes:
FWIW, I got a warning for jsonpath_gram.c.
Ugh. Confirmed here on Fedora 30 (bison 3.0.5).
I just found this just serval minutes ago. Upgrading your bison to the
latest version (3.6) is ok. I'd like we have a better way to share this
knowledge through. I spend ~30 minutes to troubleshooting this issue.
I fear that is going to mean that we revert this patch.
We are *NOT* moving the minimum bison requirement for this,
especially not to a bleeding-edge bison version.
Alternatively, it might work to go back down to warning level 3;
I see that the code in question has
/* Fall through. */
which I believe would work at the lower warning level. But that
raises the question of how far back bison generates code that
is clean --- and, again, I'm not willing to move the minimum
bison requirement. (On the other hand, if you have an old bison,
you likely also have an old gcc that doesn't know this warning
switch, so maybe it'd be all right in practice?)
regards, tom lane
On 2020-May-13, Tom Lane wrote:
Andy Fan <zhihui.fan1213@gmail.com> writes:
FWIW, I got a warning for jsonpath_gram.c.
Ugh. Confirmed here on Fedora 30 (bison 3.0.5).
Ugh.
I just found this just serval minutes ago. Upgrading your bison to the
latest version (3.6) is ok. I'd like we have a better way to share this
knowledge through. I spend ~30 minutes to troubleshooting this issue.I fear that is going to mean that we revert this patch.
Or we can filter-out the -Wimplicit-fallthrough, or change to level 3,
for bison-emitted files.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I fear that is going to mean that we revert this patch.
Or we can filter-out the -Wimplicit-fallthrough, or change to level 3,
for bison-emitted files.
Let's just go to level 3 overall (and revert the changes you made for
level 4 compliance --- they're more likely to cause back-patching
pain than do anything useful).
regards, tom lane
On 2020-May-13, Tom Lane wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
I fear that is going to mean that we revert this patch.
Or we can filter-out the -Wimplicit-fallthrough, or change to level 3,
for bison-emitted files.Let's just go to level 3 overall (and revert the changes you made for
level 4 compliance --- they're more likely to cause back-patching
pain than do anything useful).
Ok.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-May-13, Alvaro Herrera wrote:
On 2020-May-13, Tom Lane wrote:
Let's just go to level 3 overall (and revert the changes you made for
level 4 compliance --- they're more likely to cause back-patching
pain than do anything useful).Ok.
And done.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, May 13, 2020 at 10:02 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Andy Fan <zhihui.fan1213@gmail.com> writes:
FWIW, I got a warning for jsonpath_gram.c.
Ugh. Confirmed here on Fedora 30 (bison 3.0.5).
I just found this just serval minutes ago. Upgrading your bison to the
latest version (3.6) is ok. I'd like we have a better way to share this
knowledge through. I spend ~30 minutes to troubleshooting this issue.I fear that is going to mean that we revert this patch.
We are *NOT* moving the minimum bison requirement for this,
especially not to a bleeding-edge bison version.
Yes, I didn't mean revert the patch, but I was thinking moving the minimum
bison. But since down to the warning level 3 also resolved the issue,
looks it is a better way to do it.
(On the other hand, if you have an old bison,
you likely also have an old gcc that doesn't know this warning
switch, so maybe it'd be all right in practice?)
I just use an old bision and a newer gcc:( and I used "echo "COPT=-Wall
-Werror"
src/Makefile.custom" which is same as our cfbot system. Thank you all
for so quick
fix!
Best Regards
Andy Fan