Fix some cases of incorrect handling of lifetime extended temporaries.

[klimek]

Summary

MaterializeTemporaryExpr already contains information about the lifetime
of the temporary; if the lifetime is not the full statement, we do not
want to emit a destructor at the end of the full statement for it.

[alexmc]

Is there a test case that covers this bug?

[jordan_rose]

I second what Alex said, and also wonder if this affects existing destruction of lifetime-extended objects (which happens under the auto destructors path, and is also not quite correct).

[jordan_rose]

Can't you write a CFG-based test case?

[klimek]

Add tests and fix lifetime extensions in the face of the comma operator.

[klimek]

Add tests for lifetime extension of multiple objects via initializer.

[klimek]

Make test more readable.

[jordan_rose]

Can you add a test for the subobject case too, even though that doesn't work yet either?

const LifetimeExtend &ref = Aggregate{ 0, 1 }.a;
6;
// expect ~Aggregate here rather than ~LifetimeExtend

Other than that, this seems like a fine incremental improvement.

test/Analysis/cfg.cpp

421–429

I did not realize this worked. Huh. That's going to be fun.

[klimek]

Submitted as r214292. Requested tests in upcoming patch (where they don't crash any more).

lib/Analysis/CFG.cpp

CFGBlock *CFGBuilder::createNoReturnBlock() {
   return B;
 }
 
 /// addInitializer - Add C++ base or member initializer element to CFG.
 CFGBlock *CFGBuilder::addInitializer(CXXCtorInitializer *I) {
   if (!BuildOpts.AddInitializers)
     return Block;
 
-  bool IsReference = false;
   bool HasTemporaries = false;
 
   // Destructors of temporaries in initialization expression should be called
   // after initialization finishes.
   Expr *Init = I->getInit();
   if (Init) {
-    if (FieldDecl *FD = I->getAnyMember())
-      IsReference = FD->getType()->isReferenceType();
     HasTemporaries = isa<ExprWithCleanups>(Init);
 
     if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
       // Generate destructors for temporaries in initialization expression.
-      VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(),
-          IsReference);
+      VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr());
     }
   }
 
   autoCreateBlock();
   appendInitializer(Block, I);
 
   if (Init) {
     if (HasTemporaries) {
CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
   VarDecl *VD = dyn_cast<VarDecl>(DS->getSingleDecl());
 
   if (!VD) {
     // Of everything that can be declared in a DeclStmt, only VarDecls impact
     // runtime semantics.
     return Block;
   }
 
-  bool IsReference = false;
   bool HasTemporaries = false;
 
   // Guard static initializers under a branch.
   CFGBlock *blockAfterStaticInit = nullptr;
 
   if (BuildOpts.AddStaticInitBranches && VD->isStaticLocal()) {
     // For static variables, we need to create a branch to track
     // whether or not they are initialized.
     if (Block) {
       Succ = Block;
       Block = nullptr;
       if (badCFG)
         return nullptr;
     }
     blockAfterStaticInit = Succ;
   }
 
   // Destructors of temporaries in initialization expression should be called
   // after initialization finishes.
   Expr *Init = VD->getInit();
   if (Init) {
-    IsReference = VD->getType()->isReferenceType();
     HasTemporaries = isa<ExprWithCleanups>(Init);
 
     if (BuildOpts.AddTemporaryDtors && HasTemporaries) {
       // Generate destructors for temporaries in initialization expression.
-      VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr(),
-          IsReference);
+      VisitForTemporaryDtors(cast<ExprWithCleanups>(Init)->getSubExpr());
     }
   }
 
   autoCreateBlock();
   appendStmt(Block, DS);
   
   // Keep track of the last non-null block, as 'Block' can be nulled out
   // if the initializer expression is something like a 'while' in a
case Stmt::ConditionalOperatorClass:
       return VisitConditionalOperatorForTemporaryDtors(
           cast<AbstractConditionalOperator>(E), BindToTemporary);
 
     case Stmt::ImplicitCastExprClass:
       // For implicit cast we want BindToTemporary to be passed further.
       E = cast<CastExpr>(E)->getSubExpr();
       goto tryAgain;
 
+    case Stmt::CXXFunctionalCastExprClass:
+      // For functional cast we want BindToTemporary to be passed further.
+      E = cast<CXXFunctionalCastExpr>(E)->getSubExpr();
+      goto tryAgain;
+
     case Stmt::ParenExprClass:
       E = cast<ParenExpr>(E)->getSubExpr();
       goto tryAgain;
 
-    case Stmt::MaterializeTemporaryExprClass:
-      E = cast<MaterializeTemporaryExpr>(E)->GetTemporaryExpr();
+    case Stmt::MaterializeTemporaryExprClass: {
+      const MaterializeTemporaryExpr* MTE = cast<MaterializeTemporaryExpr>(E);
+      BindToTemporary = (MTE->getStorageDuration() != SD_FullExpression);
+      SmallVector<const Expr *, 2> CommaLHSs;
+      SmallVector<SubobjectAdjustment, 2> Adjustments;
+      // Find the expression whose lifetime needs to be extended.
+      E = const_cast<Expr *>(
+          cast<MaterializeTemporaryExpr>(E)
+              ->GetTemporaryExpr()
+              ->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments));
+      // Visit the skipped comma operator left-hand sides for other temporaries.
+      for (const Expr *CommaLHS : CommaLHSs) {
+        VisitForTemporaryDtors(const_cast<Expr *>(CommaLHS),
+                               /*BindToTemporary=*/false);
+      }
       goto tryAgain;
+    }
 
     case Stmt::BlockExprClass:
       // Don't recurse into blocks; their subexpressions don't get evaluated
       // here.
       return Block;
 
     case Stmt::LambdaExprClass: {
       // For lambda expressions, only recurse into the capture initializers,

test/Analysis/cfg.cpp

-// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -std=c++11 %s > %t 2>&1
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12 -analyzer-config cfg-temporary-dtors=true -std=c++11 %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 // CHECK-LABEL: void checkWrap(int i)
 // CHECK: ENTRY
 // CHECK-NEXT: Succs (1): B1
 // CHECK: [B1]
 // CHECK: Succs (21): B2 B3 B4 B5 B6 B7 B8 B9
 // CHECK: B10 B11 B12 B13 B14 B15 B16 B17 B18 B19

 // CHECK-NEXT:  Preds (1): B1
 
 void test_placement_new_array() {
   int buffer[16];
   MyClass* obj = new (buffer) MyClass[5];
 }
 
 
+// CHECK-LABEL: void test_lifetime_extended_temporaries()
+// CHECK: [B1]
+struct LifetimeExtend { LifetimeExtend(int); ~LifetimeExtend(); };
+struct Aggregate { const LifetimeExtend a; const LifetimeExtend b; };
+struct AggregateRef { const LifetimeExtend &a; const LifetimeExtend &b; };
+void test_lifetime_extended_temporaries() {
+  // CHECK: LifetimeExtend(1);
+  // CHECK-NEXT: : 1
+  // CHECK-NEXT: ~LifetimeExtend()
+  // CHECK-NOT: ~LifetimeExtend()
+  {
+    const LifetimeExtend &l = LifetimeExtend(1);
+    1;
+  }
+  // CHECK: LifetimeExtend(2)
+  // CHECK-NEXT: ~LifetimeExtend()
+  // CHECK-NEXT: : 2
+  // CHECK-NOT: ~LifetimeExtend()
+  {
+    // No life-time extension.
+    const int &l = (LifetimeExtend(2), 2);
+    2;
+  }
+  // CHECK: LifetimeExtend(3)
+  // CHECK-NEXT: : 3
+  // CHECK-NEXT: ~LifetimeExtend()
+  // CHECK-NOT: ~LifetimeExtend()
+  {
+    // The last one is lifetime extended.
+    const LifetimeExtend &l = (3, LifetimeExtend(3));
+    3;
+  }
+  // CHECK: LifetimeExtend(4)
+  // CHECK-NEXT: ~LifetimeExtend()
+  // CHECK-NEXT: ~LifetimeExtend()
+  // CHECK-NEXT: : 4
+  // CHECK-NOT: ~LifetimeExtend()
+  {
+    Aggregate a{LifetimeExtend(4), LifetimeExtend(4)};
+    4;
+  }
+  // CHECK: LifetimeExtend(5)
+  // CHECK-NEXT: : 5
+  // FIXME: We want to emit the destructors of the lifetime
+  // extended variables here.
+  // CHECK-NOT: ~LifetimeExtend()
+  {
+    AggregateRef a{LifetimeExtend(5), LifetimeExtend(5)};
+    5;
+  }
+  // FIXME: Add tests for lifetime extension via subobject
+  // references (LifetimeExtend().some_member).
+}
+
+
 // CHECK-LABEL: int *PR18472()
 // CHECK: [B2 (ENTRY)]
 // CHECK-NEXT:   Succs (1): B1
 // CHECK: [B1]
 // CHECK-NEXT:   1: 0
 // CHECK-NEXT:   2: [B1.1] (ImplicitCastExpr, NullToPointer, PR18472_t)
 // CHECK-NEXT:   3: (PR18472_t)[B1.2] (CStyleCastExpr, NoOp, PR18472_t)
 // CHECK-NEXT:   4: CFGNewAllocator(int *)