[cfe-commits] r60151 - /cfe/trunk/lib/Analysis/BasicConstraintManager.cpp

[Ted Kremenek]

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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits