Hi, I've put aside header changes for now. There are patches with bug fixes (at least, those seem to be bugs to me) and a patch with LLVM_FALLTHROUGH macro.
Please, take a look at these. Thanks! On Sun, May 27, 2012 at 5:01 AM, Jordy Rose <[email protected]> wrote: > 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 > -- Best regards, Alexander Kornienko
fallthrough-bugs-clang.diff
Description: Binary data
fallthrough-bugs-llvm.diff
Description: Binary data
fallthrough-macro.diff
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
