From: David Majnemer <david.majnemer@gmail.com>
Date: Sun, 13 Apr 2014 02:27:32 +0000 (+0000)
Subject: MS ABI: #pragma vtordisp(0) only disables new vtordisps
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ea88434120f609dc40f9890c938b0ff8095cf8ff;p=clang

MS ABI: #pragma vtordisp(0) only disables new vtordisps

Previously, it was believed that #pragma vtordisp(0) would prohibit the
generation of any and all vtordisps.

In actuality, it only disables the generation of additional vtordisps.

This fixes PR19413.

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

diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp
index ef818bb4ad..04a0fab737 100644
--- a/lib/AST/RecordLayoutBuilder.cpp
+++ b/lib/AST/RecordLayoutBuilder.cpp
@@ -2674,10 +2674,6 @@ llvm::SmallPtrSet<const CXXRecordDecl *, 2>
 MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
   llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet;
 
-  // /vd0 or #pragma vtordisp(0): Never use vtordisps when used as a vbase.
-  if (RD->getMSVtorDispMode() == MSVtorDispAttr::Never)
-    return HasVtordispSet;
-
   // /vd2 or #pragma vtordisp(2): Always use vtordisps for virtual bases with
   // vftables.
   if (RD->getMSVtorDispMode() == MSVtorDispAttr::ForVFTable) {
@@ -2690,11 +2686,6 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
     return HasVtordispSet;
   }
 
-  // /vd1 or #pragma vtordisp(1): Try to guess based on whether we think it's
-  // possible for a partially constructed object with virtual base overrides to
-  // escape a non-trivial constructor.
-  assert(RD->getMSVtorDispMode() == MSVtorDispAttr::ForVBaseOverride);
-
   // If any of our bases need a vtordisp for this type, so do we.  Check our
   // direct bases for vtordisp requirements.
   for (const auto &I : RD->bases()) {
@@ -2704,10 +2695,16 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
       if (bi.second.hasVtorDisp())
         HasVtordispSet.insert(bi.first);
   }
-  // If we do not have a user declared constructor or destructor then we don't
-  // introduce any additional vtordisps.
-  if (!RD->hasUserDeclaredConstructor() && !RD->hasUserDeclaredDestructor())
+  // We don't introduce any additional vtordisps if either:
+  // * A user declared constructor or destructor aren't declared.
+  // * #pragma vtordisp(0) or the /vd0 flag are in use.
+  if ((!RD->hasUserDeclaredConstructor() && !RD->hasUserDeclaredDestructor()) ||
+      RD->getMSVtorDispMode() == MSVtorDispAttr::Never)
     return HasVtordispSet;
+  // /vd1 or #pragma vtordisp(1): Try to guess based on whether we think it's
+  // possible for a partially constructed object with virtual base overrides to
+  // escape a non-trivial constructor.
+  assert(RD->getMSVtorDispMode() == MSVtorDispAttr::ForVBaseOverride);
   // Compute a set of base classes which define methods we override.  A virtual
   // base in this set will require a vtordisp.  A virtual base that transitively
   // contains one of these bases as a non-virtual base will also require a
diff --git a/test/Layout/ms-x86-vtordisp.cpp b/test/Layout/ms-x86-vtordisp.cpp
index 1619d1cb3b..fd34f7b2c2 100644
--- a/test/Layout/ms-x86-vtordisp.cpp
+++ b/test/Layout/ms-x86-vtordisp.cpp
@@ -381,6 +381,41 @@ struct GD: public virtual GC, public virtual GB {};
 // CHECK-X64-NEXT:      | [sizeof=40, align=8
 // CHECK-X64-NEXT:      |  nvsize=8, nvalign=8]
 
+struct HA {
+  virtual void fun() {}
+};
+#pragma vtordisp(push, 2)
+struct HB : virtual HA {};
+#pragma vtordisp(pop, 2)
+#pragma vtordisp(push, 0)
+struct HC : virtual HB {};
+#pragma vtordisp(pop, 0)
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:    0 | struct HC
+// CHECK-NEXT:    0 |   (HC vbtable pointer)
+// CHECK-NEXT:    4 |   (vtordisp for vbase HA)
+// CHECK-NEXT:    8 |   struct HA (virtual base)
+// CHECK-NEXT:    8 |     (HA vftable pointer)
+// CHECK-NEXT:   12 |   struct HB (virtual base)
+// CHECK-NEXT:   12 |     (HB vbtable pointer)
+// CHECK-NEXT:      | [sizeof=16, align=4
+// CHECK-NEXT:      |  nvsize=4, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT:    0 | struct HC
+// CHECK-X64-NEXT:    0 |   (HC vbtable pointer)
+// CHECK-X64-NEXT:   12 |   (vtordisp for vbase HA)
+// CHECK-X64-NEXT:   16 |   struct HA (virtual base)
+// CHECK-X64-NEXT:   16 |     (HA vftable pointer)
+// CHECK-X64-NEXT:   24 |   struct HB (virtual base)
+// CHECK-X64-NEXT:   24 |     (HB vbtable pointer)
+// CHECK-X64-NEXT:      | [sizeof=32, align=8
+// CHECK-X64-NEXT:      |  nvsize=8, nvalign=8]
+
 int a[
 sizeof(A)+
 sizeof(C)+
@@ -392,4 +427,5 @@ sizeof(pragma_test2::C)+
 sizeof(pragma_test3::C)+
 sizeof(pragma_test4::C)+
 sizeof(GD)+
+sizeof(HC)+
 0];