[PATCH] Proposal on how to fix temporary dtors.

Jordan Rose jordan_rose at apple.com
Wed Jun 11 10:06:56 PDT 2014


Let's bring it home!

Thanks again for working on this. I'm excited to have it in trunk and hopefully soon to turn it on by default.

================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:521-527
@@ +520,9 @@
+  }
+  StmtNodeBuilder StmtBldr(PreVisit, Dst, *currBldrCtx);
+  ProgramStateRef State = Pred->getState();
+  assert(!State->contains<InitializedTemporariesSet>(
+      std::make_pair(BTE, Pred->getStackFrame())));
+  State = State->add<InitializedTemporariesSet>(
+      std::make_pair(BTE, Pred->getStackFrame()));
+  StmtBldr.generateNode(BTE, Pred, State);
+}
----------------
Manuel Klimek wrote:
> Jordan Rose wrote:
> > Right, this is not correct. We need to iterate over the nodes in the pre-visit set and update //all// of them to generate `Dst`.
> /me headdesks. Done. I'm still not sure I understand all the necessary pieces to generate new nodes (and have no idea how to write tests that trigger the corner cases), so I'd appreciate another careful look (or some test ideas).
I'm not sure about the tests offhand, either—most likely it's currently untestable because we have no pre-statement checkers looking for CXXBindTemporaryExprs. The general principle, though, is that anything that has an ExplodedNodeSet out parameter has the potential to generate new nodes.

================
Comment at: test/Analysis/temporaries.cpp:283-288
@@ +282,8 @@
+
+  bool testRecursiveFrames(bool isInner) {
+    if (isInner || check(NoReturnDtor()) || testRecursiveFrames(true)) {
+      clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+    }
+  }
+  void testRecursiveFramesStart() { testRecursiveFrames(false); }
+
----------------
I know this was my example, but I'm concerned that testRecursiveFrames will be evaluated as top-level as well, in which case the inner case is trivially reached. Let's make sure it is trying the !isInner case around with a little hack:

    if (isInner ||
        (clang_analyzer_warnIfReached(), false) || // expected-warning{{REACHABLE}}
        check(NoReturnDtor()) ||
        testRecursiveFrames(true)) {

http://reviews.llvm.org/D3627






More information about the cfe-commits mailing list