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

[Alex McCarthy]

Thanks again for smashing on this, Manuel. This rabbit hole just keeps on getting deeper.

I'm in my usual position about not knowing enough about clang to know whether this makes sense at a high-level. Consequently, the following comments might not make sense or might miss problems with the overall approach, so please take them with a grain of salt.

================
Comment at: lib/Analysis/CFG.cpp:439
@@ +438,3 @@
+  void addAutomaticObjDtorsForLifetimeExtendedTemporaries(
+      Stmt *E, bool LifetimeExtended = false);
+  void addAutomaticObjDtorsForVarDecl(VarDecl *VD, Stmt* S);
----------------
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?

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

================
Comment at: lib/Analysis/CFG.cpp:1071
@@ -1066,1 +1070,3 @@
+      appendTemporaryDtor(Block, cast<CXXBindTemporaryExpr>(E));
     }
+    break;
----------------
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?

================
Comment at: lib/Analysis/CFG.cpp:1091
@@ +1090,3 @@
+    const CXXDestructorDecl *Dtor = QT->getAsCXXRecordDecl()->getDestructor();
+    if (Dtor->isNoReturn())
+      Block = createNoReturnBlock();
----------------
if (noReturn) { createNoReturnBlock(); } else { autoCreateBlock(); }
This seems to be a common pattern. Maybe worth adding a helper to make this code easier to read?

================
Comment at: lib/Analysis/CFG.cpp:1247
@@ +1246,3 @@
+      assert(MTE->GetTemporaryExpr());
+      const Expr *ExtendedE =
+          MTE->GetTemporaryExpr()->skipRValueSubobjectAdjustments(CommaLHSs,
----------------
Maybe add a helper here for calls like this that don't care about CommaLHSs or Adjustments to make this easier to read

http://reviews.llvm.org/D4706