MS ABI: Pure virtual functions don't contribute to vtordisps

Usually, overriding a virtual function defined in a virtual base
required emission of a vtordisp slot in the record.  However no vtordisp
is needed if the overriding function is pure; it should be impossible to
observe the pure virtual method.

This fixes PR21046.

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

lib/AST/RecordLayoutBuilder.cpp

@@ -2181,8 +2181,9 @@ public:
     FieldOffsets.push_back(FieldOffset);
   }
   /// \brief Compute the set of virtual bases for which vtordisps are required.
-  llvm::SmallPtrSet<const CXXRecordDecl *, 2>
-  computeVtorDispSet(const CXXRecordDecl *RD);
+  void computeVtorDispSet(
+      llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtorDispSet,
+      const CXXRecordDecl *RD) const;
   const ASTContext &Context;
   /// \brief The size of the record being laid out.
   CharUnits Size;
@@ -2605,14 +2606,14 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
   }
   VtorDispAlignment = std::max(VtorDispAlignment, RequiredAlignment);
   // Compute the vtordisp set.
-  llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet =
-      computeVtorDispSet(RD);
+  llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtorDispSet;
+  computeVtorDispSet(HasVtorDispSet, RD);
   // Iterate through the virtual bases and lay them out.
   const ASTRecordLayout *PreviousBaseLayout = nullptr;
   for (const CXXBaseSpecifier &VBase : RD->vbases()) {
     const CXXRecordDecl *BaseDecl = VBase.getType()->getAsCXXRecordDecl();
     const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl);
-    bool HasVtordisp = HasVtordispSet.count(BaseDecl);
+    bool HasVtordisp = HasVtorDispSet.count(BaseDecl) > 0;
     // Insert padding between two bases if the left first one is zero sized or
     // contains a zero sized subobject and the right is zero sized or one leads
     // with a zero sized base.  The padding between virtual bases is 4
@@ -2671,10 +2672,9 @@ RequiresVtordisp(const llvm::SmallPtrSetImpl<const CXXRecordDecl *> &
   return false;
 }
 
-llvm::SmallPtrSet<const CXXRecordDecl *, 2>
-MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
-  llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet;
-
+void MicrosoftRecordLayoutBuilder::computeVtorDispSet(
+    llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtordispSet,
+    const CXXRecordDecl *RD) const {
   // /vd2 or #pragma vtordisp(2): Always use vtordisps for virtual bases with
   // vftables.
   if (RD->getMSVtorDispMode() == MSVtorDispAttr::ForVFTable) {
@@ -2684,7 +2684,7 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
       if (Layout.hasExtendableVFPtr())
         HasVtordispSet.insert(BaseDecl);
     }
-    return HasVtordispSet;
+    return;
   }
 
   // If any of our bases need a vtordisp for this type, so do we.  Check our
@@ -2701,7 +2701,7 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
   // * #pragma vtordisp(0) or the /vd0 flag are in use.
   if ((!RD->hasUserDeclaredConstructor() && !RD->hasUserDeclaredDestructor()) ||
       RD->getMSVtorDispMode() == MSVtorDispAttr::Never)
-    return HasVtordispSet;
+    return;
   // /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.
@@ -2712,9 +2712,9 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
   // vtordisp.
   llvm::SmallPtrSet<const CXXMethodDecl *, 8> Work;
   llvm::SmallPtrSet<const CXXRecordDecl *, 2> BasesWithOverriddenMethods;
-  // Seed the working set with our non-destructor virtual methods.
+  // Seed the working set with our non-destructor, non-pure virtual methods.
   for (const CXXMethodDecl *MD : RD->methods())
-    if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD))
+    if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD) && !MD->isPure())
       Work.insert(MD);
   while (!Work.empty()) {
     const CXXMethodDecl *MD = *Work.begin();
@@ -2736,7 +2736,6 @@ MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {
         RequiresVtordisp(BasesWithOverriddenMethods, BaseDecl))
       HasVtordispSet.insert(BaseDecl);
   }
-  return HasVtordispSet;
 }
 
 /// \brief Get or compute information about the layout of the specified record

test/Layout/ms-x86-vtordisp.cpp

@@ -416,6 +416,31 @@ struct HC : virtual HB {};
 // CHECK-X64-NEXT:      | [sizeof=32, align=8
 // CHECK-X64-NEXT:      |  nvsize=8, nvalign=8]
 
+struct IA {
+  virtual void f();
+};
+struct __declspec(dllexport) IB : virtual IA {
+  virtual void f() = 0;
+  IB() {}
+};
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:    0 | struct IB
+// CHECK-NEXT:    0 |   (IB vbtable pointer)
+// CHECK-NEXT:    4 |   struct IA (virtual base)
+// CHECK-NEXT:    4 |     (IA vftable pointer)
+// CHECK-NEXT:      | [sizeof=8, align=4
+// CHECK-NEXT:      |  nvsize=4, nvalign=4]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT:    0 | struct IB
+// CHECK-X64-NEXT:    0 |   (IB vbtable pointer)
+// CHECK-X64-NEXT:    8 |   struct IA (virtual base)
+// CHECK-X64-NEXT:    8 |     (IA vftable pointer)
+// CHECK-X64-NEXT:      | [sizeof=16, align=8
+// CHECK-X64-NEXT:      |  nvsize=8, nvalign=8]
+
 int a[
 sizeof(A)+
 sizeof(C)+
@@ -428,4 +453,5 @@ sizeof(pragma_test3::C)+
 sizeof(pragma_test4::C)+
 sizeof(GD)+
 sizeof(HC)+
+sizeof(IB)+
 0];