[PATCH] An attempt at fixing lifetime extended temporaries in the CFG.

[Manuel Klimek]

================
Comment at: lib/Analysis/CFG.cpp:439
@@ +438,3 @@
+  void addAutomaticObjDtorsForLifetimeExtendedTemporaries(
+      Stmt *E, bool LifetimeExtended = false);
+  void addAutomaticObjDtorsForVarDecl(VarDecl *VD, Stmt* S);
----------------
Alex McCarthy wrote:
> This signature's a bit weird to me: this method is called for lifetime extended temporaries, but LifetimeExtended can be false: what does that mean?
> 
> Does this method or its arguments need to be renamed?
Yes, as I wrote, this is still pretty rough - I expect some major refactoring to happen before it lands.

================
Comment at: lib/Analysis/CFG.cpp:1057
@@ -1048,1 +1056,3 @@
+      addAutomaticObjDtorsForLifetimeExtendedTemporaries(
+          const_cast<Expr*>(ExtendedE), /*LifetimeExtended=*/true);
     }
----------------
Alex McCarthy wrote:
> Rather than const cast here, should we add or remove const from some other signature so this isn't needed?
Unfortunately the sinks are non-const, which basically kills us.

================
Comment at: lib/Analysis/CFG.cpp:1071
@@ -1066,1 +1070,3 @@
+      appendTemporaryDtor(Block, cast<CXXBindTemporaryExpr>(E));
     }
+    break;
----------------
Alex McCarthy wrote:
> Maybe add a comment explaining that we don't need to anything for a bind statement for a lifetime extended temporary because there's no object creation going on?
Added a comment (object creation is going on, but we handle the descturctor elsewhere).

================
Comment at: lib/Analysis/CFG.cpp:1091
@@ +1090,3 @@
+    const CXXDestructorDecl *Dtor = QT->getAsCXXRecordDecl()->getDestructor();
+    if (Dtor->isNoReturn())
+      Block = createNoReturnBlock();
----------------
Alex McCarthy wrote:
> if (noReturn) { createNoReturnBlock(); } else { autoCreateBlock(); }
> This seems to be a common pattern. Maybe worth adding a helper to make this code easier to read?
I actually think this is probably not quite right here yet (I have failing tests with noreturn dtor cases), so I'll do that once all that has settled down :)

================
Comment at: lib/Analysis/CFG.cpp:1247
@@ +1246,3 @@
+      assert(MTE->GetTemporaryExpr());
+      const Expr *ExtendedE =
+          MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments(CommaLHSs,
----------------
Alex McCarthy wrote:
> Maybe add a helper here for calls like this that don't care about CommaLHSs or Adjustments to make this easier to read
I'd actually like to not need to visit this 3 times. Alas, I'm not sure it's possible. You're right, if it's not possible, we should pull something out...

http://reviews.llvm.org/D4706