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 <fty...@gmail.com> wrote: > Ping. > > On 18 January 2013 22:03, Alexander Zinenko <fty...@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 <fty...@gmail.com> wrote: >> >>> On 18 January 2013 20:49, Nico Weber <tha...@chromium.org> wrote: >>> >>>> On Fri, Jan 18, 2013 at 10:41 AM, Alexander Zinenko <fty...@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@cs.uiuc.edu >>>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> > >>>> >>> >>> >> >
missing-else.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
