Hi, The patch looks pretty good to me. There are some minor style issues, e.g. missing spaces around operators in second part of EqualCompareChecker::checkPreStmt(), missing space between "//" and start of comments (which should normally start with a capital letter), etc. clang-format could take care of most of these for you.
I used hashing of the expressions in my patch, but your approach might be better since it allows you to look at the expression bit by bit and bail out early in the non-interesting cases. However, I don't feel qualified to sign off on a patch to the Analyzer. Perhaps Jordan could take a look? Thanks, Hans On Wed, Sep 25, 2013 at 2:58 AM, Daniel Marjamäki <[email protected]> wrote: > > Hello! > > To clarify. Do you want to see any changes in the checker before it can be > applied? > > .................................................................................................................. > Daniel Marjamäki Senior Engineer > Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden > > Mobile: +46 (0)709 12 42 62 > E-mail: [email protected] > > www.evidente.se > > ________________________________________ > Från: [email protected] [[email protected]] för Hans Wennborg > [[email protected]] > Skickat: den 23 september 2013 20:18 > Till: Daniel Marjamäki > Cc: [email protected]; Per Viberg > Ämne: Re: [PATCH][StaticAnalyzer] new check comparing equal expression > > On Mon, Sep 23, 2013 at 11:14 AM, Hans Wennborg <[email protected]> wrote: >> Feel free to look at my patch for inspiration. For example, I learned >> that "x == x" is a common pattern for checking if a float is NaN, so >> that shouldn't trigger a warning. > > Actually looking at your patch, I see you're well aware of this > already, and have some excellent test cases for it too :) > > - Hans _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
