[cfe-commits] [PATCH] Implicit fall-through between switch labels

[Alexander Kornienko]

Hello,

Thanks everyone for comments!

I'm sorry for being out of this thread for a while, but during this time we
discussed this once again on IRC and came to an idea to change the design
of this feature to be more clear and allow more fine-grained annotation of
execution paths which lead to fall-through to the next switch labels.

In current implementation the [[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 1:
    if (x)
      break;
    else if (y) {
      ...
      [[fallthrough]];  // annotated fall-through to case 2
    }
    else
      return 13;
  case 2:               // no warning here
    ...
    [[fallthrough]];    // annotated fall-through to case 3
  case 3:
    ...
}

So, in the new design [[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:
 * fallthrough attribute can only be attached to a null-statement;
 * [[fallthrough]]; annotation should be placed inside switch body;
 * it should be placed on an execution path between any statement inside
switch body and case/default label (this means there's fall-through to the
corresponding case/default label);
 * no statements should exist on an execution path between
[[fallthrough]];annotation and the next
case/default label.

The diagnostic wording was changed to be more common:
...
switch (n) {
  case 4:
    ;
  case 5:
}
...

test.cpp:19:5: warning: unannotated fall-through between switch labels; use
[[fallthrough]]; attribute to silence this warning [-Wimplicit-fallthrough]
    case 5:
    ^
test.cpp:18:9: note: fall-through from here; use [[fallthrough]]; attribute
to silence this warning [-Wimplicit-fallthrough]
        ;
        ^
        [[fallthrough]];

Known issues:
 * fix-it hint location has to be adjusted to the end of the statement
using lexer.

Please, review this patch.

On Sun, Apr 22, 2012 at 4:48 AM, Jordy Rose <jediknil at belkadan.com> wrote:

>
> On Apr 21, 2012, at 20:40, Chandler Carruth wrote:
>
> > On Sat, Apr 21, 2012 at 5:10 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
> >> On Sat, Apr 21, 2012 at 4:54 PM, Joe Groff <arcata at gmail.com> wrote:
> >>> For future-proofing's sake, does the standard provide any guidance for
> >>> naming nonstandardized attributes? Should the attribute be named
> >>> something like 'clang::fallthrough' instead of just 'fallthrough', in
> >>> case a future standard or other implementations provide for a similar
> >>> attribute with different behavior?
> >>>
> >> Yes, I think so. The attribute namespace mechanism was designed to
> allow such vendor extensions without creating problems for future
> standardization.
> >>
> > I would find this extremely unfortunate. The fallthrough check doesn't
> seem likely at all to be a vendor-specific extension. I can't imagine any
> possible standardized meaning for the attribute other than the use being
> proposed here. Forcing users to type 'clang::' in each place seems to
> reduce the clarity and readability of the construct with no real benefit.
> Is this really necessary? Can we not add a top-level attribute when it
> makes sense?
>
> I'm with Chandler. The namespacing syntax would be useful for something
> like analyzer_noreturn, but this is something other compiler vendors could
> implement as well. I know we're supposed to be careful about adding
> language extensions (should this discussion be on cfe-dev?) but this is
> hardly a vendor-specific warning.
>
> Jordy
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>



-- 
Alexander Kornienko | Software Engineer | alexfh at google.com | +49 151 221
77 957
Google Germany GmbH | Dienerstr. 12 | 80331 München

AG Hamburg, HRB 86891 | Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Katherine Stephens

Tax ID:- 48/725/00206
VAT ID:- DE813741370
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120423/871944f5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: switch-implicit-fallthrough.diff
Type: application/octet-stream
Size: 16674 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120423/871944f5/attachment.obj>