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

[Alexander Kornienko]

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.

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120423/da94bb63/attachment.html>