public inbox for [email protected]help / color / mirror / Atom feed
Re: 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]> 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-21 11:34 Peter Eisentraut <[email protected]> parent: Jelte Fennema-Nio <[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-02-23 07:32 Peter Eisentraut <[email protected]> parent: Peter Eisentraut <[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-04-06 21:31 Tom Lane <[email protected]> parent: 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-04-07 14:31 Peter Eisentraut <[email protected]> parent: 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-04-07 14:35 Tom Lane <[email protected]> parent: Peter Eisentraut <[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