[analyzer] Add support for pre-C++17 copy elision.

r335795 adds copy elision information to CFG. This commit allows static analyzer
to elide elidable copy constructors by constructing the objects that were
previously subject to elidable copy directly in the target region of the copy.

The chain of elided constructors may potentially be indefinitely long. This
only happens when the object is being returned from a function which in turn is
returned from another function, etc.

NRVO is not supported yet.

Differential Revision: https://reviews.llvm.org/D47671


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@335800 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h

@@ -780,9 +780,30 @@ private:
       llvm::PointerUnion<const Stmt *, const CXXCtorInitializer *> P,
       const LocationContext *LC);
 
+  /// If the given expression corresponds to a temporary that was used for
+  /// passing into an elidable copy/move constructor and that constructor
+  /// was actually elided, track that we also need to elide the destructor.
+  static ProgramStateRef elideDestructor(ProgramStateRef State,
+                                         const CXXBindTemporaryExpr *BTE,
+                                         const LocationContext *LC);
+
+  /// Stop tracking the destructor that corresponds to an elided constructor.
+  static ProgramStateRef
+  cleanupElidedDestructor(ProgramStateRef State,
+                          const CXXBindTemporaryExpr *BTE,
+                          const LocationContext *LC);
+
+  /// Returns true if the given expression corresponds to a temporary that
+  /// was constructed for passing into an elidable copy/move constructor
+  /// and that constructor was actually elided.
+  static bool isDestructorElided(ProgramStateRef State,
+                                 const CXXBindTemporaryExpr *BTE,
+                                 const LocationContext *LC);
+
   /// Check if all objects under construction have been fully constructed
   /// for the given context range (including FromLC, not including ToLC).
-  /// This is useful for assertions.
+  /// This is useful for assertions. Also checks if elided destructors
+  /// were cleaned up.
   static bool areAllObjectsFullyConstructed(ProgramStateRef State,
                                             const LocationContext *FromLC,
                                             const LocationContext *ToLC);

lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -139,7 +139,8 @@ public:
     // encountered. This list may be expanded when new actions are implemented.
     assert(getCXXCtorInitializer() || isa<DeclStmt>(getStmt()) ||
            isa<CXXNewExpr>(getStmt()) || isa<CXXBindTemporaryExpr>(getStmt()) ||
-           isa<MaterializeTemporaryExpr>(getStmt()));
+           isa<MaterializeTemporaryExpr>(getStmt()) ||
+           isa<CXXConstructExpr>(getStmt()));
   }
 
   const Stmt *getStmt() const {
@@ -183,6 +184,14 @@ typedef llvm::ImmutableMap<ConstructedObjectKey, SVal>
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction,
                                  ObjectsUnderConstructionMap)
 
+// Additionally, track a set of destructors that correspond to elided
+// constructors when copy elision occurs.
+typedef std::pair<const CXXBindTemporaryExpr *, const LocationContext *>
+    ElidedDestructorItem;
+typedef llvm::ImmutableSet<ElidedDestructorItem>
+    ElidedDestructorSet;
+REGISTER_TRAIT_WITH_PROGRAMSTATE(ElidedDestructors,
+                                 ElidedDestructorSet);
 
 //===----------------------------------------------------------------------===//
 // Engine construction and deletion.
@@ -358,14 +367,12 @@ ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
   // a new temporary region out of thin air and copy the contents of the object
   // (which are currently present in the Environment, because Init is an rvalue)
   // into that region. This is not correct, but it is better than nothing.
-  bool FoundOriginalMaterializationRegion = false;
   const TypedValueRegion *TR = nullptr;
   if (const auto *MT = dyn_cast<MaterializeTemporaryExpr>(Result)) {
     if (Optional<SVal> V = getObjectUnderConstruction(State, MT, LC)) {
-      FoundOriginalMaterializationRegion = true;
-      TR = cast<CXXTempObjectRegion>(V->getAsRegion());
-      assert(TR);
       State = finishObjectConstruction(State, MT, LC);
+      State = State->BindExpr(Result, LC, *V);
+      return State;
     } else {
       StorageDuration SD = MT->getStorageDuration();
       // If this object is bound to a reference with static storage duration, we
@@ -402,35 +409,33 @@ ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
     }
   }
 
