Hi Dan,
On Sat, Oct 20, 2018 at 9:22 PM Dan Carpenter dan.carpenter@oracle.com 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