[cfe-commits] [PATCH] RegionStore

[Ted Kremenek]

On Oct 19, 2008, at 6:24 AM, Zhongxing Xu wrote:

> 1. Initial support for local arrays in RegionStoreManager::AddDecl()
> 2. Move special processing of loading array variable's lvalue from  
> GRExprEngine to Store. This simplifies code. And this is also what  
> we have to do when we processing code like:

Hi Zhongxing,

The following down cast is not necessary, as R is a VarRegion:

+++ lib/Analysis/BasicStore.cpp	（工作副本）
@@ -42,7 +42,6 @@

    virtual MemRegionManager& getRegionManager() { return MRMgr; }

-  // FIXME: Investigate what is using this. This method should be  
removed.
    virtual Loc getLoc(const VarDecl* VD) {
      return loc::MemRegionVal(MRMgr.getVarRegion(VD));
    }
@@ -148,7 +147,15 @@

        if (!R)
          return UnknownVal();
-
+
+      // We let an array variable's rvalue be its lvalue, because we  
have to
+      // process code like: int a[10]; int (*p)[10]; p = &a; (*p)[3]  
= 1; The
+      // other benifit is that we can save the special handling of  
array
+      // variables in VisitDeclRefExpr().
+      if (const VarRegion* VR = dyn_cast<VarRegion>(R))
+        if (VR->getDecl()->getType()->isArrayType())
+          return LV;
+

I'm also not certain why this logic is needed (or if it is correct).   
The expression '&a' is handled by VisitUnaryOperator, and for the case  
of '&' it calls VisitLValue on its subexpression.  I don't see where a  
call to GetSVal is involved.

The cleanups to GRExprEngine are very nice.

All in all the patch looks good, but I am a little unclear about why  
BasicStore::GetSVal should return the lvalue for an array in an rvalue  
context.  GRExprEngine handles the implicit conversions between  
lvalues and rvalues, not the store.