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

[Peter Cooper]

Hi Alexander
On Apr 23, 2012, at 10:49 AM, Alexander Kornienko <alexfh at google.com> wrote:

> Hi,
> 
> On Mon, Apr 23, 2012 at 7:10 PM, Matthieu Monrocq <matthieu.monrocq at gmail.com> wrote:
> 
> Le 23 avril 2012 15:05, Alexander Kornienko <alexfh at google.com> a écrit :
> ... 
> In current implementation the [[fallthrough]] attribute should be applied to a null statement ...
> Just a tiny remark on the wording of the diagnostics: I would avoid using a `;` after `[[fallthrough]]`. The attribute is `fallthrough`, the syntax for attributes leads to `[[fallthrough]]`, but there is no `;` in general. And similarly for the fix-it hint on the note when the `;` is already present.
> 
> Thanks for the comment, but you may have missed one of my points: ';' is a null statement, to which the [[fallthrough]] attribute has to be attached. So this is neither redundant nor optional. And the wording of diagnostic messages is correct in regard to this. Having this null statement is probably the most reasonable way to mimic control-flow statements like return or break. It allows this attribute to be placed in any position where any other statement could be - just by requiring it to be attached to a statement. And an empty statement doesn't affect semantics of code, so it is a reasonable choice.
Can you add checks for the null statement being unreachable?  Normally null statements never warn if they are unreachable but in the case where its really your attribute this is a very useful warning to have.

Thanks,
Pete
> 
> Does it make sense?
>  
> This is looking really great, it'll be nice to be able to document intentional fallthroughs :)
> 
> Yep, llvm code has hundreds of intentional fall-through locations marked (or not marked) with comments.
>  
> Known issues:
>  * fix-it hint location has to be adjusted to the end of the statement using lexer.
> 
> More known issues:
>  * currently this code doesn't try to minimize or somehow optimize the number of fix-it hints, it just offers them in all execution paths containing fall-through after the last executable statement - this provides fine-grained diagnostic, but probably too verbose;
>  * fix-it hints do not take in account necessity of adding a curly braces when fallthrough annotation is suggested in a place where only one statement is allowed:
>     if(x) ...; [[fallthrough]]; else return 13;
>   here fix-it hint should have suggested something like this:
>     if(x) { ...; [[fallthrough]]; } else return 13;
> 
> -- 
> Best regards,
> Alexander Kornienko
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120423/0352d43a/attachment.html>