[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