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

[David Majnemer]

Author: majnemer
Date: Sat Apr 12 21:27:32 2014
New Revision: 206124

URL: http://llvm.org/viewvc/llvm-project?rev=206124&view=rev
Log:
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.

Modified:
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
    cfe/trunk/test/Layout/ms-x86-vtordisp.cpp

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=206124&r1=206123&r2=206124&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Sat Apr 12 21:27:32 2014
@@ -2674,10 +2674,6 @@ llvm::SmallPtrSet<const CXXRecordDecl *,
 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::computeVto
     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::computeVto
       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

Modified: cfe/trunk/test/Layout/ms-x86-vtordisp.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-vtordisp.cpp?rev=206124&r1=206123&r2=206124&view=diff
==============================================================================
--- cfe/trunk/test/Layout/ms-x86-vtordisp.cpp (original)
+++ cfe/trunk/test/Layout/ms-x86-vtordisp.cpp Sat Apr 12 21:27:32 2014
@@ -381,6 +381,41 @@ struct GD: public virtual GC, public vir
 // 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];