Maybe we should add a vftable test for this case too?
2014-09-24 2:58 GMT+04:00 David Majnemer <[email protected]>: > Author: majnemer > Date: Tue Sep 23 17:58:15 2014 > New Revision: 218340 > > URL: http://llvm.org/viewvc/llvm-project?rev=218340&view=rev > Log: > 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. > > 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=218340&r1=218339&r2=218340&view=diff > ============================================================================== > --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original) > +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Sep 23 17:58:15 2014 > @@ -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::layou > } > 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::SmallPtrSet > 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::computeVto > 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::computeVto > // * #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::computeVto > // 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::computeVto > RequiresVtordisp(BasesWithOverriddenMethods, BaseDecl)) > HasVtordispSet.insert(BaseDecl); > } > - return HasVtordispSet; > } > > /// \brief Get or compute information about the layout of the specified > record > > 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=218340&r1=218339&r2=218340&view=diff > ============================================================================== > --- cfe/trunk/test/Layout/ms-x86-vtordisp.cpp (original) > +++ cfe/trunk/test/Layout/ms-x86-vtordisp.cpp Tue Sep 23 17:58:15 2014 > @@ -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]; > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
