public inbox for [email protected]
help / color / mirror / Atom feedRe: enable fallthrough warnings on clang
6+ messages / 3 participants
[nested] [flat]
* Re: enable fallthrough warnings on clang
@ 2026-01-20 15:03 Jelte Fennema-Nio <[email protected]>
2026-01-21 11:34 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Jelte Fennema-Nio @ 2026-01-20 15:03 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers
On Tue, 20 Jan 2026 at 12:16, Peter Eisentraut <[email protected]> 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.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: enable fallthrough warnings on clang
2026-01-20 15:03 Re: enable fallthrough warnings on clang Jelte Fennema-Nio <[email protected]>
@ 2026-01-21 11:34 ` Peter Eisentraut <[email protected]>
2026-02-23 07:32 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Peter Eisentraut @ 2026-01-21 11:34 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: pgsql-hackers
On 20.01.26 16:03, Jelte Fennema-Nio wrote:
> On Tue, 20 Jan 2026 at 12:16, Peter Eisentraut <[email protected]> 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.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: enable fallthrough warnings on clang
2026-01-20 15:03 Re: enable fallthrough warnings on clang Jelte Fennema-Nio <[email protected]>
2026-01-21 11:34 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
@ 2026-02-23 07:32 ` Peter Eisentraut <[email protected]>
2026-04-06 21:31 ` Re: enable fallthrough warnings on clang Tom Lane <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Peter Eisentraut @ 2026-02-23 07:32 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: pgsql-hackers
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 <[email protected]>
>> 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.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: enable fallthrough warnings on clang
2026-01-20 15:03 Re: enable fallthrough warnings on clang Jelte Fennema-Nio <[email protected]>
2026-01-21 11:34 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
2026-02-23 07:32 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
@ 2026-04-06 21:31 ` Tom Lane <[email protected]>
2026-04-07 14:31 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Tom Lane @ 2026-04-06 21:31 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; pgsql-hackers
Peter Eisentraut <[email protected]> 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
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: enable fallthrough warnings on clang
2026-01-20 15:03 Re: enable fallthrough warnings on clang Jelte Fennema-Nio <[email protected]>
2026-01-21 11:34 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
2026-02-23 07:32 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
2026-04-06 21:31 ` Re: enable fallthrough warnings on clang Tom Lane <[email protected]>
@ 2026-04-07 14:31 ` Peter Eisentraut <[email protected]>
2026-04-07 14:35 ` Re: enable fallthrough warnings on clang Tom Lane <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Peter Eisentraut @ 2026-04-07 14:31 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On 06.04.26 23:31, Tom Lane wrote:
> Peter Eisentraut <[email protected]> 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-fallthrou...
Also, this only appears to affect C++. In C mode, before clang 10, the
warning option -Wimplicit-fallthrough doesn't appear to do anything.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: enable fallthrough warnings on clang
2026-01-20 15:03 Re: enable fallthrough warnings on clang Jelte Fennema-Nio <[email protected]>
2026-01-21 11:34 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
2026-02-23 07:32 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
2026-04-06 21:31 ` Re: enable fallthrough warnings on clang Tom Lane <[email protected]>
2026-04-07 14:31 ` Re: enable fallthrough warnings on clang Peter Eisentraut <[email protected]>
@ 2026-04-07 14:35 ` Tom Lane <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Tom Lane @ 2026-04-07 14:35 UTC (permalink / raw)
To: Peter Eisentraut <[email protected]>; +Cc: Jelte Fennema-Nio <[email protected]>; pgsql-hackers
Peter Eisentraut <[email protected]> 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
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-04-07 14:35 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-01-20 15:03 Re: enable fallthrough warnings on clang Jelte Fennema-Nio <[email protected]>
2026-01-21 11:34 ` Peter Eisentraut <[email protected]>
2026-02-23 07:32 ` Peter Eisentraut <[email protected]>
2026-04-06 21:31 ` Tom Lane <[email protected]>
2026-04-07 14:31 ` Peter Eisentraut <[email protected]>
2026-04-07 14:35 ` Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox