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

[Alexander Kornienko]

Hi all,

I'm reposting this message, as it seems to be lost. It would be really nice
if someone could take a look at llvm changes and find out if they relate to
real bugs or not. I would also appreciate to hear opinions on the suggested
LLVM_FALLTHROUGH macro as a compiler-verifiable way to annotate
fall-throughs in switch statements.

Thank you!

I compiled llvm and clang with the recently added
-Wimplicit-fallthrough diagnostic
> 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 LLVM_FALLTHROUGH [[clang::fallthrough]]
> **#endif
> **#endif
> ** **#ifndef LLVM_FALLTHROUGH
> **#define LLVM_FALLTHROUGH do { } while (0)
> **#endif*



--
Regards,
Alexander Kornienko
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120604/94ec9f7d/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fallthrough-bugs-llvm.diff
Type: application/octet-stream
Size: 2402 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120604/94ec9f7d/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fallthrough-macro.diff
Type: application/octet-stream
Size: 1798 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120604/94ec9f7d/attachment-0001.obj>