[cfe-commits] [PATCH] VisitInitListExpr

[Ted Kremenek]

On Oct 29, 2008, at 6:35 PM, Zhongxing Xu wrote:

> This patch implements GRExprEngine::VisitInitListExpr().  
> CompoundValsData is added to BasicValue classes. I found it can  
> represent recursive struct compound value, so we do not need to  
> flatten struct init list values. (not implemented in this patch).


Now the review for GRExprEngine.

Index: include/clang/Analysis/PathSensitive/GRExprEngine.h
===================================================================
--- include/clang/Analysis/PathSensitive/GRExprEngine.h	（版本  
58411）
+++ include/clang/Analysis/PathSensitive/GRExprEngine.h	（工作副 
本）
@@ -516,7 +516,9 @@

    /// VisitGuardedExpr - Transfer function logic for ?,  
__builtin_choose
    void VisitGuardedExpr(Expr* Ex, Expr* L, Expr* R, NodeTy* Pred,  
NodeSet& Dst);
-
+
+  void VisitInitListExpr(InitListExpr* E, NodeTy* Pred, NodeSet& Dst);
+

Looks good.


Index: lib/Analysis/GRExprEngine.cpp
===================================================================
--- lib/Analysis/GRExprEngine.cpp	（版本 58411）
+++ lib/Analysis/GRExprEngine.cpp	（工作副本）
@@ -350,6 +350,10 @@
        VisitCast(C, C->getSubExpr(), Pred, Dst);
        break;
      }
+
+    case Stmt::InitListExprClass:
+      VisitInitListExpr(cast<InitListExpr>(S), Pred, Dst);
+      break;

Looks good.


+void GRExprEngine::VisitInitListExpr(InitListExpr* E, NodeTy* Pred,
+                                     NodeSet& Dst) {
+  const GRState* state = GetState(Pred);

+  QualType T = E->getType();
+
+  unsigned NumInitElements = E->getNumInits();
+
+  llvm::SmallVector<SVal, 10> InitVals;
+  InitVals.reserve(NumInitElements);
+
+
+  if (T->isArrayType()) {
+    for (unsigned i = 0; i < NumInitElements; ++i) {
+      Expr* Init = E->getInit(i);

Not certain if we need E->IngoreParens()->getInit().

+      NodeSet Tmp;
+      Visit(Init, Pred, Tmp);
+
+      // We do not allow state splitting during Initializer visiting.
+      assert(Tmp.size() == 1);

I'm fine with this for an initial patch (keeps things simple), but  
this is a bogus limitation that is easily broken.  For example,  
initializers that contain function calls may easily have state- 
splitting.  We allow state-splitting when processing CallExprs and  
ObjCMessageExprs; this is no different.  The difference here is that  
initializer lists might be very long, so using recursive calls to  
"Aux" functions probably isn't a good idea.  We can manually do this  
recursion by maintaining a stack of NodeSets (ala llvm::SmallVector?)  
and processing it like a worklist.  This is a little complicated, so  
it should go in a second patch.

+      // Get the new intermediate node and its state.
+      Pred = *Tmp.begin();
+      state = GetState(Pred);
+
+      SVal InitV = GetSVal(state, Init);
+      InitVals.push_back(InitV);
+    }

Looks good.

+    // Now we have a vector holding all init values. Make  
CompoundSValData.
+    SVal V = NonLoc::MakeCompoundVal(T, NumInitElements, &InitVals[0],
+                                     StateMgr.getBasicVals());
+
+    // Make final state and node.
+    state = SetSVal(state, E, V);
+
+    MakeNode(Dst, E, Pred, state);
+    return;
+  }

Looks great.

+
+  if (T->isStructureType()) {
+    // FIXME: to be implemented.
+    MakeNode(Dst, E, Pred, state);
+    return;
+  }

Isn't the case for structs exactly the same as for arrays?  I don't  
see anything in the above code that is specific to arrays.

+
+  if (Loc::IsLocType(T) || T->isIntegerType()) {
+    // FIXME: to be implemented.
+    MakeNode(Dst, E, Pred, state);
+    return;
+  }

Is this possible?  Can InitListExpr have one of these types?

+
+  if (T->isUnionType()) {
+    // FIXME: to be implemented.
+    MakeNode(Dst, E, Pred, state);
+    return;
+  }

Yeah, don't know what to do about this one yet.

+  printf("InitListExpr type = %s\n", T.getAsString().c_str());
+  assert(0 && "unprocessed InitListExpr type");
+}

The assertion is a great idea.