On Thu, Feb 12, 2009 at 6:36 PM, Ted Kremenek <[email protected]> wrote: > > On Feb 12, 2009, at 4:57 AM, Ben Laurie wrote: > >> My new RangeConstraintManager shares a lot of code with >> BasicConstraintManager. This patch refactors BasicConstraintManager to >> inherit the shared code from SimpleConstraintManager. No code changes >> have been made, stuff has just been moved around. >> >> It also constifies getSymVal, since non-constness prevents its use in >> isEqual. >> <constraint-refactor.patch>_______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > Hi Ben, > > This looks pretty good. Three comments, the first two just very trivial > stuff: > > 1) Please use spaces instead of tabs. It's just part of the coding style of > the entire codebase.
OK. > 2) We also limit to 80 cols. Again another coding style. There are a few > lines here and there that overflow that limit. Well, if you had, I would have, too. But glad to hear you do, will fix up places where this is not true. > It's not clear that SimpleConstraintManager needs to be part of the public > API in "include/clang". For now I suggest we just put > SimpleConstraintManager.h in "lib/Analysis" to avoid cluttering the "public" > API. OK. > My reasoning is that SimpleConstraintManager appears to be (for the moment) > just an implementation detail of BasicConstraintManager and > RangeConstraintManager. When there is a need to make it more public we can > do so later. > > Ted > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
