Remove lval::FieldOffset, lval::ArrayOffset. These will be replaced with regions.

Remove GRExprEngine::getLVal and RValues::MakeVal.
Enhance StoreManager "GetLValue" methods to dispatch for specific kinds of lvalue queries, as opposed to interogating the expression tree (GRExprEngine already does this).

Added FIXMEs.  In particular, we no longer "assume" that a base pointer in a field/array access is null (this logic was removed).  Perhaps we should do this when fetching the lvalue for fields and array elements?



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@57657 91177308-0d34-0410-b5e6-96231b3b80d8

6 files changed, 40 insertions, 126 deletions

diff --git a/lib/Analysis/BasicConstraintManager.cpp b/lib/Analysis/BasicConstraintManager.cpp
index 66bf082b13..8d391bbb58 100644
--- a/lib/Analysis/BasicConstraintManager.cpp
+++ b/lib/Analysis/BasicConstraintManager.cpp
@@ -136,14 +136,6 @@ const GRState* BasicConstraintManager::AssumeAux(const GRState* St, LVal Cond,
     isFeasible = Assumption;
     return St;
 
-  case lval::FieldOffsetKind:
-    return AssumeAux(St, cast<lval::FieldOffset>(Cond).getBase(),
-                     Assumption, isFeasible);
-
-  case lval::ArrayOffsetKind:
-    return AssumeAux(St, cast<lval::ArrayOffset>(Cond).getBase(),
-                     Assumption, isFeasible);
-
   case lval::ConcreteIntKind: {
     bool b = cast<lval::ConcreteInt>(Cond).getValue() != 0;
     isFeasible = b ? Assumption : !Assumption;
diff --git a/lib/Analysis/BasicStore.cpp b/lib/Analysis/BasicStore.cpp
index 15a20c843a..000ea1bef2 100644
--- a/lib/Analysis/BasicStore.cpp
+++ b/lib/Analysis/BasicStore.cpp
@@ -46,9 +46,11 @@ public:
   virtual LVal getLVal(const VarDecl* VD) {
     return lval::MemRegionVal(MRMgr.getVarRegion(VD));
   }
-
-  virtual RVal getLValue(const GRState* St, const Expr* Ex);
-  virtual RVal getLValue(const GRState* St, const ObjCIvarDecl* D, RVal Base);
+  
+  RVal getLValueVar(const GRState* St, const VarDecl* VD);
+  RVal getLValueIvar(const GRState* St, const ObjCIvarDecl* D, RVal Base);  
+  RVal getLValueField(const GRState* St, const FieldDecl* D, RVal Base);  
+  RVal getLValueElement(const GRState* St, RVal Base, RVal Offset);
   
   virtual Store
   RemoveDeadBindings(Store store, Stmt* Loc, const LiveVariables& Live,
@@ -76,34 +78,26 @@ public:
 StoreManager* clang::CreateBasicStoreManager(GRStateManager& StMgr) {
   return new BasicStoreManager(StMgr);
 }
+RVal BasicStoreManager::getLValueVar(const GRState* St, const VarDecl* VD) {
+  QualType T = VD->getType();
+  assert(!T->isArrayType() && "Array and struct variable have no lvalue.");
+  return lval::MemRegionVal(MRMgr.getVarRegion(VD));
+}
+  
+RVal BasicStoreManager::getLValueIvar(const GRState* St, const ObjCIvarDecl* D,
+                                      RVal Base) {
+  return UnknownVal();
+}
+  
+  
+RVal BasicStoreManager::getLValueField(const GRState* St, const FieldDecl* D,
+                                       RVal Base) {
+  return UnknownVal();
+}
 
-// FIXME: replace ArrayOffset and FieldOffset with some region value.
-RVal BasicStoreManager::getLValue(const GRState* St, const Expr* Ex) {
-  if (const DeclRefExpr* DRE = dyn_cast<DeclRefExpr>(Ex)) {
-    const VarDecl* VD = cast<VarDecl>(DRE->getDecl());
-    QualType T = VD->getType();
-
-    // Array and struct variable have no lvalue.
-    assert(!T->isArrayType());
-
-    return lval::MemRegionVal(MRMgr.getVarRegion(VD));
-
-  } else if (const ArraySubscriptExpr* A = dyn_cast<ArraySubscriptExpr>(Ex)) {
-    const Expr* Base = A->getBase()->IgnoreParens();
-    const Expr* Idx  = A->getIdx()->IgnoreParens();
-    RVal BaseV = StateMgr.GetRVal(St, Base);
-    RVal IdxV = StateMgr.GetRVal(St, Idx);
-    return lval::ArrayOffset::Make(StateMgr.getBasicVals(), BaseV, IdxV);
-
-  } else if (const MemberExpr* M = dyn_cast<MemberExpr>(Ex)) {
-    Expr* Base = M->getBase()->IgnoreParens();
-    RVal BaseV = StateMgr.GetRVal(St, Base);
-    return lval::FieldOffset::Make(StateMgr.getBasicVals(), BaseV, 
-                                   M->getMemberDecl());
-  } else {
-    Ex->dump();
-    assert(0);
-  }
+RVal BasicStoreManager::getLValueElement(const GRState* St, RVal Base,
+                                         RVal Offset) {
+  return UnknownVal();
 }
 
 RVal BasicStoreManager::GetRVal(Store St, LVal LV, QualType T) {
@@ -134,12 +128,7 @@ RVal BasicStoreManager::GetRVal(Store St, LVal LV, QualType T) {
       // Some clients may call GetRVal with such an option simply because
       // they are doing a quick scan through their LVals (potentially to
       // invalidate their bindings).  Just return Undefined.
-      return UndefinedVal();
-      
-    case lval::ArrayOffsetKind:
-    case lval::FieldOffsetKind:
-      return UnknownVal();
-      
+      return UndefinedVal();            
     case lval::FuncValKind:
       return LV;
       
@@ -154,11 +143,6 @@ RVal BasicStoreManager::GetRVal(Store St, LVal LV, QualType T) {
   
   return UnknownVal();
 }
-
-RVal BasicStoreManager::getLValue(const GRState* St, const ObjCIvarDecl* D,
-                                  RVal Base) {
-  return UnknownVal();  
-}
   
 Store BasicStoreManager::SetRVal(Store store, LVal LV, RVal V) {    
   switch (LV.getSubKind()) {      
diff --git a/lib/Analysis/CheckNSError.cpp b/lib/Analysis/CheckNSError.cpp
index 03c9af3a72..6e52dd952d 100644
--- a/lib/Analysis/CheckNSError.cpp
+++ b/lib/Analysis/CheckNSError.cpp
@@ -216,7 +216,7 @@ void NSErrorCheck::CheckParamDeref(VarDecl* Param, GRStateRef rootState,
                                    GRExprEngine& Eng, GRBugReporter& BR,
                                    bool isNSErrorWarning) {
   
-  RVal ParamRVal = rootState.GetRVal(Eng.getLVal(Param));
+  RVal ParamRVal = rootState.GetLValue(Param);
 
   // FIXME: For now assume that ParamRVal is symbolic.  We need to generalize
   // this later.
diff --git a/lib/Analysis/GRExprEngine.cpp b/lib/Analysis/GRExprEngine.cpp
index f974dfc2c4..2bc46d5990 100644
--- a/lib/Analysis/GRExprEngine.cpp
+++ b/lib/Analysis/GRExprEngine.cpp
@@ -817,7 +817,8 @@ void GRExprEngine::VisitDeclRefExpr(DeclRefExpr* Ex, NodeTy* Pred, NodeSet& Dst,
       return;
     }
 
-    RVal V = GetLValue(St, Ex);
+    RVal V = StateMgr.GetLValue(St, VD);
+    
     if (asLValue)
       MakeNode(Dst, Ex, Pred, SetRVal(St, Ex, V));
     else
@@ -850,22 +851,16 @@ void GRExprEngine::VisitArraySubscriptExpr(ArraySubscriptExpr* A, NodeTy* Pred,
   
   Expr* Base = A->getBase()->IgnoreParens();
   Expr* Idx  = A->getIdx()->IgnoreParens();
-  
   NodeSet Tmp;
-
-  // Get Base's rvalue, which should be an LocVal.
-  Visit(Base, Pred, Tmp);
+  Visit(Base, Pred, Tmp);   // Get Base's rvalue, which should be an LocVal.
   
-  for (NodeSet::iterator I1=Tmp.begin(), E1=Tmp.end(); I1!=E1; ++I1) {
-    
-    // Evaluate the index.
+  for (NodeSet::iterator I1=Tmp.begin(), E1=Tmp.end(); I1!=E1; ++I1) {    
     NodeSet Tmp2;
-    Visit(Idx, *I1, Tmp2);
+    Visit(Idx, *I1, Tmp2);     // Evaluate the index.
       
     for (NodeSet::iterator I2=Tmp2.begin(), E2=Tmp2.end(); I2!=E2; ++I2) {
-
       const GRState* St = GetState(*I2);
-      RVal V = GetLValue(St, A);
+      RVal V = StateMgr.GetLValue(St, GetRVal(St, Base), GetRVal(St, Idx));
 
       if (asLValue)
         MakeNode(Dst, A, *I2, SetRVal(St, A, V));
@@ -880,15 +875,16 @@ void GRExprEngine::VisitMemberExpr(MemberExpr* M, NodeTy* Pred,
                                    NodeSet& Dst, bool asLValue) {
   
   Expr* Base = M->getBase()->IgnoreParens();
-
   NodeSet Tmp;
-
-  // Get Base expr's rvalue.
   Visit(Base, Pred, Tmp);
 
   for (NodeSet::iterator I = Tmp.begin(), E = Tmp.end(); I != E; ++I) {
     const GRState* St = GetState(*I);
-    RVal L = GetLValue(St, M);
+    // FIXME: Should we insert some assumption logic in here to determine
+    // if "Base" is a valid piece of memory?  Before we put this assumption
+    // later when using FieldOffset lvals (which we no longer have).    
+    RVal L = StateMgr.GetLValue(St, M->getMemberDecl(), GetRVal(St, Base));
+
     if (asLValue)
       MakeNode(Dst, M, *I, SetRVal(St, M, L));
     else
diff --git a/lib/Analysis/GRSimpleVals.cpp b/lib/Analysis/GRSimpleVals.cpp
index 44703163e0..07ec3e8bff 100644
--- a/lib/Analysis/GRSimpleVals.cpp
+++ b/lib/Analysis/GRSimpleVals.cpp
@@ -282,11 +282,6 @@ RVal GRSimpleVals::EvalEQ(GRExprEngine& Eng, LVal L, LVal R) {
       return UnknownVal();      
     }
       
-      // FIXME: Different offsets can map to the same memory cell.
-    case lval::ArrayOffsetKind:
-    case lval::FieldOffsetKind:
-      // Fall-through.
-      
     case lval::MemRegionKind:
     case lval::FuncValKind:
     case lval::GotoLabelKind:
@@ -346,11 +341,6 @@ RVal GRSimpleVals::EvalNE(GRExprEngine& Eng, LVal L, LVal R) {
       break;
     }
       
-      // FIXME: Different offsets can map to the same memory cell.
-    case lval::ArrayOffsetKind:
-    case lval::FieldOffsetKind:
-      // Fall-through.
-      
     case lval::MemRegionKind:
     case lval::FuncValKind:
     case lval::GotoLabelKind:
diff --git a/lib/Analysis/RValues.cpp b/lib/Analysis/RValues.cpp
index 2b573681e8..551945ce26 100644
--- a/lib/Analysis/RValues.cpp
+++ b/lib/Analysis/RValues.cpp
@@ -43,10 +43,9 @@ RVal::symbol_iterator RVal::symbol_begin() const {
     const nonlval::LValAsInteger& V = cast<nonlval::LValAsInteger>(*this);
     return  V.getPersistentLVal().symbol_begin();
   }
-  else if (isa<lval::FieldOffset>(this)) {
-    const lval::FieldOffset& V = cast<lval::FieldOffset>(*this);
-    return V.getPersistentBase().symbol_begin();
-  }
+  
+  // FIXME: We need to iterate over the symbols of regions.
+
   return NULL;
 }
 
@@ -267,37 +266,6 @@ LVal LVal::MakeVal(StringLiteral* S) {
 }
 
 //===----------------------------------------------------------------------===//
-// Utility methods for constructing RVals (both NonLVals and LVals).
-//===----------------------------------------------------------------------===//
-
-// Remove this method?
-RVal RVal::MakeVal(GRStateManager& SMgr, DeclRefExpr* E) {
-  
-  ValueDecl* D = cast<DeclRefExpr>(E)->getDecl();
-  
-  if (VarDecl* VD = dyn_cast<VarDecl>(D)) {
-    return SMgr.getLVal(VD);
-  }
-  else if (EnumConstantDecl* ED = dyn_cast<EnumConstantDecl>(D)) {
-    
-    // FIXME: Do we need to cache a copy of this enum, since it
-    // already has persistent storage?  We do this because we
-    // are comparing states using pointer equality.  Perhaps there is
-    // a better way, since APInts are fairly lightweight.
-    BasicValueFactory& BasicVals = SMgr.getBasicVals();
-    return nonlval::ConcreteInt(BasicVals.getValue(ED->getInitVal()));          
-  }
-  else if (FunctionDecl* FD = dyn_cast<FunctionDecl>(D)) {
-    return lval::FuncVal(FD);
-  }
-  
-  assert (false &&
-          "ValueDecl support for this ValueDecl not implemented.");
-  
-  return UnknownVal();
-}
-
-//===----------------------------------------------------------------------===//
 // Pretty-Printing.
 //===----------------------------------------------------------------------===//
 
@@ -424,22 +392,6 @@ void LVal::print(std::ostream& Out) const {
           << "\"";
       break;
       
-    case lval::FieldOffsetKind: {
-      const lval::FieldOffset& C = *cast<lval::FieldOffset>(this);
-      C.getBase().print(Out);
-      Out << "." << C.getFieldDecl()->getName() << " (field LVal)";
-      break;
-    }
-      
-    case lval::ArrayOffsetKind: {
-      const lval::ArrayOffset& C = *cast<lval::ArrayOffset>(this);
-      C.getBase().print(Out);
-      Out << "[";
-      C.getOffset().print(Out);
-      Out << "] (lval array entry)";
-      break;
-    }
-      
     default:
       assert (false && "Pretty-printing not implemented for this LVal.");
       break;