Horton

Thread

Commits

  1. Enable -Wimplicit-fallthrough option for clang

  2. Fix additional fallthrough warning

  3. Fix additional fallthrough warnings from clang

  4. test_cplusplusext: Add C++ pg_fallthrough test case

  5. Use fallthrough attribute instead of comment

  6. Remove useless fallthrough annotation

  1. enable fallthrough warnings on clang

    Peter Eisentraut <peter@eisentraut.org> — 2026-01-20T11:16:31Z

    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.)

  2. Re: enable fallthrough warnings on clang

    Jelte Fennema-Nio <postgres@jeltef.nl> — 2026-01-20T15:03:59Z

    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.

  3. Re: enable fallthrough warnings on clang

    Peter Eisentraut <peter@eisentraut.org> — 2026-01-21T11:34:42Z

    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.

  4. Re: enable fallthrough warnings on clang

    Peter Eisentraut <peter@eisentraut.org> — 2026-02-23T07:32:34Z

    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.

  5. Re: enable fallthrough warnings on clang

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-04-06T21:31:49Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> I have committed this patch set.  I also added a test into the C++ module.

BF member ayu is failing said test:


ccache clang++-4.0 -std=gnu++11 -Wall -Wpointer-arith -Werror=vla -Wmissing-format-attribute -Wimplicit-fallthrough -Wformat-security -fno-strict-aliasing -fwrapv -g -O2 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -I. -I. -I../../../../src/include -D_GNU_SOURCE  -I/usr/include/libxml2     -c -o test_cplusplusext.o test_cplusplusext.cpp
test_cplusplusext.cpp:66:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
                case 2:
                ^
test_cplusplusext.cpp:66:3: note: insert '[[clang::fallthrough]];' to silence this warning
                case 2:
                ^
                [[clang::fallthrough]]; 
test_cplusplusext.cpp:66:3: note: insert 'break;' to avoid fall-through
                case 2:
                ^
                break; 
1 warning generated.


I don't know if it's worth catering to this extremely old
clang version ...

			regards, tom lane

  6. Re: enable fallthrough warnings on clang

    Peter Eisentraut <peter@eisentraut.org> — 2026-04-07T14:31:49Z

    On 06.04.26 23:31, Tom Lane wrote:
> Peter Eisentraut <peter@eisentraut.org> writes:
>> I have committed this patch set.  I also added a test into the C++ module.
> 
> BF member ayu is failing said test:
> 
> 
> ccache clang++-4.0 -std=gnu++11 -Wall -Wpointer-arith -Werror=vla -Wmissing-format-attribute -Wimplicit-fallthrough -Wformat-security -fno-strict-aliasing -fwrapv -g -O2 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -I. -I. -I../../../../src/include -D_GNU_SOURCE  -I/usr/include/libxml2     -c -o test_cplusplusext.o test_cplusplusext.cpp
> test_cplusplusext.cpp:66:3: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
>                  case 2:
>                  ^
> test_cplusplusext.cpp:66:3: note: insert '[[clang::fallthrough]];' to silence this warning
>                  case 2:
>                  ^
>                  [[clang::fallthrough]];
> test_cplusplusext.cpp:66:3: note: insert 'break;' to avoid fall-through
>                  case 2:
>                  ^
>                  break;
> 1 warning generated.
> 
> 
> I don't know if it's worth catering to this extremely old
> clang version ...

It appears that we could satisfy clang 6 through 9 with something like

#elif defined(__clang__)
#define pg_fallthrough [[clang::fallthrough]]
#else

Clang >=10 support the existing #elif __has_attribute(fallthrough) branch.

But AFAICT, clang 5 and older are completely broken in this regard, 
because they react to the presence of [[clang::fallthrough]] with a 
syntax error ("error: expected expression").  Not even clang's own 
example code[0] works.

[0]: 
https://releases.llvm.org/5.0.2/tools/clang/docs/AttributeReference.html#fallthrough-clang-fallthrough

Also, this only appears to affect C++.  In C mode, before clang 10, the 
warning option -Wimplicit-fallthrough doesn't appear to do anything.

  7. Re: enable fallthrough warnings on clang

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-04-07T14:35:27Z

    Peter Eisentraut <peter@eisentraut.org> writes:
> On 06.04.26 23:31, Tom Lane wrote:
>> BF member ayu is failing said test:
>> ...
>> I don't know if it's worth catering to this extremely old
>> clang version ...

> But AFAICT, clang 5 and older are completely broken in this regard, 
> because they react to the presence of [[clang::fallthrough]] with a 
> syntax error ("error: expected expression").  Not even clang's own 
> example code[0] works.

Hah, so we'd make it worse not better by adding
[[clang::fallthrough]].  Let's leave well enough alone then.

			regards, tom lane