<div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 27, 2012 at 2:02 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div><div>On Thu, Jul 26, 2012 at 4:24 PM, Jim Grosbach <span dir="ltr"><<a href="mailto:grosbach@apple.com" target="_blank">grosbach@apple.com</a>></span> wrote:<br>



<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
On Jul 26, 2012, at 4:07 PM, Chris Lattner <<a href="mailto:clattner@apple.com" target="_blank">clattner@apple.com</a>> wrote:<br>
<br>
> <dropping llvm-commits><br>
><br>
> On Jul 2, 2012, at 9:59 AM, Alexander Kornienko wrote:<br>
><br>
>> Hi llvmdev, llvm-commits,<br>
>><br>
>> There was a discussion on this topic a while ago, and now I've decided to make a formal proposal and post it here.<br>
><br>
> I missed the earlier discussion, so I'm sorry for chiming in late.<br>
><br>
>> I propose to add the LLVM_FALLTHROUGH macro for specifying intended fall-through locations between switch cases.<br>
><br>
> I don't really see that the tradeoff here is worthwhile.  It is possible that we have some fallthrough bugs, but the cost of sprinkling this macro everywhere doesn't seem like the right tradeoff.<br>
><br>
<br>
</div>While I tend to agree with you, it's also true that for many (most?) of the locations where we have an intentional fall through, there's already a comment to that effect as a matter of style. This would simply formalize that currently voluntary bit of style and use the macro rather than a comment. Depending on how many bugs this would enable find (perhaps some experiments are in order?), I could be convinced the tradeoff is worthwhile.</blockquote>




<div><br></div></div></div><div>I believe Alex has already gone through the hits from this diagnostic; see the fallthrough-bugs-llvm.diff patch attached to the original mail on this thread for the bugs he found.</div><div>



<br></div><div>
Alex: can you tell us how many FALLTHROUGH annotations would be required, and how many bugs you found, when running this over LLVM and Clang?</div></div>
</blockquote></div>Two months ago it was like this:</div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><div class="gmail_extra"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

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></blockquote>

</div><div class="gmail_extra"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">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. </blockquote>

</div><div class="gmail_extra"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">I can't guarantee that all these 6 locations are real bugs, but they look very much like unintended fall-throughs.</blockquote>
</div></blockquote><div><br></div>The patch with fixes is attached, it's unchecked, not reviewed, and most probably out-of-sync with current HEAD. But it shows the idea of how these bugs look like, and that it's not always easy to spot them manually. The number of actual bugs I found (5 in LLVM + 1 in Clang) isn't overwhelming, but I'm sure that many of similar bugs never got to the repository, but took enough debugging time to be found.<br>
<div class="gmail_extra"><div><br></div><div>If the idea seems to be not bad in general, I can do all the comments to macro conversion routine, it should be totally about 400 single line changes, which doesn't seem to be a great problem for reviewers. As for me, I'm going to come up with a tool for mostly automatic transition.</div>
<div><br></div>-- <br>
</div><div class="gmail_extra">Best regards,</div><div class="gmail_extra">Alexander Kornienko</div>