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

[Alexander Kornienko]

On Thu, Jul 26, 2012 at 8:53 PM, Cameron McInally
<cameron.mcinally at nyu.edu>wrote:

> Hey Alex,
>
> Sorry if this is a silly question... are you asking if anyone "wants the
> functionality proposed" or "wants to write the code for the functionality
> proposed"?
>
*-Wimplicit-fallthrough* diagnostic is already implemented, and the patch
in this thread is a portability macro for
*[[clang::fallthrough]]*attribute (which is also implemented, of
course). I only need an approval
from community to add this macro and replace comment-only fall-through
annotations with the *LLVM_FALLTHROUGH;* macro.


> -Cameron
>
> On Thu, Jul 26, 2012 at 2:17 PM, Alexander Kornienko <alexfh at google.com>wrote:
>
>> Is anyone interested in introducing fall-through annotations and the
>> related diagnostic in LLVM/Clang code?
>>
>>
>> On Mon, Jul 2, 2012 at 6:59 PM, Alexander Kornienko <alexfh at google.com>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 propose to add the LLVM_FALLTHROUGH macro for specifying intended
>>> fall-through locations between switch cases.
>>>
>>> *INTRODUCTION*
>>>
>>> The switch construct of C/C++ languages allows fall-throughs between
>>> switch labels when control flow is not directed elsewhere by a break,
>>> return or continue statement or other ways. There are certain coding
>>> idioms which utilize this behavior, e.g. Duff's device<http://en.wikipedia.org/wiki/Duff's_device>,
>>> but much more frequently the use of switch statements doesn't involve a
>>> fall-through. (A commonly used case - when several switch labels mark one
>>> statement - is not considered a fall-through here.) As there's no special
>>> way to mark a fall-through, it's relatively easy to make a mistake by just
>>> missing a break/return statement before the next switch label. This kind of
>>> mistake is sometimes difficult to spot by manual code examination, and the
>>> compiler can't provide any help either. It's common to mark fall-through
>>> locations with a comment, but parsing comments would incur a significant
>>> performance penalty and is probably an unhealthy approach overall. So the
>>> use of comment-only annotations is limited to manual inspections.
>>>
>>> Some time ago I've added the 'Unintended fall-through in switch
>>> statement' diagnostics to clang (
>>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120430/057180.html
>>> ).
>>>
>>> This is a proposal to introduce clang's unintended fall-through
>>> diagnostic to llvm/clang code.
>>>
>>> *DESCRIPTION OF 'UNINTENDED FALL-THROUGH IN SWITCH STATEMENTS'
>>> DIAGNOSTIC IN CLANG*
>>>
>>> Related functionality in clang introduces several diagnostic messages
>>> and a syntax for intended fall-through annotation, based on C++11
>>> attributes ([dcl.attr],
>>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3376.pdf, 7.6).
>>>
>>> The [[clang::fallthrough]] attribute should be applied to a null
>>> statement placed in a point of execution where fall-through to the next
>>> switch label occurs, e.g.:
>>>
>>> switch (n) {
>>>   case 0:
>>>     ...  // no break here
>>>   case 1:  // warning: unannotated fall-through between switch labels
>>>     if (x)
>>>       break;
>>>     else if (y) {
>>>       ...
>>>       *[[clang::fallthrough]];*  // annotated fall-through to case 2
>>>     }
>>>     else
>>>       return 13;
>>>   case 2:  // no warning here
>>>     ...
>>>     *[[clang::fallthrough]];*  // annotated fall-through to case 3
>>>   case 3:
>>>     ...
>>> }
>>>
>>> So, the [[clang::fallthrough]]; annotation can be used in mostly in the
>>> same way as break; or return; statements (but it doesn't change
>>> control-flow, it just annotates a fall-through). The following rules are
>>> checked for this annotation:
>>>   * the clang::fallthrough attribute can only be attached to a
>>> null-statement;
>>>   * the [[clang::fallthrough]]; annotation should be placed inside a
>>> switch body;
>>>   * it should be placed on an execution path between any statement
>>> inside a switch body and a case/default label (this means there *is* a
>>> fall-through to the corresponding case/default label);
>>>   * no statements should exist on an execution path between the
>>> [[clang::fallthrough]]; annotation and the next case/default label.
>>>
>>> *PROPOSAL FOR LLVM/CLANG CODE*
>>>
>>> I ran the unintended fall-through diagnostics against llvm/clang code
>>> and found a number of fall-through locations, most of them are probably
>>> intended. But some look like bugs (see the attached *
>>> fallthrough-bugs-llvm.diff* file).
>>>
>>> To start using the proposed diagnostics code has to be prepared.
>>>
>>> First, we need to mark all intended fall-through locations with the
>>> clang::fallthrough attribute. It makes sense to 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 the
>>> macro (see *fallthrough-macro.diff*):
>>> *#ifdef __clang__
>>> **#if __has_feature(cxx_attributes) &&
>>> __has_warning("-Wimplicit-fallthrough")
>>> *
>>> *#define LLVM_FALLTHROUGH [[clang::fallthrough]]
>>> **#endif
>>> **#endif
>>> ****#ifndef LLVM_FALLTHROUGH
>>> **#define LLVM_FALLTHROUGH do { } while (0)
>>> **#endif*
>>>
>>> After this we can start using *-Wimplicit-fallthrough*.
>>>
>>> Please express your opinions on this proposal.
>>>
>>> I'm also ready to provide diffs for marking all fall-through locations
>>> with the LLVM_FALLTHROUGH macro, if this proposal gets approval.
>>>
>>> --
>>> Best regards,
>>> Alexander Kornienko
>>>
>>
-- 
Best regards,
Alexander Kornienko
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120727/a1e49c93/attachment.html>