Re: pgsql: Clean up warnings from -Wimplicit-fallthrough.

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.