[PATCH] [analyzer] Enable limited support for temporary destructors

Jordan Rose jordan_rose at apple.com
Thu Aug 29 18:58:53 PDT 2013


  Not sure if you're still working on this, but I think your plan to stage in `cfg-temporary-dtors` sounds all right. Hopefully we can get the CFG right sooner rather than later...but it probably means a lifetime-extended temporary is going to have to snapshot the current state so that it can decide how to destroy itself at the end of the scope:

  ```
  {
    const Cleanup &cleanup = condition ? NormalCleanup() : NoReturnDtor();
    // lots of code
  } // noreturn or not?
  ```


================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:99-102
@@ -98,2 +98,6 @@
     Ty = AT->getElementType();
-    LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
+    // FIXME: The if test is just a temporary workaround, becase
+    // ProcessTemporaryDtor sends us NULL regions. It will not be necessary once
+    // we can properly process temporary destructors.
+    if (LValue.getAsRegion())
+      LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
----------------
Isn't it easier to move the test outside the while loop?

================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:99-102
@@ -98,2 +98,6 @@
     Ty = AT->getElementType();
-    LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
+    // FIXME: The if test is just a temporary workaround, because
+    // ProcessTemporaryDtor sends us NULL regions. It will not be necessary once
+    // we can properly process temporary destructors.
+    if (LValue.getAsRegion())
+      LValue = State->getLValue(Ty, SVB.makeZeroArrayIndex(), LValue);
----------------
This could probably be put outside the loop, huh?

================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:810-814
@@ -809,1 +809,7 @@
 
+  // Temporary object destructor processing is currently broken, so we never
+  // inline them.
+  // FIXME: Remove this once temp destructors are working.
+  if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>())
+    return false;
+
----------------
Does it make sense to check the call kind first? These are all simple operations but they do require several dereferences.

================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1362
@@ +1361,3 @@
+  // If the value is already available, we don't need to do anything.
+  if(Pred->getState()->getSVal(Condition, Pred->getLocationContext()).isUnknownOrUndef()) {
+    // Resolve the condition in the presence of nested '||' and '&&'.
----------------
This looks like it's from the other patch, doesn't it?


http://llvm-reviews.chandlerc.com/D1259



More information about the cfe-commits mailing list