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

[Jordy Rose]

I like the idea of adopting a fallthrough notation in our sources. I would suggest naming it LLVM_FALLTHROUGH, though, in line with most of the rest of llvm/Support/Compiler.h.

The 6 bugs you found seem scary, though I guess they can't be awful or we'd have found them sooner. I'm not happy with the header changes, though -- the asserts are both a lot hairier this way.

Jordy


On May 26, 2012, at 15:30, Alexander Kornienko wrote:

> Hello llvm-commits, clang-commits,
> 
> 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 FALLTHROUGH [[clang::fallthrough]]
> #endif
> #endif
> 
> #ifndef FALLTHROUGH
> #define FALLTHROUGH do { } while (0)
> #endif
> 
> 
> -- 
> Best regards,
> Alexander Kornienko
> <fallthrough-bugs-clang.diff><fallthrough-bugs-llvm.diff>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits