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. 2) We also limit to 80 cols. Again another coding style. There are a few lines here and there that overflow that limit. 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. 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
