<br><br><div class="gmail_quote">On Sun, May 27, 2012 at 12:30 AM, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello llvm-commits, clang-commits,<div><br></div><div>I compiled llvm and clang with the recently added <font face="courier new, monospace">-Wimplicit-fallthrough</font> 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 <font face="courier new, monospace">assert(false)</font> not followed by <font face="courier new, monospace">break;</font></div>

<div><div><br></div><div>From the remaining ~70 locations 6 look like real bugs. I've prepared two patches: for llvm and clang, which add <font face="courier new, monospace">break;</font> for all these locations. I've also removed two unnecessary fall-throughs in headers to reduce total amount of 'unannotated fall-through' warning messages. </div>

<div><br></div><div>I can't guarantee that all these 6 locations are real bugs, but they look very much like unintended fall-throughs.</div><div><br></div><div>Could you, please, review this patch?</div><div>Thanks!</div>

<div><br></div><div>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?</div><div>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:</div>

<div><pre><b>#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</b></pre></div><span class="HOEnZb"><font color="#888888"><div><br></div>-- <br>
<div>Best regards,</div><div>Alexander Kornienko</div></font></span></div>
<br></blockquote><div> </div><div>One question: in the proposed macro, why not put the the `;` in the macro since it will be mandatory anyway ?<br><br>Also, it might be easier to just do:<br><br>#if defined(__has_feature) and __has_feature(cxx_attributes)<br>
#  define FALLTHROUGH [[clang::fallthrough]];<br>#else<br>#  define FALLTHROUGH while(0) {}<br>#endif<br><br>-- Matthieu<br></div></div>