[cfe-commits] Refactor BasicConstraintManager

[Ben Laurie]

On Thu, Feb 12, 2009 at 6:36 PM, Ted Kremenek <kremenek at apple.com> 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
>> cfe-commits at cs.uiuc.edu
>> 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
>