gcc -Wimplicit-fallthrough and pg_unreachable
I noticed in CI builds of PL/Java with PG 13 that -Wimplicit-fallthrough=3
in pg_config's CFLAGS was causing some switch case fallthrough warnings
after an elog(ERROR. [1]https://travis-ci.com/github/tada/pljava/jobs/447208803#L694
I added my own pg_unreachable() after the elog(ERROR, ...) calls [2]https://github.com/tada/pljava/blame/2c8d992/pljava-so/src/main/c/type/Type.c#L246
and that did away with the warnings.
But it looks odd, and shouldn't it be unnecessary if elog is supplying
its own pg_unreachable() in the elevel >= ERROR case? [3]https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/utils/elog.h;h=a16f925#l104
Has anyone else seen this behavior? Am I doing something that stops gcc
from recognizing ERROR as a compile-time constant? Is it just a gcc bug?
(Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0 seems to be the gcc version on the
Travis runner.
Regards,
-Chap
[1]: https://travis-ci.com/github/tada/pljava/jobs/447208803#L694
AFAICT, the #L694 doesn't really take you to line 694 in a Travis log
because they collapse lines so the browser will not see the anchor,
so one must click to expand the containing line range first. Irksome.
Copied below.
[2]: https://github.com/tada/pljava/blame/2c8d992/pljava-so/src/main/c/type/Type.c#L246
https://github.com/tada/pljava/blame/2c8d992/pljava-so/src/main/c/type/Type.c#L246
[3]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/utils/elog.h;h=a16f925#l104
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/include/utils/elog.h;h=a16f925#l104
In file included from /usr/include/postgresql/13/server/postgres.h:47:0,
from
/home/travis/build/tada/pljava/pljava-so/src/main/c/type/Type.c:13:
/usr/include/postgresql/13/server/utils/elog.h:125:5: warning: this
statement may fall through [-Wimplicit-fallthrough=]
do { \
^
/usr/include/postgresql/13/server/utils/elog.h:145:2: note: in expansion of
macro ‘ereport_domain’
ereport_domain(elevel, TEXTDOMAIN, __VA_ARGS__)
^~~~~~~~~~~~~~
/usr/include/postgresql/13/server/utils/elog.h:215:2: note: in expansion of
macro ‘ereport’
ereport(elevel, errmsg_internal(__VA_ARGS__))
^~~~~~~
/home/travis/build/tada/pljava/pljava-so/src/main/c/type/Type.c:256:3: note:
in expansion of macro ‘elog’
elog(ERROR, "COERCEVIAIO not implemented from (regtype) %d to %d",
^~~~
/home/travis/build/tada/pljava/pljava-so/src/main/c/type/Type.c:258:2: note:
here
case COERCION_PATH_ARRAYCOERCE:
^~~~
Chapman Flack <chap@anastigmatix.net> writes:
I noticed in CI builds of PL/Java with PG 13 that -Wimplicit-fallthrough=3
in pg_config's CFLAGS was causing some switch case fallthrough warnings
after an elog(ERROR. [1]
Yeah, I can replicate this here (gcc 8.3.1 on RHEL8). My recollection
is that we saw this when trialling -Wimplicit-fallthrough, and determined
that the hack used to teach the compiler that elog(ERROR) doesn't return
fails to prevent -Wimplicit-fallthrough warnings even though it does work
for other purposes. Using this test case:
int
foo(int p)
{
int x;
switch (p)
{
case 0:
x = 1;
break;
case 1:
elog(ERROR, "bogus");
break;
case 2:
x = 2;
break;
default:
x = 3;
}
return x;
}
I do not get a warning about x being possibly uninitialized (so it
knows elog(ERROR) doesn't return?), but without the "break" after
elog() I do get a fallthrough warning (so it doesn't know that?).
Seems like a minor gcc bug; no idea if anyone's complained to them.
I think we've found other weak spots in -Wimplicit-fallthrough's
coverage though, so it's not one of gcc's best areas.
As illustrated here, I'd just add a "break" rather than
"pg_unreachable()", but that's a matter of taste.
regards, tom lane