All fixed, I hope.

On Thu, Feb 12, 2009 at 6:38 PM, Ben Laurie <[email protected]> wrote:
> 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
>>
>

Attachment: constraint-refactor-2.patch
Description: Binary data

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to