<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi Alexander<br><div><div>On Apr 23, 2012, at 10:49 AM, Alexander Kornienko <<a href="mailto:alexfh@google.com">alexfh@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_extra">Hi,<br><br><div class="gmail_quote">On Mon, Apr 23, 2012 at 7:10 PM, Matthieu Monrocq <span dir="ltr"><<a href="mailto:matthieu.monrocq@gmail.com" target="_blank">matthieu.monrocq@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><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 :</div>
</div></blockquote><div>... </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div>In current implementation <b>the [[fallthrough]] attribute should be applied to a null statement</b> ...</div></div></blockquote></div></div><div>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>
</div></div></div></blockquote><div><br></div><div>Thanks for the comment, but you may have missed one of my points: '<font face="courier new, monospace">;</font>' 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 <font face="courier new, monospace">return</font> or <font face="courier new, monospace">break</font>. 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.</div></div></div></blockquote>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.</div><div><br></div><div>Thanks,</div><div>Pete<br><blockquote type="cite"><div class="gmail_extra"><div class="gmail_quote">
<div><br></div><div>Does it make sense?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_extra"><div class="gmail_quote"><div>This is looking really great, it'll be nice to be able to document intentional fallthroughs :)</div>
</div></div></blockquote><div><br></div><div>Yep, llvm code has hundreds of intentional fall-through locations marked (or not marked) with comments.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="gmail_extra"><div class="gmail_quote"><div><div class="h5"><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></blockquote></div></div></div></div></blockquote><div><br></div><div>More known issues:</div><div>
* 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;</div>
<div> * 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:</div><div><font face="courier new, monospace"> if(x) ...; [[fallthrough]]; else return 13;</font></div>
<div> here fix-it hint should have suggested something like this:</div><div><font face="courier new, monospace"> if(x) { ...; [[fallthrough]]; } else return 13;</font></div><div><br></div></div>-- <br>
</div><div class="gmail_extra">Best regards,</div><div class="gmail_extra">Alexander Kornienko</div>
_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></div><br></body></html>