r218340 - MS ABI: Pure virtual functions don't contribute to vtordisps
David Majnemer
david.majnemer at gmail.com
Fri Oct 3 01:04:18 PDT 2014
Added in r218965.
On Wed, Sep 24, 2014 at 6:53 AM, Timur Iskhodzhanov <timurrrr at google.com>
wrote:
> Maybe we should add a vftable test for this case too?
>
> 2014-09-24 2:58 GMT+04:00 David Majnemer <david.majnemer at gmail.com>:
> > 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
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141003/638b4671/attachment.html>
More information about the cfe-commits
mailing list