Hi llvm-commits,<div><br></div><div>Did anyone have a chance to look at these patches?<br><br><div class="gmail_quote">On Wed, May 30, 2012 at 2:28 PM, 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">Could anyone, please, look at the patches? Should I commit these? Thanks!<div class="HOEnZb"><div class="h5"><br><br><div class="gmail_quote">
On Tue, May 29, 2012 at 3:08 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">Hi,<div><br></div><div>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.</div>

<div><br></div><div>Please, take a look at these. Thanks!<div><div><br>
<br><div class="gmail_quote">On Sun, May 27, 2012 at 5:01 AM, Jordy Rose <span dir="ltr"><<a href="mailto:jediknil@belkadan.com" target="_blank">jediknil@belkadan.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


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.<br>
<br>
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.<br>
<br>
Jordy<br>
<div><div><br>
<br>
On May 26, 2012, at 15:30, Alexander Kornienko wrote:<br>
<br>
> Hello llvm-commits, clang-commits,<br>
><br>
> 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;<br>



><br>
> 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.<br>



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



> #ifdef __clang__<br>
> #if __has_feature(cxx_attributes) && __has_warning("-Wimplicit-fallthrough")<br>
> #define FALLTHROUGH [[clang::fallthrough]]<br>
> #endif<br>
> #endif<br>
><br>
> #ifndef FALLTHROUGH<br>
> #define FALLTHROUGH do { } while (0)<br>
> #endif<br>
><br>
><br>
> --<br>
> Best regards,<br>
> Alexander Kornienko</div></div></blockquote></div></div></div></div></blockquote><div> </div>-- <br>Best regards,<br><div><div><div>Alexander Kornienko</div>
</div></div></div><br><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div><div><font color="#666666"><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(213,15,37);border-right-color:rgb(213,15,37);border-bottom-color:rgb(213,15,37);border-left-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Alexander Kornienko |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(51,105,232);border-right-color:rgb(51,105,232);border-bottom-color:rgb(51,105,232);border-left-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span></font><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(0,153,57);border-right-color:rgb(0,153,57);border-bottom-color:rgb(0,153,57);border-left-color:rgb(0,153,57);padding-top:2px;margin-top:2px"><font color="#666666"> </font><a href="mailto:alexfh@google.com" style="color:rgb(17,85,204)" target="_blank">alexfh@google.com</a> |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(238,178,17);border-right-color:rgb(238,178,17);border-bottom-color:rgb(238,178,17);border-left-color:rgb(238,178,17);padding-top:2px;margin-top:2px"> <a value="+35315435283" style="color:rgb(17,85,204)">+49 151 221 77 957</a></span></div>
</div><div><font color="#666666"><span style="background-color:rgb(255,255,255);font-family:Arial,Verdana,sans-serif">Google Germany GmbH | </span><span style="background-color:rgb(255,255,255);font-family:Arial,Verdana,sans-serif">Dienerstr. 12 | </span><span style="background-color:rgb(255,255,255);font-family:Arial,Verdana,sans-serif">80331 München</span></font></div>
<br>
</div>