[PATCH] D47616: [CFG] [analyzer] Explain copy elision through construction contexts.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 149566.
NoQ added a comment.

Fix comments in the callback test. The test itself is pretty pointless now, 
because the behavior that it was supposed to test was a workaround for lack of 
lifetime extension support.


https://reviews.llvm.org/D47616

Files:
  lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  test/Analysis/lifetime-extension.cpp
  test/Analysis/temporaries-callback-order.cpp


Index: test/Analysis/temporaries-callback-order.cpp
===================================================================
--- test/Analysis/temporaries-callback-order.cpp
+++ test/Analysis/temporaries-callback-order.cpp
@@ -8,11 +8,7 @@
 };
 
 void testTemporaries() {
-  // This triggers RegionChanges twice:
-  // - Once for zero-initialization of the structure.
-  // - Once for creating a temporary region and copying the structure there.
-  // FIXME: This code shouldn't really produce the extra temporary, however
-  // that's how we behave for now.
+  // This triggers RegionChanges once for zero-initialization of the structure.
   Sub().m();
 }
 
@@ -29,7 +25,6 @@
 
 // testTemporaries():
 // CHECK-NEXT: RegionChanges
-// CHECK-NEXT: RegionChanges
 
 // Make sure there's no further output.
 // CHECK-NOT: Bind
Index: test/Analysis/lifetime-extension.cpp
===================================================================
--- test/Analysis/lifetime-extension.cpp
+++ test/Analysis/lifetime-extension.cpp
@@ -234,25 +234,24 @@
 } // end namespace maintain_original_object_address_on_move
 
 namespace maintain_address_of_copies {
-class C;
 
-struct AddressVector {
-  C *buf[10];
+template <typename T> struct AddressVector {
+  const T *buf[10];
   int len;
 
   AddressVector() : len(0) {}
 
-  void push(C *c) {
-    buf[len] = c;
+  void push(const T *t) {
+    buf[len] = t;
     ++len;
   }
 };
 
 class C {
-  AddressVector &v;
+  AddressVector<C> &v;
 
 public:
-  C(AddressVector &v) : v(v) { v.push(this); }
+  C(AddressVector<C> &v) : v(v) { v.push(this); }
   ~C() { v.push(this); }
 
 #ifdef MOVES
@@ -268,11 +267,11 @@
 #endif
   } // no-warning
 
-  static C make(AddressVector &v) { return C(v); }
+  static C make(AddressVector<C> &v) { return C(v); }
 };
 
 void f1() {
-  AddressVector v;
+  AddressVector<C> v;
   {
     C c = C(v);
   }
@@ -296,7 +295,7 @@
 }
 
 void f2() {
-  AddressVector v;
+  AddressVector<C> v;
   {
     const C &c = C::make(v);
   }
@@ -320,7 +319,7 @@
 }
 
 void f3() {
-  AddressVector v;
+  AddressVector<C> v;
   {
     C &&c = C::make(v);
   }
@@ -343,12 +342,12 @@
 #endif
 }
 
-C doubleMake(AddressVector &v) {
+C doubleMake(AddressVector<C> &v) {
   return C::make(v);
 }
 
 void f4() {
-  AddressVector v;
+  AddressVector<C> v;
   {
     C c = doubleMake(v);
   }
@@ -382,4 +381,18 @@
   // expected-warning@-12{{UNKNOWN}}
 #endif
 }
+
+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
Index: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -214,12 +214,6 @@
       const CXXBindTemporaryExpr *BTE = TCC->getCXXBindTemporaryExpr();
       const MaterializeTemporaryExpr *MTE = TCC->getMaterializedTemporaryExpr();
 
-      if (!BTE) {
-        // FIXME: Lifetime extension for temporaries without destructors
-        // is not implemented yet.
-        MTE = nullptr;
-      }
-
       if (MTE) {
         if (const ValueDecl *VD = MTE->getExtendingDecl()) {
           assert(MTE->getStorageDuration() != SD_FullExpression);
@@ -234,16 +228,25 @@
         }
       }
 
-      if (MTE && MTE->getStorageDuration() != SD_FullExpression) {
-        // If the temporary is lifetime-extended, don't save the BTE,
-        // because we don't need a temporary destructor, but an automatic
-        // destructor.
-        BTE = nullptr;
+      SVal V = UnknownVal();
+      if (MTE) {
+        if (MTE->getStorageDuration() != SD_FullExpression) {
+          // If the temporary is lifetime-extended, don't save the BTE,
+          // because we don't need a temporary destructor, but an automatic
+          // destructor.
+          BTE = nullptr;
+
+          if (MTE->getStorageDuration() == SD_Static ||
+              MTE->getStorageDuration() == SD_Thread)
+            V = loc::MemRegionVal(MRMgr.getCXXStaticTempObjectRegion(E));
+        }
       }
 
+      if (V.isUnknown())
+        V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
+
       // FIXME: Support temporaries lifetime-extended via static references.
       // They'd need a getCXXStaticTempObjectRegion().
-      SVal V = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(E, LCtx));
 
       if (BTE)
         State = addObjectUnderConstruction(State, BTE, LCtx, V);

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits