enable fallthrough warnings on clang
The warnings about fallthrough in switch statements currently only work
on gcc. It would be nice if they also worked on clang, and in the long
run other compilers.
gcc has a convention of annotation with comments, like /*fallthrough*/,
but that is not available in other compilers. The standard for this is
using attributes, like [[fallthrough]] in C23 and C++, and
__attribute__((fallthrough)) as extensions in gcc and clang. (MSVC
doesn't have anything like this in C mode, but it supports
[[fallthrough]] in C++, so there is a plausible path to get this into C
sometime as well.)
So my proposal is that we wrap the appropriate attribute into a
pg_fallthrough macro, and replace the current comments with that.
There is a bit of fiddliness in setting the right compiler warning
option. You need -Wimplicit-fallthrough=5 with gcc, but
-Wimplicit-fallthrough with clang, but you don't want the latter with
gcc, because that doesn't enforce the use of the attribute.
Also, I found some places where clang warns but gcc does not, so there
are patches to also fix those. (These appear to be bugs in gcc.)
Attachments:
0001-Remove-useless-fallthrough-annotation.patchtext/plain; charset=UTF-8; name=0001-Remove-useless-fallthrough-annotation.patchDownload+0-2
0002-Use-fallthrough-attribute-instead-of-comment.patchtext/plain; charset=UTF-8; name=0002-Use-fallthrough-attribute-instead-of-comment.patchDownload+281-269
0003-Enable-Wimplicit-fallthrough-option-for-clang.patchtext/plain; charset=UTF-8; name=0003-Enable-Wimplicit-fallthrough-option-for-clang.patchDownload+134-4
0004-Fix-additional-fallthrough-warnings-from-clang.patchtext/plain; charset=UTF-8; name=0004-Fix-additional-fallthrough-warnings-from-clang.patchDownload+8-3
0005-Fix-additional-fallthrough-warning.patchtext/plain; charset=UTF-8; name=0005-Fix-additional-fallthrough-warning.patchDownload+1-2
On Tue, 20 Jan 2026 at 12:16, Peter Eisentraut <peter@eisentraut.org> wrote:
So my proposal is that we wrap the appropriate attribute into a
pg_fallthrough macro, and replace the current comments with that.
All looks okay to me. The only thing that stood out is that it checks
for C++ with __cpp_attributes instead of __cplusplus. Is it really
worth using this more specific attribute? Given that we're already
requiring C++11 and afaict all C++11 compilers should support the
general notion of attributes.
On 20.01.26 16:03, Jelte Fennema-Nio wrote:
On Tue, 20 Jan 2026 at 12:16, Peter Eisentraut <peter@eisentraut.org> wrote:
So my proposal is that we wrap the appropriate attribute into a
pg_fallthrough macro, and replace the current comments with that.All looks okay to me. The only thing that stood out is that it checks
for C++ with __cpp_attributes instead of __cplusplus. Is it really
worth using this more specific attribute? Given that we're already
requiring C++11 and afaict all C++11 compilers should support the
general notion of attributes.
I agree. I will make that change.
On 21.01.26 12:34, Peter Eisentraut wrote:
On 20.01.26 16:03, Jelte Fennema-Nio wrote:
On Tue, 20 Jan 2026 at 12:16, Peter Eisentraut <peter@eisentraut.org>
wrote:So my proposal is that we wrap the appropriate attribute into a
pg_fallthrough macro, and replace the current comments with that.All looks okay to me. The only thing that stood out is that it checks
for C++ with __cpp_attributes instead of __cplusplus. Is it really
worth using this more specific attribute? Given that we're already
requiring C++11 and afaict all C++11 compilers should support the
general notion of attributes.I agree. I will make that change.
I have committed this patch set. I also added a test into the C++ module.