Hi Dan, On Sat, Oct 20, 2018 at 9:22 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Sat, Oct 20, 2018 at 04:42:07PM +0200, Miguel Ojeda wrote: > > Using an attribute is indeed better whenever possible. In C++17 it is > > an standard attribute and there have been proposals to include some of > > them for C as well since 2016 at least, e.g. the latest for > > fallthrough at: > > > > http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2268.pdf > > > > I have taken a quick look into supporting it (typing it here to save > > it on the mailing list :-), and we have: > > > > * gcc >= 7.1 supports -Wimplicit-fallthrough with > > __attribute__((fallthrough)), the comment syntax and the C++ > > [[fallthrough]] syntax. > > * gcc < 7.1 complains about empty declarations (it does not know > > about attributes for null statements) and also > > -Wdeclaration-after-statement. > > I'm not sure I understand about empty decalarations. The idea is that > we would do: That paragraph tried to explain that gcc < 7.1 did not know about __attribute__((fallthrough)); i.e. that everything was introduced in gcc 7.1. Anyway, the conclusion was a neuron misfiring of mine -- in my mind I was thinking clang supported the comment syntax (when I clearly wrote that it didn't). Never mind --- thanks for pointing it out! > > case 3: > frob(); > __fall_through(); > case 4: > frob(); > > #if GCC > 7.1 > #define __fall_through() __attribute__((fallthrough)) > #else > #define __fall_through() > #endif Yes, of course, that is what we do for other attributes -- actually in -next we have pending a better way for checking, using __has_attribute: #if __has_attribute(fallthrough) #define __fallthrough __attribute__((fallthrough)) #else #define __fallthrough #endif > > So long as GCC and static analysis tools understand about the attribute > that's enought to catch the bugs. No one cares what clang and icc do. Not so sure about that -- there are quite some people looking forward to building Linux with clang, even if only to have another compiler to check for warnings and to use the clang/llvm-related tools (and to write new ones). > We would just disable the fall through warnings on those until they are > updated to support the fallthrough attribute. You mean if they start supporting the warning but not the attribute? I don't think that would be likely (i.e. if clang enables the warning on C mode, they will have to introduce a way to suppress it; which should be the attribute (at least), since they typically like to be compatible with gcc and since they already support it in C++ >= 11), but everything is possible. > > We wouldn't update all the fall through comments until later, but going > forward people could use the __fall_through() macro if they want. Agreed. I will introduce it in the compiler-attributes tree -- should be there for -rc1 if no one complains. CC'ing all of you in the patch. Cheers, Miguel
