[llvm-commits] [cfe-commits] [PATCH] Fixes for unintended fall-through bugs (-Wimplicit-fallthrough)

[Matthieu Monrocq]

On Sun, May 27, 2012 at 12:30 AM, Alexander Kornienko <alexfh at google.com>wrote:

> Hello llvm-commits, clang-commits,
>
> I compiled llvm and clang with the recently added -Wimplicit-fallthroughdiagnostic and analyzed the results. For those who are curious: there are
> about 400 'unannotated fall-through' warnings in total, about 290
> fall-through locations are annotated in comments, about 30 locations
> fall-through to an empty block (case 1: ....<no break>  case 2: break;), 7
> locations have assert(false) not followed by break;
>
> From the remaining ~70 locations 6 look like real bugs. I've prepared two
> patches: for llvm and clang, which add break; for all these locations.
> I've also removed two unnecessary fall-throughs in headers to reduce total
> amount of 'unannotated fall-through' warning messages.
>
> I can't guarantee that all these 6 locations are real bugs, but they look
> very much like unintended fall-throughs.
>
> Could you, please, review this patch?
> Thanks!
>
> P.S. It would also be interesting to know what people think about
> converting fall-through annotations in comments to a checked annotation so
> we could use this diagnostic on regular basis?
> We could wrap [[clang::fallthrough]] to a macro, so the code would
> continue to work in c++98 mode, but in c++11 mode it would also allow to
> run this diagnostic (btw, is llvm compilable in c++11 mode?). Sample
> implementation of macro:
>
> *#ifdef __clang__
> #if __has_feature(cxx_attributes) && __has_warning("-Wimplicit-fallthrough")
> #define FALLTHROUGH [[clang::fallthrough]]
> #endif
> #endif
>
> #ifndef FALLTHROUGH
> #define FALLTHROUGH do { } while (0)
> #endif*
>
>
> --
> Best regards,
> Alexander Kornienko
>
>
One question: in the proposed macro, why not put the the `;` in the macro
since it will be mandatory anyway ?

Also, it might be easier to just do:

#if defined(__has_feature) and __has_feature(cxx_attributes)
#  define FALLTHROUGH [[clang::fallthrough]];
#else
#  define FALLTHROUGH while(0) {}
#endif

-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120527/ec5a69e1/attachment.html>