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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
