[cfe-commits] [PATCH] "missing else" warning

[Alexander Zinenko]

Hi chandlerc!

Since you originally proposed the idea of this warning, could you, please,
look at the implementation?


On 23 January 2013 01:43, Alexander Zinenko <ftynse at gmail.com> wrote:

> Ping.
>
> On 18 January 2013 22:03, Alexander Zinenko <ftynse at gmail.com> wrote:
>
>> 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 <ftynse at gmail.com> wrote:
>>
>>> On 18 January 2013 20:49, Nico Weber <thakis at chromium.org> wrote:
>>>
>>>> On Fri, Jan 18, 2013 at 10:41 AM, Alexander Zinenko <ftynse at gmail.com>
>>>> 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
>>>> > cfe-commits at cs.uiuc.edu
>>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>> >
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130225/0617e46e/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: missing-else.patch
Type: application/octet-stream
Size: 4975 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130225/0617e46e/attachment.obj>