Added in r218965. On Wed, Sep 24, 2014 at 6:53 AM, Timur Iskhodzhanov <[email protected]> wrote:
> 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