-  if (!FoundOriginalMaterializationRegion) {
-    // What remains is to copy the value of the object to the new region.
-    // FIXME: In other words, what we should always do is copy value of the
-    // Init expression (which corresponds to the bigger object) to the whole
-    // temporary region TR. However, this value is often no longer present
-    // in the Environment. If it has disappeared, we instead invalidate TR.
-    // Still, what we can do is assign the value of expression Ex (which
-    // corresponds to the sub-object) to the TR's sub-region Reg. At least,
-    // values inside Reg would be correct.
-    SVal InitVal = State->getSVal(Init, LC);
-    if (InitVal.isUnknown()) {
-      InitVal = getSValBuilder().conjureSymbolVal(Result, LC, Init->getType(),
-                                                  currBldrCtx->blockCount());
-      State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false);
-
-      // Then we'd need to take the value that certainly exists and bind it
-      // over.
-      if (InitValWithAdjustments.isUnknown()) {
-        // Try to recover some path sensitivity in case we couldn't
-        // compute the value.
-        InitValWithAdjustments = getSValBuilder().conjureSymbolVal(
-            Result, LC, InitWithAdjustments->getType(),
-            currBldrCtx->blockCount());
-      }
-      State =
-          State->bindLoc(Reg.castAs<Loc>(), InitValWithAdjustments, LC, false);
-    } else {
-      State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false);
+  // What remains is to copy the value of the object to the new region.
+  // FIXME: In other words, what we should always do is copy value of the
+  // Init expression (which corresponds to the bigger object) to the whole
+  // temporary region TR. However, this value is often no longer present
+  // in the Environment. If it has disappeared, we instead invalidate TR.
+  // Still, what we can do is assign the value of expression Ex (which
+  // corresponds to the sub-object) to the TR's sub-region Reg. At least,
+  // values inside Reg would be correct.
+  SVal InitVal = State->getSVal(Init, LC);
+  if (InitVal.isUnknown()) {
+    InitVal = getSValBuilder().conjureSymbolVal(Result, LC, Init->getType(),
+                                                currBldrCtx->blockCount());
+    State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false);
+
+    // Then we'd need to take the value that certainly exists and bind it
+    // over.
+    if (InitValWithAdjustments.isUnknown()) {
+      // Try to recover some path sensitivity in case we couldn't
+      // compute the value.
+      InitValWithAdjustments = getSValBuilder().conjureSymbolVal(
+          Result, LC, InitWithAdjustments->getType(),
+          currBldrCtx->blockCount());
     }
+    State =
+        State->bindLoc(Reg.castAs<Loc>(), InitValWithAdjustments, LC, false);
+  } else {
+    State = State->bindLoc(BaseReg.castAs<Loc>(), InitVal, LC, false);
   }
 
   // The result expression would now point to the correct sub-region of the
@@ -438,10 +443,8 @@ ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
   // correctly in case (Result == Init).
   State = State->BindExpr(Result, LC, Reg);
 
-  if (!FoundOriginalMaterializationRegion) {
-    // Notify checkers once for two bindLoc()s.
-    State = processRegionChange(State, TR, LC);
-  }
+  // Notify checkers once for two bindLoc()s.
+  State = processRegionChange(State, TR, LC);
 
   return State;
 }
@@ -475,6 +478,30 @@ ProgramStateRef ExprEngine::finishObjectConstruction(
   return State->remove<ObjectsUnderConstruction>(Key);
 }
 
