[cfe-commits] [PATCH] Add Assumer class
Ted Kremenek
kremenek at apple.com
Wed Aug 27 06:47:18 PDT 2008
On Aug 26, 2008, at 11:44 PM, Zhongxing Xu wrote:
>
> Here are some comments!
>
> First, I have to be honest that I don't like the name "Assumer" for
> the actual interface. I think that using the word "Assume" is fine
> for the methods since that matches with the model-checking
> terminology (essentially, "Assume" represents a verb, or action,
> that we are taking with respect to modifying the state). I think
> the name of the Assumer class should reflect what it represents,
> which are constraints on the values of state. So I suggest
> *something* like the following (we can iterate on the precise name):
>
> Assumer -> ConstraintsManager
>
> This makes it much more clear that the ConstraintsManager manages
> 'constraints' within the state. We can iterate on the name, but
> "Assumer" I feel that isn't descriptive enough.
>
> I also think this patch is a little incomplete, since the code it
> introduces doesn't actually get hooked up to GRState. All of the
> Assume methods in GRStateManager should just forward over to the
> ConstraintManager object, e.g:
>
> class GRStateManager {
> ...
> const GRState* Assume(const GRState* St, NonLVal Cond, bool
> Assumption,
> bool& isFeasible) {
> return ConstraintsMgr.Assume(St, *this, Cond, Assumption,
> isFeasible);
> }
>
> If you actually migrate this core logic over to
> BasicConstraintsManager (BasicAssumer in your patch), then you can
> actually run "make test" to see if your patch does the right thing.
>
> New patch attached.
>
> <constraintmanager.patch>
Looks great! Please apply.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20080827/7f045f57/attachment.html>
More information about the cfe-commits
mailing list