<div class="gmail_extra"><br><br><div class="gmail_quote">Le 23 avril 2012 15:05, Alexander Kornienko <span dir="ltr"><<a href="mailto:alexfh@google.com" target="_blank">alexfh@google.com</a>></span> a écrit :<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hello,<div><br></div><div>Thanks everyone for comments!</div><div><br></div><div>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.<br>
<div><br></div><div>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.:</div><div><br></div>
<div><font face="courier new, monospace">switch (n) {</font></div><div><font face="courier new, monospace"> case 1:</font></div><div><font face="courier new, monospace"> if (x)</font></div><div><font face="courier new, monospace"> break;</font></div>
<div><font face="courier new, monospace"> else if (y) {</font></div><div><font face="courier new, monospace"> ...</font></div><div><font face="courier new, monospace"> [[fallthrough]]; // annotated fall-through to case 2</font></div>
<div><font face="courier new, monospace"> }</font></div><div><font face="courier new, monospace"> else</font></div><div><font face="courier new, monospace"> return 13;</font></div><div><font face="courier new, monospace"> case 2: // no warning here</font></div>
<div><font face="courier new, monospace"> ...</font></div><div><font face="courier new, monospace"> [[fallthrough]]; // annotated fall-through to case 3</font></div><div><font face="courier new, monospace"> case 3:</font></div>
<div><font face="courier new, monospace"> ...</font></div><div><font face="courier new, monospace">}</font></div><div><br></div><div>So, in the new design <font face="courier new, monospace">[[fallthrough]];</font> annotation can be used in mostly in the same way as <font face="courier new, monospace">break;</font> or <font face="courier new, monospace">return;</font> statements (but it doesn't change control-flow, it just annotates a fall-through). The following rules are checked for this annotation:</div>
<div> * <font face="courier new, monospace">fallthrough</font> attribute can only be attached to a null-statement;</div><div> * <font face="courier new, monospace">[[fallthrough]];</font> annotation should be placed inside <font face="courier new, monospace">switch</font> body;</div>
<div> * it should be placed on an execution path between any statement inside <font face="courier new, monospace">switch</font> body and <font face="courier new, monospace">case/default</font> label (this means there's fall-through to the corresponding <font face="courier new, monospace">case/default</font> label);</div>
<div> * no statements should exist on an execution path between <font face="courier new, monospace">[[fallthrough]];</font> annotation and the next <font face="courier new, monospace">case</font>/<font face="courier new, monospace">default</font> label.</div>
<div><br></div><div>The diagnostic wording was changed to be more common:</div><div><font face="courier new, monospace">...</font></div><div><span style="font-family:'courier new',monospace">switch (n) {</span></div>
<div><div><font face="courier new, monospace"> case 4:</font></div><div><font face="courier new, monospace"> </font><span style="font-family:'courier new',monospace">;</span></div><div><font face="courier new, monospace"> case 5:</font></div>
<div><span style="font-family:'courier new',monospace">}</span></div><div><font face="courier new, monospace">...</font></div><div><font face="courier new, monospace"><br></font></div><div><font face="courier new, monospace">test.cpp:19:5: warning: unannotated fall-through between switch labels; use [[fallthrough]]; attribute to silence this warning [-Wimplicit-fallthrough]</font></div>
<div><font face="courier new, monospace"> case 5:</font></div><div><font face="courier new, monospace"> ^</font></div><div><font face="courier new, monospace">test.cpp:18:9: note: fall-through from here; use [[fallthrough]]; attribute to silence this warning [-Wimplicit-fallthrough]</font></div>
<div><font face="courier new, monospace"> ;</font></div><div><font face="courier new, monospace"> ^</font></div><div><font face="courier new, monospace"> [[fallthrough]];</font></div></div><div><br></div>
</div></blockquote><div><br>Hello,<br><br>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.<br>
<br>This is looking really great, it'll be nice to be able to document intentional fallthroughs :)<br><br>-- Matthieu<br> </div><blockquote class="gmail_quote" style="margin:0pt 0pt 0pt 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div></div>
<div>Known issues:</div><div> * fix-it hint location has to be adjusted to the end of the statement using lexer.</div><div><br></div><div>Please, review this patch.</div><div><br><div class="gmail_quote">On Sun, Apr 22, 2012 at 4:48 AM, Jordy Rose <span dir="ltr"><<a href="mailto:jediknil@belkadan.com" target="_blank">jediknil@belkadan.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br>
On Apr 21, 2012, at 20:40, Chandler Carruth wrote:<br>
<br>
> On Sat, Apr 21, 2012 at 5:10 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:<br>
>> On Sat, Apr 21, 2012 at 4:54 PM, Joe Groff <<a href="mailto:arcata@gmail.com" target="_blank">arcata@gmail.com</a>> wrote:<br>
</div><div>>>> For future-proofing's sake, does the standard provide any guidance for<br>
>>> naming nonstandardized attributes? Should the attribute be named<br>
>>> something like 'clang::fallthrough' instead of just 'fallthrough', in<br>
>>> case a future standard or other implementations provide for a similar<br>
>>> attribute with different behavior?<br>
>>><br>
>> Yes, I think so. The attribute namespace mechanism was designed to allow such vendor extensions without creating problems for future standardization.<br>
>><br>
> 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?<br>
<br>
</div>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.<br>
<span><font color="#888888"><br>
Jordy<br>
</font></span><div><div><br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><span class="HOEnZb"><font color="#888888"><br>
</font></span></div></div></blockquote></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div><div><font color="#666666"><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(213,15,37);border-right-color:rgb(213,15,37);border-bottom-color:rgb(213,15,37);border-left-color:rgb(213,15,37);padding-top:2px;margin-top:2px">Alexander Kornienko |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(51,105,232);border-right-color:rgb(51,105,232);border-bottom-color:rgb(51,105,232);border-left-color:rgb(51,105,232);padding-top:2px;margin-top:2px"> Software Engineer |</span></font><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(0,153,57);border-right-color:rgb(0,153,57);border-bottom-color:rgb(0,153,57);border-left-color:rgb(0,153,57);padding-top:2px;margin-top:2px"><font color="#666666"> </font><a href="mailto:alexfh@google.com" style="color:rgb(17,85,204)" target="_blank">alexfh@google.com</a> |</span><span style="border-top-width:2px;border-right-width:0px;border-bottom-width:0px;border-left-width:0px;border-top-style:solid;border-right-style:solid;border-bottom-style:solid;border-left-style:solid;border-top-color:rgb(238,178,17);border-right-color:rgb(238,178,17);border-bottom-color:rgb(238,178,17);border-left-color:rgb(238,178,17);padding-top:2px;margin-top:2px"> <a value="+35315435283" style="color:rgb(17,85,204)">+49 151 221 77 957</a></span></div>
</div><div><font color="#666666"><span style="font-family:Arial,Verdana,sans-serif">Google Germany GmbH | </span><span style="font-family:Arial,Verdana,sans-serif">Dienerstr. 12 | </span><span style="font-family:Arial,Verdana,sans-serif">80331 München</span></font></div>
<div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font color="#666666"><font><br>AG Hamburg, HRB 86891 | Sitz der Gesellschaft: </font><font face="arial, sans-serif">Hamburg</font><font><br>
Geschäftsführer: Graham Law, Katherine Stephens<span style="border-collapse:separate"><span style="font-family:inherit"><br></span></span></font></font></div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif">
<font color="#666666"><br></font></div><div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font><span style="border-collapse:collapse"><font color="#666666" face="arial, sans-serif">Tax ID:- 48/725/00206 </font></span></font></div>
<div style="font-size:13px;font-family:Arial,Verdana,sans-serif"><font><span style="border-collapse:collapse"><font color="#666666" face="arial, sans-serif">VAT ID:- DE813741370</font></span></font></div>
</div><br>
</font></span></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div>