+ProgramStateRef ExprEngine::elideDestructor(ProgramStateRef State,
+                                            const CXXBindTemporaryExpr *BTE,
+                                            const LocationContext *LC) {
+  ElidedDestructorItem I(BTE, LC);
+  assert(!State->contains<ElidedDestructors>(I));
+  return State->add<ElidedDestructors>(I);
+}
+
+ProgramStateRef
+ExprEngine::cleanupElidedDestructor(ProgramStateRef State,
+                                    const CXXBindTemporaryExpr *BTE,
+                                    const LocationContext *LC) {
+  ElidedDestructorItem I(BTE, LC);
+  assert(State->contains<ElidedDestructors>(I));
+  return State->remove<ElidedDestructors>(I);
+}
+
+bool ExprEngine::isDestructorElided(ProgramStateRef State,
+                                    const CXXBindTemporaryExpr *BTE,
+                                    const LocationContext *LC) {
+  ElidedDestructorItem I(BTE, LC);
+  return State->contains<ElidedDestructors>(I);
+}
+
 bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State,
                                                const LocationContext *FromLC,
                                                const LocationContext *ToLC) {
@@ -485,6 +512,10 @@ bool ExprEngine::areAllObjectsFullyConstructed(ProgramStateRef State,
       if (I.first.getLocationContext() == LC)
         return false;
 
+    for (auto I: State->get<ElidedDestructors>())
+      if (I.second == LC)
+        return false;
+
     LC = LC->getParent();
   }
   return true;
@@ -529,6 +560,14 @@ static void printObjectsUnderConstructionForContext(raw_ostream &Out,
     Key.print(Out, nullptr, PP);
     Out << " : " << Value << NL;
   }
+
+  for (auto I : State->get<ElidedDestructors>()) {
+    if (I.second != LC)
+      continue;
+    Out << '(' << I.second << ',' << (const void *)I.first << ") ";
+    I.first->printPretty(Out, nullptr, PP);
+    Out << " : (constructor elided)" << NL;
+  }
 }
 
 void ExprEngine::printState(raw_ostream &Out, ProgramStateRef State,
@@ -1003,10 +1042,11 @@ void ExprEngine::ProcessMemberDtor(const CFGMemberDtor D,
 void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
                                       ExplodedNode *Pred,
                                       ExplodedNodeSet &Dst) {
-  ExplodedNodeSet CleanDtorState;
-  StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx);
+  const CXXBindTemporaryExpr *BTE = D.getBindTemporaryExpr();
   ProgramStateRef State = Pred->getState();
+  const LocationContext *LC = Pred->getLocationContext();
   const MemRegion *MR = nullptr;
+
   if (Optional<SVal> V =
           getObjectUnderConstruction(State, D.getBindTemporaryExpr(),
                                      Pred->getLocationContext())) {
@@ -1017,6 +1057,21 @@ void ExprEngine::ProcessTemporaryDtor(const CFGTemporaryDtor D,
                                      Pred->getLocationContext());
     MR = V->getAsRegion();
   }
+
+  // If copy elision has occured, and the constructor corresponding to the
+  // destructor was elided, we need to skip the destructor as well.
+  if (isDestructorElided(State, BTE, LC)) {
+    State = cleanupElidedDestructor(State, BTE, LC);
+    NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+    PostImplicitCall PP(D.getDestructorDecl(getContext()),
+                        D.getBindTemporaryExpr()->getLocStart(),
+                        Pred->getLocationContext());
+    Bldr.generateNode(PP, State, Pred);
+    return;
+  }
+
+  ExplodedNodeSet CleanDtorState;
+  StmtNodeBuilder StmtBldr(Pred, CleanDtorState, *currBldrCtx);
   StmtBldr.generateNode(D.getBindTemporaryExpr(), Pred, State);
 
   QualType T = D.getBindTemporaryExpr()->getSubExpr()->getType();
@@ -2201,8 +2256,21 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext& BC,
           // it must be a separate problem.
           assert(isa<CXXBindTemporaryExpr>(I.first.getStmt()));
           State = State->remove<ObjectsUnderConstruction>(I.first);
+          // Also cleanup the elided destructor info.
+          ElidedDestructorItem Item(
+              cast<CXXBindTemporaryExpr>(I.first.getStmt()),
+              I.first.getLocationContext());
+          State = State->remove<ElidedDestructors>(Item);
         }
 
+      // Also suppress the assertion for elided destructors when temporary
+      // destructors are not provided at all by the CFG, because there's no
+      // good place to clean them up.
+      if (!AMgr.getAnalyzerOptions().includeTemporaryDtorsInCFG())
+        for (auto I : State->get<ElidedDestructors>())
+          if (I.second == LC)
+            State = State->remove<ElidedDestructors>(I);
+
       LC = LC->getParent();
     }
     if (State != Pred->getState()) {

lib/StaticAnalyzer/Core/ExprEngineCXX.cpp

@@ -209,12 +209,37 @@ std::pair<ProgramStateRef, SVal> ExprEngine::prepareForObjectConstruction(
       }
       llvm_unreachable("Unhandled return value construction context!");
     }
