[PATCH] [analyzer] Add very limited support for temporary destructors

Jordan Rose jordan_rose at apple.com
Mon Jul 15 10:11:15 PDT 2013


  I don't think this gets the right region (see the inlined comments below).

  Please also add some tests for this. A noreturn destructor test would be good enough on the analyzer side, since none of our checkers actually check the location of the object being destructed. I think the CFG-side tests in temp-obj-dtors-cfg-output.cpp are good enough, but if you think of anything worth adding please do add it. Like I said, this area is fairly under tested, since the analyzer doesn't use it and the analysis-based warnings only rely on it indirectly.


================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:598-600
@@ +597,5 @@
+
+  const MemRegion *Region =
+      getSValBuilder().getRegionManager().getCXXTempObjectRegion(
+          D.getBindTemporaryExpr(), Pred->getLocationContext());
+
----------------
Hm. This doesn't actually refer to the object being bound -- the temp region is created for the MaterializeTemporaryExpr, not the CXXBindTemporaryExpr. Moreover, it's currently not consistent -- if the value being materialized is already represented as a temporary region in the analyzer, we're not going to copy it.

(Actually, I'm not sure this happens anymore. I've tightened up our handling of glvalues vs. prvalues since writing that code.)

The best thing to do might be to just use UnknownVal as the "this" value for now (i.e. until you're/we're ready to tackle this problem for real). I think this should just work if you pass NULL to VisitCXXDestructor (which will trickle down to a NULL value in CXXDestructorCall), but there might be some fallout cleanup work.

================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:596
@@ +595,3 @@
+
+  ProgramStateRef State = Pred->getState();
+
----------------
Unused?

================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:809
@@ +808,3 @@
+  // inline them.
+  // FIME: Remove this once temp destructors are working.
+  if ((*currBldrCtx->getBlock())[currStmtIdx].getAs<CFGTemporaryDtor>())
----------------
Typo: "FIME"


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

BRANCH
  master

ARCANIST PROJECT
  clang



More information about the cfe-commits mailing list