[PATCH] Proposal on how to fix temporary dtors.

Manuel Klimek klimek at google.com
Tue Jun 3 01:59:12 PDT 2014

We still have the question about what to do with the FoldingSetNode for pairs in the other patch open.

Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:765-766
@@ +764,4 @@
+      ExplodedNodeSet Next;
+      VisitCXXBindTemporaryExpr(cast<CXXBindTemporaryExpr>(S), Pred, PreVisit,
+                                Next);
+      getCheckerManager().runCheckersForPostStmt(Dst, Next, S, *this);
Jordan Rose wrote:
> Why is `Pred` being passed here? The `PreVisit` set has the same information.

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);
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).

Comment at: test/Analysis/temporaries.cpp:291-294
@@ +290,6 @@
+  void testNestedFullStatements() {
+    []() { check(NoReturnDtor()); } != nullptr || check(Dtor());
+    // The CFG for this now looks correct, but we still do not reach the line
+    // below.
+    clang_analyzer_warnIfReached(); // FIXME: Should warn.
+  }
Jordan Rose wrote:
> I don't think we assume that lambda-exprs are always non-null. We should.
It's much worse. We currently ignore lambdas. I updated the test / added a comment.


More information about the cfe-commits mailing list