On 2018-05-01 17:32:39 -0700, Andres Freund wrote:
> Hi,
>
> On 2018-05-01 23:35:18 +0000, Tom Lane wrote:
> > Clean up warnings from -Wimplicit-fallthrough.
> >
> > Recent gcc can warn about switch-case fall throughs that are not
> > explicitly labeled as intentional. This seems like a good thing,
> > so clean up the warnings exposed thereby by labeling all such
> > cases with comments that gcc will recognize.
> >
> > In files that already had one or more suitable comments, I generally
> > matched the existing style of those. Otherwise I went with
> > /* FALLTHROUGH */, which is one of the spellings approved at the
> > more-restrictive-than-default level -Wimplicit-fallthrough=4.
> > (At the default level you can also spell it /* FALL ?THRU */,
> > and it's not picky about case. What you can't do is include
> > additional text in the same comment, so some existing comments
> > containing versions of this aren't good enough.)
>
> There's also a few cases in the LLVM using code, I'll clean that up momentarily.
>
>
> > Testing with gcc 8.0.1 (Fedora 28's current version), I found that
> > I also had to put explicit "break"s after elog(ERROR) or ereport(ERROR);
> > apparently, for this purpose gcc doesn't recognize that those don't
> > return. That seems like possibly a gcc bug, but it's fine because
> > in most places we did that anyway; so this amounts to a visit from the
> > style police.
>
> I found one more oddity with the current committed state:
> /home/andres/src/postgresql/src/interfaces/libpq/fe-secure.c: In function ‘pqsecure_raw_write’:
> /home/andres/src/postgresql/src/interfaces/libpq/fe-secure.c:91:6: warning: this statement may fall through [-Wimplicit-fallthrough=]
> if (cond) \
> ^
> /home/andres/src/postgresql/src/interfaces/libpq/fe-secure.c:362:5: note: in expansion of macro ‘REMEMBER_EPIPE’
> REMEMBER_EPIPE(spinfo, true);
> ^~~~~~~~~~~~~~
> /home/andres/src/postgresql/src/interfaces/libpq/fe-secure.c:366:4: note: here
> case ECONNRESET:
> ^~~~
>
> It seems that gcc gets confused by the #ifdef ECONNRESET. Somehow adding
> a newline before cures it. But the nicer fix seems to be to move the
> FALL THRU into the ifdef, which even seems more correct when nitpicking
> in god mode.
>
> Unless you've a better idea I'll fix that together with the LLVM stuff.
Pushed that way.