-    case ConstructionContext::ElidedTemporaryObjectKind:
+    case ConstructionContext::ElidedTemporaryObjectKind: {
       assert(AMgr.getAnalyzerOptions().shouldElideConstructors());
-      // FALL-THROUGH
+      const auto *TCC = cast<ElidedTemporaryObjectConstructionContext>(CC);
+      const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
+      const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
+      const CXXConstructExpr *CE = TCC->getConstructorAfterElision();
+
+      // Support pre-C++17 copy elision. We'll have the elidable copy
+      // constructor in the AST and in the CFG, but we'll skip it
+      // and construct directly into the final object. This call
+      // also sets the CallOpts flags for us.
+      SVal V;
+      std::tie(State, V) = prepareForObjectConstruction(
+          CE, State, LCtx, TCC->getConstructionContextAfterElision(), CallOpts);
+
+      // Remember that we've elided the constructor.
+      State = addObjectUnderConstruction(State, CE, LCtx, V);
+
+      // Remember that we've elided the destructor.
+      if (BTE)
+        State = elideDestructor(State, BTE, LCtx);
+
+      // Instead of materialization, shamelessly return
+      // the final object destination.
+      if (MTE)
+        State = addObjectUnderConstruction(State, MTE, LCtx, V);
+
+      return std::make_pair(State, V);
+    }
     case ConstructionContext::SimpleTemporaryObjectKind: {
-      // TODO: Copy elision implementation goes here.
-      const auto *TCC = cast<TemporaryObjectConstructionContext>(CC);
+      const auto *TCC = cast<SimpleTemporaryObjectConstructionContext>(CC);
       const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
       const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
       SVal V = UnknownVal();
@@ -266,6 +291,20 @@ void ExprEngine::VisitCXXConstructExpr(const CXXConstructExpr *CE,
 
   SVal Target = UnknownVal();
 
+  if (Optional<SVal> ElidedTarget =
+          getObjectUnderConstruction(State, CE, LCtx)) {
+    // We've previously modeled an elidable constructor by pretending that it in
+    // fact constructs into the correct target. This constructor can therefore
+    // be skipped.
+    Target = *ElidedTarget;
+    StmtNodeBuilder Bldr(Pred, destNodes, *currBldrCtx);
+    State = finishObjectConstruction(State, CE, LCtx);
+    if (auto L = Target.getAs<Loc>())
+      State = State->BindExpr(CE, LCtx, State->getSVal(*L, CE->getType()));
+    Bldr.generateNode(CE, Pred, State);
+    return;
+  }
+
   // FIXME: Handle arrays, which run the same constructor for every element.
   // For now, we just run the first constructor (which should still invalidate
   // the entire array).

test/Analysis/cxx17-mandatory-elision.cpp

@@ -1,5 +1,17 @@
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -verify %s
 // RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++11 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -std=c++17 -analyzer-config elide-constructors=false -DNO_ELIDE_FLAG -verify %s
+
+// Copy elision always occurs in C++17, otherwise it's under
+// an on-by-default flag.
+#if __cplusplus >= 201703L
+  #define ELIDE 1
+#else
+  #ifndef NO_ELIDE_FLAG
+    #define ELIDE 1
+  #endif
+#endif
 
 void clang_analyzer_eval(bool);
 
@@ -39,9 +51,9 @@ public:
   C() : t(T(4)) {
     S s = {1, 2, 3};
     t.s = s;
-    // FIXME: Should be TRUE in C++11 as well.
+    // FIXME: Should be TRUE regardless of copy elision.
     clang_analyzer_eval(t.w == 4);
-#if __cplusplus >= 201703L
+#ifdef ELIDE
     // expected-warning@-2{{TRUE}}
 #else
     // expected-warning@-4{{UNKNOWN}}
@@ -149,7 +161,7 @@ void testMultipleReturns() {
   AddressVector<ClassWithoutDestructor> v;
   ClassWithoutDestructor c = make3(v);
 
-#if __cplusplus >= 201703L
+#if ELIDE
   clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
   clang_analyzer_eval(v.buf[0] == &c); // expected-warning{{TRUE}}
 #else
@@ -184,13 +196,13 @@ void testVariable() {
     ClassWithDestructor c = ClassWithDestructor(v);
     // Check if the last destructor is an automatic destructor.
     // A temporary destructor would have fired by now.
-#if __cplusplus >= 201703L
+#if ELIDE
     clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
 #else
     clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}}
 #endif
   }
-#if __cplusplus >= 201703L
+#if ELIDE
   // 0. Construct the variable.
   // 1. Destroy the variable.
   clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
@@ -218,13 +230,13 @@ void testCtorInitializer() {
     TestCtorInitializer t(v);
     // Check if the last destructor is an automatic destructor.
     // A temporary destructor would have fired by now.
-#if __cplusplus >= 201703L
+#if ELIDE
     clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
 #else
     clang_analyzer_eval(v.len == 3); // expected-warning{{TRUE}}
 #endif
   }
-#if __cplusplus >= 201703L
+#if ELIDE
   // 0. Construct the member variable.
   // 1. Destroy the member variable.
   clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
@@ -257,14 +269,14 @@ void testMultipleReturnsWithDestructors() {
     ClassWithDestructor c = make3(v);
     // Check if the last destructor is an automatic destructor.
     // A temporary destructor would have fired by now.
-#if __cplusplus >= 201703L
+#if ELIDE
     clang_analyzer_eval(v.len == 1); // expected-warning{{TRUE}}
 #else
     clang_analyzer_eval(v.len == 9); // expected-warning{{TRUE}}
 #endif
   }
 
-#if __cplusplus >= 201703L
+#if ELIDE
   // 0. Construct the variable. Yes, constructor in make1() constructs
   //    the variable 'c'.
   // 1. Destroy the variable.

test/Analysis/gtest.cpp

@@ -154,13 +154,11 @@ void testConstrainState(int p) {
 void testAssertSymbolicPtr(const bool *b) {
   ASSERT_TRUE(*b); // no-crash
 
-  // FIXME: Our solver doesn't handle this well yet.
-  clang_analyzer_eval(*b); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(*b); // expected-warning{{TRUE}}
 }
 
 void testAssertSymbolicRef(const bool &b) {
   ASSERT_TRUE(b); // no-crash
 
-  // FIXME: Our solver doesn't handle this well yet.
-  clang_analyzer_eval(b); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(b); // expected-warning{{TRUE}}
 }

test/Analysis/inlining/temp-dtors-path-notes.cpp

@@ -30,19 +30,14 @@ public:
 };
 
 void test(int coin) {
-  // We'd divide by zero in the temporary destructor that goes after the
-  // elidable copy-constructor from C(0) to the lifetime-extended temporary.
-  // So in fact this example has nothing to do with lifetime extension.
-  // Actually, it would probably be better to elide the constructor, and
-  // put the note for the destructor call at the closing brace after nop.
+  // We'd divide by zero in the automatic destructor for variable 'c'.
   const C &c = coin ? C(1) : C(0); // expected-note   {{Assuming 'coin' is 0}}
                                    // expected-note@-1{{'?' condition is false}}
                                    // expected-note@-2{{Passing the value 0 via 1st parameter 'x'}}
                                    // expected-note@-3{{Calling constructor for 'C'}}
                                    // expected-note@-4{{Returning from constructor for 'C'}}
-                                   // expected-note@-5{{Calling '~C'}}
   c.nop();
-}
+} // expected-note{{Calling '~C'}}
 } // end namespace test_lifetime_extended_temporary
 
 namespace test_bug_after_dtor {

test/Analysis/lifetime-extension.cpp

@@ -97,13 +97,10 @@ void f1() {
 
 void f2() {
   C *after, *before;
-  C c = C(1, &after, &before);
-  clang_analyzer_eval(after == before);
-#ifdef TEMPORARIES
-  // expected-warning@-2{{TRUE}}
-#else
-  // expected-warning@-4{{UNKNOWN}}
-#endif
+  {
+    C c = C(1, &after, &before);
+  }
+  clang_analyzer_eval(after == before); // expected-warning{{TRUE}}
 }
 
 void f3(bool coin) {
@@ -129,6 +126,7 @@ void f4(bool coin) {
   // operator. Ideally also add support for the binary conditional operator in
   // C++. Because for now it calls the constructor for the condition twice.
   if (coin) {
+    // FIXME: Should not warn.
     clang_analyzer_eval(after == before);
 #ifdef TEMPORARIES
   // expected-warning@-2{{The left operand of '==' is a garbage value}}
@@ -136,13 +134,12 @@ void f4(bool coin) {
   // expected-warning@-4{{UNKNOWN}}
 #endif
   } else {
+    // FIXME: Should be TRUE.
     clang_analyzer_eval(after == before);
 #ifdef TEMPORARIES
-    // Seems to work at the moment, but also seems accidental.
-    // Feel free to break.
-  // expected-warning@-4{{TRUE}}
+  // expected-warning@-2{{FALSE}}
 #else
-  // expected-warning@-6{{UNKNOWN}}
+  // expected-warning@-4{{UNKNOWN}}
 #endif
   }
 }
@@ -234,24 +231,25 @@ void f2() {
 } // end namespace maintain_original_object_address_on_move
 
 namespace maintain_address_of_copies {
+class C;
 
-template <typename T> struct AddressVector {
-  const T *buf[10];
+struct AddressVector {
+  C *buf[10];
   int len;
 
   AddressVector() : len(0) {}
 
-  void push(const T *t) {
-    buf[len] = t;
+  void push(C *c) {
+    buf[len] = c;
     ++len;
   }
 };
 
 class C {
-  AddressVector<C> &v;
+  AddressVector &v;
 
 public:
-  C(AddressVector<C> &v) : v(v) { v.push(this); }
+  C(AddressVector &v) : v(v) { v.push(this); }
   ~C() { v.push(this); }
 
 #ifdef MOVES
@@ -267,132 +265,70 @@ public:
 #endif
   } // no-warning
 
-  static C make(AddressVector<C> &v) { return C(v); }
+  static C make(AddressVector &v) { return C(v); }
 };
 
 void f1() {
-  AddressVector<C> v;
+  AddressVector v;
   {
     C c = C(v);
   }
-  // 0. Create the original temporary and lifetime-extend it into variable 'c'
-  //    construction argument.
-  // 1. Construct variable 'c' (elidable copy/move).
-  // 2. Destroy the temporary.
-  // 3. Destroy variable 'c'.
-  clang_analyzer_eval(v.len == 4);
-  clang_analyzer_eval(v.buf[0] == v.buf[2]);
-  clang_analyzer_eval(v.buf[1] == v.buf[3]);
-#ifdef TEMPORARIES
-  // expected-warning@-4{{TRUE}}
-  // expected-warning@-4{{TRUE}}
-  // expected-warning@-4{{TRUE}}
-#else
-  // expected-warning@-8{{UNKNOWN}}
-  // expected-warning@-8{{UNKNOWN}}
-  // expected-warning@-8{{UNKNOWN}}
-#endif
+  // 0. Construct variable 'c' (copy/move elided).
+  // 1. Destroy variable 'c'.
+  clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
 }
 
 void f2() {
-  AddressVector<C> v;
+  AddressVector v;
   {
     const C &c = C::make(v);
   }
-  // 0. Construct the original temporary within make(),
-  // 1. Construct the return value of make() (elidable copy/move) and
-  //    lifetime-extend it via reference 'c',
-  // 2. Destroy the temporary within make(),
-  // 3. Destroy the temporary lifetime-extended by 'c'.
-  clang_analyzer_eval(v.len == 4);
-  clang_analyzer_eval(v.buf[0] == v.buf[2]);
-  clang_analyzer_eval(v.buf[1] == v.buf[3]);
+  // 0. Construct the return value of make() (copy/move elided) and
+  //    lifetime-extend it directly via reference 'c',
+  // 1. Destroy the temporary lifetime-extended by 'c'.
+  clang_analyzer_eval(v.len == 2);
+  clang_analyzer_eval(v.buf[0] == v.buf[1]);
 #ifdef TEMPORARIES
-  // expected-warning@-4{{TRUE}}
-  // expected-warning@-4{{TRUE}}
-  // expected-warning@-4{{TRUE}}
+  // expected-warning@-3{{TRUE}}
+  // expected-warning@-3{{TRUE}}
 #else
-  // expected-warning@-8{{UNKNOWN}}
-  // expected-warning@-8{{UNKNOWN}}
-  // expected-warning@-8{{UNKNOWN}}
+  // expected-warning@-6{{UNKNOWN}}
+  // expected-warning@-6{{UNKNOWN}}
 #endif
 }
 
 void f3() {
-  AddressVector<C> v;
+  AddressVector v;
   {
     C &&c = C::make(v);
   }
-  // 0. Construct the original temporary within make(),
-  // 1. Construct the return value of make() (elidable copy/move) and
-  //    lifetime-extend it via reference 'c',
-  // 2. Destroy the temporary within make(),
-  // 3. Destroy the temporary lifetime-extended by 'c'.
-  clang_analyzer_eval(v.len == 4);
-  clang_analyzer_eval(v.buf[0] == v.buf[2]);
-  clang_analyzer_eval(v.buf[1] == v.buf[3]);
+  // 0. Construct the return value of make() (copy/move elided) and
+  //    lifetime-extend it directly via reference 'c',
+  // 1. Destroy the temporary lifetime-extended by 'c'.
+  clang_analyzer_eval(v.len == 2);
+  clang_analyzer_eval(v.buf[0] == v.buf[1]);
 #ifdef TEMPORARIES
-  // expected-warning@-4{{TRUE}}
-  // expected-warning@-4{{TRUE}}
-  // expected-warning@-4{{TRUE}}
+  // expected-warning@-3{{TRUE}}
+  // expected-warning@-3{{TRUE}}
 #else
-  // expected-warning@-8{{UNKNOWN}}
-  // expected-warning@-8{{UNKNOWN}}
-  // expected-warning@-8{{UNKNOWN}}
+  // expected-warning@-6{{UNKNOWN}}
+  // expected-warning@-6{{UNKNOWN}}
 #endif
 }
 
-C doubleMake(AddressVector<C> &v) {
+C doubleMake(AddressVector &v) {
   return C::make(v);
 }
 
 void f4() {
-  AddressVector<C> v;
+  AddressVector v;
   {
     C c = doubleMake(v);
   }
-  // 0. Construct the original temporary within make(),
-  // 1. Construct the return value of make() (elidable copy/move) and
-  //    lifetime-extend it into the return value constructor argument within
-  //    doubleMake(),
-  // 2. Destroy the temporary within make(),
-  // 3. Construct the return value of doubleMake() (elidable copy/move) and
-  //    lifetime-extend it into the variable 'c' constructor argument,
-  // 4. Destroy the return value of make(),
-  // 5. Construct variable 'c' (elidable copy/move),
-  // 6. Destroy the return value of doubleMake(),
-  // 7. Destroy variable 'c'.
-  clang_analyzer_eval(v.len == 8);
-  clang_analyzer_eval(v.buf[0] == v.buf[2]);
-  clang_analyzer_eval(v.buf[1] == v.buf[4]);
-  clang_analyzer_eval(v.buf[3] == v.buf[6]);
-  clang_analyzer_eval(v.buf[5] == v.buf[7]);
-#ifdef TEMPORARIES
-  // expected-warning@-6{{TRUE}}
-  // expected-warning@-6{{TRUE}}
-  // expected-warning@-6{{TRUE}}
-  // expected-warning@-6{{TRUE}}
-  // expected-warning@-6{{TRUE}}
-#else
-  // expected-warning@-12{{UNKNOWN}}
-  // expected-warning@-12{{UNKNOWN}}
-  // expected-warning@-12{{UNKNOWN}}
-  // expected-warning@-12{{UNKNOWN}}
-  // expected-warning@-12{{UNKNOWN}}
-#endif
+  // 0. Construct variable 'c' (all copies/moves elided),
+  // 1. Destroy variable 'c'.
+  clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}}
 }
-
-class NoDtor {
-  AddressVector<NoDtor> &v;
-
-public:
-  NoDtor(AddressVector<NoDtor> &v) : v(v) { v.push(this); }
-};
-
-void f5() {
-  AddressVector<NoDtor> v;
-  const NoDtor &N = NoDtor(v);
-  clang_analyzer_eval(v.buf[0] == &N); // expected-warning{{TRUE}}
-}
-
 } // end namespace maintain_address_of_copies

test/Analysis/temporaries.cpp

@@ -612,107 +612,44 @@ void test() {
   clang_analyzer_eval(c3.getY() == 2); // expected-warning{{TRUE}}
 
   C c4 = returnTemporaryWithConstruction();
-  clang_analyzer_eval(c4.getX() == 1);
-  clang_analyzer_eval(c4.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(c4.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(c4.getY() == 2); // expected-warning{{TRUE}}
 
   C c5 = returnTemporaryWithAnotherFunctionWithConstruction();
-  clang_analyzer_eval(c5.getX() == 1);
-  clang_analyzer_eval(c5.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(c5.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(c5.getY() == 2); // expected-warning{{TRUE}}
 
   C c6 = returnTemporaryWithCopyConstructionWithConstruction();
-  clang_analyzer_eval(c5.getX() == 1);
-  clang_analyzer_eval(c5.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(c5.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(c5.getY() == 2); // expected-warning{{TRUE}}
 
 #if __cplusplus >= 201103L
 
   C c7 = returnTemporaryWithBraces();
-  clang_analyzer_eval(c7.getX() == 1);
-  clang_analyzer_eval(c7.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(c7.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(c7.getY() == 2); // expected-warning{{TRUE}}
 
   C c8 = returnTemporaryWithAnotherFunctionWithBraces();
-  clang_analyzer_eval(c8.getX() == 1);
-  clang_analyzer_eval(c8.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(c8.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(c8.getY() == 2); // expected-warning{{TRUE}}
 
   C c9 = returnTemporaryWithCopyConstructionWithBraces();
-  clang_analyzer_eval(c9.getX() == 1);
-  clang_analyzer_eval(c9.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(c9.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(c9.getY() == 2); // expected-warning{{TRUE}}
 
 #endif // C++11
 
   D d1 = returnTemporaryWithVariableAndNonTrivialCopy();
-  clang_analyzer_eval(d1.getX() == 1);
-  clang_analyzer_eval(d1.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(d1.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d1.getY() == 2); // expected-warning{{TRUE}}
 
   D d2 = returnTemporaryWithAnotherFunctionWithVariableAndNonTrivialCopy();
-  clang_analyzer_eval(d2.getX() == 1);
-  clang_analyzer_eval(d2.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(d2.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d2.getY() == 2); // expected-warning{{TRUE}}
 
   D d3 = returnTemporaryWithCopyConstructionWithVariableAndNonTrivialCopy();
-  clang_analyzer_eval(d3.getX() == 1);
-  clang_analyzer_eval(d3.getY() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(d3.getX() == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(d3.getY() == 2); // expected-warning{{TRUE}}
 }
 } // namespace test_return_temporary
 
@@ -767,19 +704,9 @@ void test(int coin) {
 
   C c2 = coin ? C(1) : C(2);
   if (coin) {
-    clang_analyzer_eval(c2.getX() == 1);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-2{{TRUE}}
-#else
-  // expected-warning@-4{{UNKNOWN}}
-#endif
+    clang_analyzer_eval(c2.getX() == 1); // expected-warning{{TRUE}}
   } else {
-    clang_analyzer_eval(c2.getX() == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-2{{TRUE}}
-#else
-  // expected-warning@-4{{UNKNOWN}}
-#endif
+    clang_analyzer_eval(c2.getX() == 2); // expected-warning{{TRUE}}
   }
 }
 
@@ -816,16 +743,9 @@ void test_simple_temporary_with_copy() {
   {
     C c = C(x, y);
   }
-  // Two constructors (temporary object expr and copy) and two destructors.
-  clang_analyzer_eval(x == 2);
-  clang_analyzer_eval(y == 2);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-3{{TRUE}}
-  // expected-warning@-3{{TRUE}}
-#else
-  // expected-warning@-6{{UNKNOWN}}
-  // expected-warning@-6{{UNKNOWN}}
-#endif
+  // Only one constructor directly into the variable, and one destructor.
+  clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 1); // expected-warning{{TRUE}}
 }
 
 void test_ternary_temporary(int coin) {
@@ -833,10 +753,10 @@ void test_ternary_temporary(int coin) {
   {
     const C &c = coin ? C(x, y) : C(z, w);
   }
-  // This time each branch contains an additional elidable copy constructor.
+  // Only one constructor on every branch, and one automatic destructor.
   if (coin) {
-    clang_analyzer_eval(x == 2);
-    clang_analyzer_eval(y == 2);
+    clang_analyzer_eval(x == 1);
+    clang_analyzer_eval(y == 1);
 #ifdef TEMPORARY_DTORS
     // expected-warning@-3{{TRUE}}
     // expected-warning@-3{{TRUE}}
@@ -850,8 +770,8 @@ void test_ternary_temporary(int coin) {
   } else {
     clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
     clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
-    clang_analyzer_eval(z == 2);
-    clang_analyzer_eval(w == 2);
+    clang_analyzer_eval(z == 1);
+    clang_analyzer_eval(w == 1);
 #ifdef TEMPORARY_DTORS
     // expected-warning@-3{{TRUE}}
     // expected-warning@-3{{TRUE}}
@@ -867,33 +787,18 @@ void test_ternary_temporary_with_copy(int coin) {
   {
     C c = coin ? C(x, y) : C(z, w);
   }
-  // Temporary expression, elidable copy within branch,
-  // constructor for variable - 3 total.
+  // On each branch the variable is constructed directly.
   if (coin) {
-    clang_analyzer_eval(x == 3);
-    clang_analyzer_eval(y == 3);
-#ifdef TEMPORARY_DTORS
-    // expected-warning@-3{{TRUE}}
-    // expected-warning@-3{{TRUE}}
-#else
-    // expected-warning@-6{{UNKNOWN}}
-    // expected-warning@-6{{UNKNOWN}}
-#endif
+    clang_analyzer_eval(x == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(y == 1); // expected-warning{{TRUE}}
     clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
     clang_analyzer_eval(w == 0); // expected-warning{{TRUE}}
 
   } else {
     clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
     clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
-    clang_analyzer_eval(z == 3);
-    clang_analyzer_eval(w == 3);
-#ifdef TEMPORARY_DTORS
-    // expected-warning@-3{{TRUE}}
-    // expected-warning@-3{{TRUE}}
-#else
-    // expected-warning@-6{{UNKNOWN}}
-    // expected-warning@-6{{UNKNOWN}}
-#endif
+    clang_analyzer_eval(z == 1); // expected-warning{{TRUE}}
+    clang_analyzer_eval(w == 1); // expected-warning{{TRUE}}
   }
 }
 } // namespace test_match_constructors_and_destructors
@@ -928,12 +833,13 @@ void testLifetimeExtendedCall() {
 }
 
 void testCopiedCall() {
-  C c = make();
-  // Should have divided by zero in the temporary destructor.
-  clang_analyzer_warnIfReached();
-#ifndef TEMPORARY_DTORS
-    // expected-warning@-2{{REACHABLE}}
-#endif
+  {
+    C c = make();
+    // Should have elided the constructor/destructor for the temporary
+    clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  }
+  // Should have divided by zero in the destructor.
+  clang_analyzer_warnIfReached(); // no-warning
 }
 } // namespace destructors_for_return_values
 
@@ -1018,20 +924,10 @@ void test() {
 #endif
 
   S s = 20;
-  clang_analyzer_eval(s.x == 20);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-2{{TRUE}}
-#else
-  // expected-warning@-4{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(s.x == 20); // expected-warning{{TRUE}}
 
   C c2 = s;
-  clang_analyzer_eval(c2.getX() == 20);
-#ifdef TEMPORARY_DTORS
-  // expected-warning@-2{{TRUE}}
-#else
-  // expected-warning@-4{{UNKNOWN}}
-#endif
+  clang_analyzer_eval(c2.getX() == 20); // expected-warning{{TRUE}}
 }
 } // end namespace implicit_constructor_conversion