static_assert, sfc, and clang issues

Morten Brørup mb at smartsharesystems.com
Wed Jan 17 10:37:21 CET 2024


> From: Stephen Hemminger [mailto:stephen at networkplumber.org]
> Sent: Tuesday, 16 January 2024 23.50
> 
> On Tue, 16 Jan 2024 23:14:36 +0100
> Morten Brørup <mb at smartsharesystems.com> wrote:
> 
> > > +1 for #2 just make it a block.
> >
> > I prefer that you implement the workaround in the RTE_BUILD_BUG_ON()
> macro, by surrounding it by "do { } while (0)", like this:
> >
> > #define RTE_BUILD_BUG_ON(condition) do { static_assert(!(condition),
> #condition); } while (0)
> >
> > And please mention the workaround reason, with the reference to the
> clang bug, in the source code comment describing the function.
> >
> > The godbolt link in the clang issue seems happy with this workaround.
> 
> In the patch series sent, already did that. Didn't need to do { }
> while(0) hack to make it work.

It works in the problematic file, but not generally.

Without the do/while, "RTE_BUILD_BUG_ON(condition);" will expand to two lines:

{ static_assert(...); }
;

... which may be problematic when used with if/else without curly braces.

However, I do admit that this problem is probably purely theoretical.

> Reference is in commit message.

Yes, but if someone only looks at the source code, the workaround looks strange and unnecessary.

Personally, I prefer not having to dig into commit logs when reviewing code. Maybe it's just me. I'll leave the decision to you.



More information about the dev mailing list