<div dir="ltr">Here is an updated version with wording changes and a fixit attached to the note.<div>Maybe a better wording is possible for the note; suggestions welcome!</div><div class="gmail_extra"><br><br><div class="gmail_quote">
On 18 January 2013 21:31, Alexander Zinenko <span dir="ltr"><<a href="mailto:ftynse@gmail.com" target="_blank">ftynse@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 dir="ltr"><div>On 18 January 2013 20:49, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote">
<div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div>On Fri, Jan 18, 2013 at 10:41 AM, Alexander Zinenko <<a href="mailto:ftynse@gmail.com" target="_blank">ftynse@gmail.com</a>> wrote:<br>
> Hi!<br>
><br>
> I implemented a warning in parser as suggested here<br>
> <a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20121217/159766.html" target="_blank">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20121217/159766.html</a>,<br>
> e.g. warn in the following case<br>
> if () {<br>
> } if () { // probably should have been 'else if'<br>
> } else {<br>
> }<br>
><br>
> Please review!<br>
><br>
> This diagnostic found two places in chromium<br>
<br>
</div>Can you link to these? I thought I grepped chromium's source for "}<br>
if" after this thread and found 2 occurrences in WebKit (both false<br>
positives, and both now fixed). Where the ones you found in WebKit<br>
too? If so, then the true positive rate for this diagnostic is 0 / 2.<br></blockquote><div><br></div></div><div>Nothing in WebKit.</div><div>Actually one of them is in libxml inside chromium repo</div><div>third_party/libxml/src/xlink.c:153:4</div>
<div>another one is</div><div>jingle/glue/pseudotcp_adapter.cc:372:5</div><div>They both match the pattern above, but as far as I see do not introduce errors.</div><div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>
> and another one in firefox<br>
> source base that are suspicious to have missed else.<br>
<br>
</div>Was this a true positive?<br></blockquote></div><div>Yes, his one is inside a long chain of if/else if/else if containing switches...</div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div><br>
><br>
> By the way, there is a name clash between<br>
> Parser::ParenParseOption::CompoundStmt and CompoundStmt from AST. Maybe the<br>
> former is worth renaming?<br>
><br>
</div>> _______________________________________________<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><br>
><br>
</blockquote></div></div><br></div></div>
</blockquote></div><br></div></div>