[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [cfe-commits] r60151 -
From:       Ted Kremenek <kremenek () apple ! com>
Date:       2008-11-27 6:31:07
Message-ID: 3EE396D0-3A72-4A40-8042-AB3CAD8D899B () apple ! com
[Download RAW message or body]

Wow.  Thanks Zhongxing.  I guess this wasn't noticed before because  
all sets are allocated using the shared BumpPtrAllocator, so this  
wasn't a correctness problem (as the sets would stay in memory).   
However, allocating all sets from separate factories would mean that  
two equivalent sets wouldn't be referentially equivalent.  I can see  
this as seriously hurting our ability to have states cache out more  
frequently when using BasicConstraintManager.  Awesome fix.

On Nov 26, 2008, at 10:08 PM, Zhongxing Xu wrote:

> Author: zhongxingxu
> Date: Thu Nov 27 00:08:40 2008
> New Revision: 60151
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=60151&view=rev
> Log:
> Factory objects should not be temporary. It caches all objects in  
> the set.
> 
> Modified:
> cfe/trunk/lib/Analysis/BasicConstraintManager.cpp
> 
> Modified: cfe/trunk/lib/Analysis/BasicConstraintManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BasicConstraintManager.cpp?rev=60151&r1=60150&r2=60151&view=diff
>  
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/BasicConstraintManager.cpp (original)
> +++ cfe/trunk/lib/Analysis/BasicConstraintManager.cpp Thu Nov 27  
> 00:08:40 2008
> @@ -29,9 +29,10 @@
> // constants and integer variables.
> class VISIBILITY_HIDDEN BasicConstraintManager : public  
> ConstraintManager {
> GRStateManager& StateMgr;
> -
> +  GRState::IntSetTy::Factory ISetFactory;
> public:
> -  BasicConstraintManager(GRStateManager& statemgr) :  
> StateMgr(statemgr) {}
> +  BasicConstraintManager(GRStateManager& statemgr)
> +    : StateMgr(statemgr), ISetFactory(statemgr.getAllocator()) {}
> 
> virtual const GRState* Assume(const GRState* St, SVal Cond,
> bool Assumption, bool& isFeasible);
> @@ -409,7 +410,7 @@
> 
> const GRState* BasicConstraintManager::AddNE(const GRState* St,  
> SymbolID sym,
> const llvm::APSInt& V) {
> -  GRState::IntSetTy::Factory ISetFactory(StateMgr.getAllocator());
> +
> GRStateRef state(St, StateMgr);
> 
> // First, retrieve the NE-set associated with the given symbol.
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]