Le 23 avril 2012 15:05, Alexander Kornienko <ale...@google.com> a écrit :
> 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]];
>
>
Hello,
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.
This is looking really great, it'll be nice to be able to document
intentional fallthroughs :)
-- Matthieu
> 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 <jedik...@belkadan.com> wrote:
>
>>
>> On Apr 21, 2012, at 20:40, Chandler Carruth wrote:
>>
>> > On Sat, Apr 21, 2012 at 5:10 PM, Richard Smith <rich...@metafoo.co.uk>
>> wrote:
>> >> On Sat, Apr 21, 2012 at 4:54 PM, Joe Groff <arc...@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@cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
>
> --
> Alexander Kornienko | Software Engineer | ale...@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
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
