[PATCH] D66404: [CFG] Make destructor calls more accurate

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Thanks @nathanchance! I think i fixed it in rC370406 <https://reviews.llvm.org/rC370406>.



================
Comment at: cfe/trunk/lib/Analysis/CFG.cpp:2983-2984
 
-  // Add the return statement to the block.  This may create new blocks if R
-  // contains control-flow (short-circuit operations).
-  return VisitStmt(S, AddStmtChoice::AlwaysAdd);
----------------
This is basically what this comment was about. We're building the CFG from bottom to top, so when the return-value expression has a non-trivial CFG on its own, we need to continue building from the entry to the return-value expression CFG rather than from the block to which we've just appended the return statement.


================
Comment at: cfe/trunk/lib/Analysis/CFG.cpp:2999-3002
+    Expr *O = RS->getRetValue();
+    if (O)
+      Visit(O, AddStmtChoice::AlwaysAdd, /*ExternallyDestructed=*/true);
+    return Block;
----------------
Yeah, something broke.

{F9879548}
//Fig. 1.// Before.

{F9879549}
//Fig. 2.// After.

```lang=diff
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index e4ed0f86b91..5f2f46413b9 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -2996,7 +2996,7 @@ CFGBlock *CFGBuilder::VisitReturnStmt(Stmt *S) {
   if (ReturnStmt *RS = dyn_cast<ReturnStmt>(S)) {
     Expr *O = RS->getRetValue();
     if (O)
-      Visit(O, AddStmtChoice::AlwaysAdd, /*ExternallyDestructed=*/true);
+      return Visit(O, AddStmtChoice::AlwaysAdd, /*ExternallyDestructed=*/true);
     return Block;
   } else { // co_return
     return VisitChildren(S);
```
//Fig. 3.// Suggested fix.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66404/new/

https://reviews.llvm.org/D66404