Here is an updated version with wording changes and a fixit attached to the
note.
Maybe a better wording is possible for the note; suggestions welcome!


On 18 January 2013 21:31, Alexander Zinenko <[email protected]> wrote:

> On 18 January 2013 20:49, Nico Weber <[email protected]> wrote:
>
>> On Fri, Jan 18, 2013 at 10:41 AM, Alexander Zinenko <[email protected]>
>> wrote:
>> > Hi!
>> >
>> > I implemented a warning in parser as suggested here
>> >
>> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20121217/159766.html
>> ,
>> > e.g. warn in the following case
>> > if () {
>> > } if () { // probably should have been 'else if'
>> > } else {
>> > }
>> >
>> > Please review!
>> >
>> > This diagnostic found two places in chromium
>>
>> Can you link to these? I thought I grepped chromium's source for "}
>> if" after this thread and found 2 occurrences in WebKit (both false
>> positives, and both now fixed). Where the ones you found in WebKit
>> too? If so, then the true positive rate for this diagnostic is 0 / 2.
>>
>
> Nothing in WebKit.
> Actually one of them is in libxml inside chromium repo
> third_party/libxml/src/xlink.c:153:4
> another one is
> jingle/glue/pseudotcp_adapter.cc:372:5
> They both match the pattern above, but as far as I see do not introduce
> errors.
>
>
>> > and another one in firefox
>> > source base that are suspicious to have missed else.
>>
>> Was this a true positive?
>>
> Yes, his one is inside a long chain of if/else if/else if containing
> switches...
>
>>
>> >
>> > By the way, there is a name clash between
>> > Parser::ParenParseOption::CompoundStmt and CompoundStmt from AST.
>>  Maybe the
>> > former is worth renaming?
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > [email protected]
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >
>>
>
>

Attachment: missing-else.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to