[LLVMdev] [llvm-commits] PROPOSAL: LLVM_FALLTHROUGH macro for intended fall-throughs between switch cases

[Jim Grosbach]

This is Chris' call to make, imo.

-j
On Aug 22, 2012, at 3:59 PM, Alexander Kornienko <alexfh at google.com> wrote:

> Ping.
> 
> On Thu, Aug 9, 2012 at 3:04 PM, Alexander Kornienko <alexfh at google.com> wrote:
> Ping.
> 
> 
> On Fri, Jul 27, 2012 at 3:42 AM, Alexander Kornienko <alexfh at google.com> wrote:
> On Fri, Jul 27, 2012 at 2:02 AM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Thu, Jul 26, 2012 at 4:24 PM, Jim Grosbach <grosbach at apple.com> wrote:
> 
> On Jul 26, 2012, at 4:07 PM, Chris Lattner <clattner at apple.com> wrote:
> 
> > <dropping llvm-commits>
> >
> > On Jul 2, 2012, at 9:59 AM, Alexander Kornienko wrote:
> >
> >> Hi llvmdev, llvm-commits,
> >>
> >> There was a discussion on this topic a while ago, and now I've decided to make a formal proposal and post it here.
> >
> > I missed the earlier discussion, so I'm sorry for chiming in late.
> >
> >> I propose to add the LLVM_FALLTHROUGH macro for specifying intended fall-through locations between switch cases.
> >
> > 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.
> >
> 
> 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.
> 
> 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.
> 
> 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?
> Two months ago it was like this:
> 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.
> 
> 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.
> 
> 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.
> 
> -- 
> Best regards,
> Alexander Kornienko
> 
> 
> -- 
> Best regards,
> Alexander Kornienko
> 
> 
> 
> 
> -- 
> Alexander Kornienko | Software Engineer | alexfh at google.com | +49 151 221 77 957
> Google Germany GmbH | Dienerstr. 12 | 80331 München
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120822/51dac086/attachment.html>