[PATCH] D44238: [CFG] Fix automatic destructors when a member is bound to a reference.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ created this revision.
NoQ added reviewers: rsmith, doug.gregor, dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.

In code like

  const int &x = A().x;

the destructor for `A()` was not present in the CFG due to two problems in the pattern-matching in `getReferenceInitTemporaryType()`:

1. The no-op cast from `T` to `const T` is not necessarily performed on a record type. It may be performed on essentially any type. In the current example, it is performed on an xvalue of type `int` in order to turn it into a `const int`.
2. Since https://reviews.llvm.org/rC288563 member access is no longer performed over rvalues, but goes after the `MaterializeTemporaryExpr` instead.

**This is not an analyzer-specific change.** It may add/remove compiler warnings, but i don't think it makes any sense to hide this behind an analyzer-specific flag.


Repository:
  rC Clang

https://reviews.llvm.org/D44238

Files:
  lib/Analysis/CFG.cpp
  test/Analysis/auto-obj-dtors-cfg-output.cpp


Index: test/Analysis/auto-obj-dtors-cfg-output.cpp
===================================================================
--- test/Analysis/auto-obj-dtors-cfg-output.cpp
+++ test/Analysis/auto-obj-dtors-cfg-output.cpp
@@ -14,6 +14,8 @@
 
 class A {
 public:
+  int x;
+
 // CHECK:      [B1 (ENTRY)]
 // CHECK-NEXT:   Succs (1): B0
 // CHECK:      [B0 (EXIT)]
@@ -67,6 +69,25 @@
   const A& c = A();
 }
 
+// CHECK:      [B2 (ENTRY)]
+// CHECK-NEXT:   Succs (1): B1
+// CHECK:      [B1]
+// WARNINGS-NEXT:   1: A() (CXXConstructExpr, class A)
+// ANALYZER-NEXT:   1: A() (CXXConstructExpr, [B1.2], [B1.3], class A)
+// CHECK-NEXT:   2: [B1.1] (BindTemporary)
+// CHECK-NEXT:   3: [B1.2]
+// CHECK-NEXT:   4: [B1.3].x
+// CHECK-NEXT:   5: [B1.4] (ImplicitCastExpr, NoOp, const int)
+// CHECK-NEXT:   6: const int &x = A().x;
+// CHECK-NEXT:   7: [B1.6].~A() (Implicit destructor)
+// CHECK-NEXT:   Preds (1): B2
+// CHECK-NEXT:   Succs (1): B0
+// CHECK:      [B0 (EXIT)]
+// CHECK-NEXT:   Preds (1): B1
+void test_const_ref_to_field() {
+  const int &x = A().x;
+}
+
 // CHECK:      [B2 (ENTRY)]
 // CHECK-NEXT:   Succs (1): B1
 // CHECK:      [B1]
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -1458,16 +1458,15 @@
     if (const CastExpr *CE = dyn_cast<CastExpr>(Init)) {
       if ((CE->getCastKind() == CK_DerivedToBase ||
            CE->getCastKind() == CK_UncheckedDerivedToBase ||
-           CE->getCastKind() == CK_NoOp) &&
-          Init->getType()->isRecordType()) {
+           CE->getCastKind() == CK_NoOp)) {
         Init = CE->getSubExpr();
         continue;
       }
     }
     
-    // Skip member accesses into rvalues.
+    // Skip member accesses.
     if (const MemberExpr *ME = dyn_cast<MemberExpr>(Init)) {
-      if (!ME->isArrow() && ME->getBase()->isRValue()) {
+      if (!ME->isArrow()) {
         Init = ME->getBase();
         continue;
       }
@@ -4873,10 +4872,12 @@
     const VarDecl *VD = DE->getVarDecl();
     Helper.handleDecl(VD, OS);
 
-    const Type* T = VD->getType().getTypePtr();
-    if (const ReferenceType* RT = T->getAs<ReferenceType>())
-      T = RT->getPointeeType().getTypePtr();
-    T = T->getBaseElementTypeUnsafe();
+    ASTContext &ACtx = VD->getASTContext();
+    QualType T = VD->getType();
+    if (T->isReferenceType())
+      T = getReferenceInitTemporaryType(ACtx, VD->getInit(), nullptr);
+    if (const ArrayType *AT = ACtx.getAsArrayType(T))
+      T = ACtx.getBaseElementType(AT);
 
     OS << ".~" << T->getAsCXXRecordDecl()->getName().str() << "()";
     OS << " (Implicit destructor)\n";


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44238.137516.patch
Type: text/x-patch
Size: 2664 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180308/910dd50b/attachment.bin>