[cfe-commits] Refactor BasicConstraintManager

Ben Laurie benl at google.com
Thu Feb 12 10:57:13 PST 2009


All fixed, I hope.

On Thu, Feb 12, 2009 at 6:38 PM, Ben Laurie <benl at google.com> wrote:
> 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
>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: constraint-refactor-2.patch
Type: application/octet-stream
Size: 24697 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20090212/a38928eb/attachment.obj>


More information about the cfe-commits mailing list