[PATCH] Proposal on how to fix temporary dtors.

[Manuel Klimek]

Ping :)


On Tue, Jun 3, 2014 at 10:59 AM, Manuel Klimek <klimek at google.com> wrote:

> 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.
> Done.
>
> ================
> 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.
>
> http://reviews.llvm.org/D3627
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140610/971be336/attachment.html>