<div dir="ltr">Added in r218965.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Sep 24, 2014 at 6:53 AM, Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Maybe we should add a vftable test for this case too?<br>
<div class="HOEnZb"><div class="h5"><br>
2014-09-24 2:58 GMT+04:00 David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>>:<br>
> Author: majnemer<br>
> Date: Tue Sep 23 17:58:15 2014<br>
> New Revision: 218340<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=218340&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=218340&view=rev</a><br>
> Log:<br>
> MS ABI: Pure virtual functions don't contribute to vtordisps<br>
><br>
> Usually, overriding a virtual function defined in a virtual base<br>
> required emission of a vtordisp slot in the record. However no vtordisp<br>
> is needed if the overriding function is pure; it should be impossible to<br>
> observe the pure virtual method.<br>
><br>
> This fixes PR21046.<br>
><br>
> Modified:<br>
> cfe/trunk/lib/AST/RecordLayoutBuilder.cpp<br>
> cfe/trunk/test/Layout/ms-x86-vtordisp.cpp<br>
><br>
> Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=218340&r1=218339&r2=218340&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=218340&r1=218339&r2=218340&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)<br>
> +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Sep 23 17:58:15 2014<br>
> @@ -2181,8 +2181,9 @@ public:<br>
> FieldOffsets.push_back(FieldOffset);<br>
> }<br>
> /// \brief Compute the set of virtual bases for which vtordisps are required.<br>
> - llvm::SmallPtrSet<const CXXRecordDecl *, 2><br>
> - computeVtorDispSet(const CXXRecordDecl *RD);<br>
> + void computeVtorDispSet(<br>
> + llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtorDispSet,<br>
> + const CXXRecordDecl *RD) const;<br>
> const ASTContext &Context;<br>
> /// \brief The size of the record being laid out.<br>
> CharUnits Size;<br>
> @@ -2605,14 +2606,14 @@ void MicrosoftRecordLayoutBuilder::layou<br>
> }<br>
> VtorDispAlignment = std::max(VtorDispAlignment, RequiredAlignment);<br>
> // Compute the vtordisp set.<br>
> - llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet =<br>
> - computeVtorDispSet(RD);<br>
> + llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtorDispSet;<br>
> + computeVtorDispSet(HasVtorDispSet, RD);<br>
> // Iterate through the virtual bases and lay them out.<br>
> const ASTRecordLayout *PreviousBaseLayout = nullptr;<br>
> for (const CXXBaseSpecifier &VBase : RD->vbases()) {<br>
> const CXXRecordDecl *BaseDecl = VBase.getType()->getAsCXXRecordDecl();<br>
> const ASTRecordLayout &BaseLayout = Context.getASTRecordLayout(BaseDecl);<br>
> - bool HasVtordisp = HasVtordispSet.count(BaseDecl);<br>
> + bool HasVtordisp = HasVtorDispSet.count(BaseDecl) > 0;<br>
> // Insert padding between two bases if the left first one is zero sized or<br>
> // contains a zero sized subobject and the right is zero sized or one leads<br>
> // with a zero sized base. The padding between virtual bases is 4<br>
> @@ -2671,10 +2672,9 @@ RequiresVtordisp(const llvm::SmallPtrSet<br>
> return false;<br>
> }<br>
><br>
> -llvm::SmallPtrSet<const CXXRecordDecl *, 2><br>
> -MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl *RD) {<br>
> - llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet;<br>
> -<br>
> +void MicrosoftRecordLayoutBuilder::computeVtorDispSet(<br>
> + llvm::SmallPtrSetImpl<const CXXRecordDecl *> &HasVtordispSet,<br>
> + const CXXRecordDecl *RD) const {<br>
> // /vd2 or #pragma vtordisp(2): Always use vtordisps for virtual bases with<br>
> // vftables.<br>
> if (RD->getMSVtorDispMode() == MSVtorDispAttr::ForVFTable) {<br>
> @@ -2684,7 +2684,7 @@ MicrosoftRecordLayoutBuilder::computeVto<br>
> if (Layout.hasExtendableVFPtr())<br>
> HasVtordispSet.insert(BaseDecl);<br>
> }<br>
> - return HasVtordispSet;<br>
> + return;<br>
> }<br>
><br>
> // If any of our bases need a vtordisp for this type, so do we. Check our<br>
> @@ -2701,7 +2701,7 @@ MicrosoftRecordLayoutBuilder::computeVto<br>
> // * #pragma vtordisp(0) or the /vd0 flag are in use.<br>
> if ((!RD->hasUserDeclaredConstructor() && !RD->hasUserDeclaredDestructor()) ||<br>
> RD->getMSVtorDispMode() == MSVtorDispAttr::Never)<br>
> - return HasVtordispSet;<br>
> + return;<br>
> // /vd1 or #pragma vtordisp(1): Try to guess based on whether we think it's<br>
> // possible for a partially constructed object with virtual base overrides to<br>
> // escape a non-trivial constructor.<br>
> @@ -2712,9 +2712,9 @@ MicrosoftRecordLayoutBuilder::computeVto<br>
> // vtordisp.<br>
> llvm::SmallPtrSet<const CXXMethodDecl *, 8> Work;<br>
> llvm::SmallPtrSet<const CXXRecordDecl *, 2> BasesWithOverriddenMethods;<br>
> - // Seed the working set with our non-destructor virtual methods.<br>
> + // Seed the working set with our non-destructor, non-pure virtual methods.<br>
> for (const CXXMethodDecl *MD : RD->methods())<br>
> - if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD))<br>
> + if (MD->isVirtual() && !isa<CXXDestructorDecl>(MD) && !MD->isPure())<br>
> Work.insert(MD);<br>
> while (!Work.empty()) {<br>
> const CXXMethodDecl *MD = *Work.begin();<br>
> @@ -2736,7 +2736,6 @@ MicrosoftRecordLayoutBuilder::computeVto<br>
> RequiresVtordisp(BasesWithOverriddenMethods, BaseDecl))<br>
> HasVtordispSet.insert(BaseDecl);<br>
> }<br>
> - return HasVtordispSet;<br>
> }<br>
><br>
> /// \brief Get or compute information about the layout of the specified record<br>
><br>
> Modified: cfe/trunk/test/Layout/ms-x86-vtordisp.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-vtordisp.cpp?rev=218340&r1=218339&r2=218340&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-vtordisp.cpp?rev=218340&r1=218339&r2=218340&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Layout/ms-x86-vtordisp.cpp (original)<br>
> +++ cfe/trunk/test/Layout/ms-x86-vtordisp.cpp Tue Sep 23 17:58:15 2014<br>
> @@ -416,6 +416,31 @@ struct HC : virtual HB {};<br>
> // CHECK-X64-NEXT: | [sizeof=32, align=8<br>
> // CHECK-X64-NEXT: | nvsize=8, nvalign=8]<br>
><br>
> +struct IA {<br>
> + virtual void f();<br>
> +};<br>
> +struct __declspec(dllexport) IB : virtual IA {<br>
> + virtual void f() = 0;<br>
> + IB() {}<br>
> +};<br>
> +<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK: *** Dumping AST Record Layout<br>
> +// CHECK-NEXT: 0 | struct IB<br>
> +// CHECK-NEXT: 0 | (IB vbtable pointer)<br>
> +// CHECK-NEXT: 4 | struct IA (virtual base)<br>
> +// CHECK-NEXT: 4 | (IA vftable pointer)<br>
> +// CHECK-NEXT: | [sizeof=8, align=4<br>
> +// CHECK-NEXT: | nvsize=4, nvalign=4]<br>
> +// CHECK-X64: *** Dumping AST Record Layout<br>
> +// CHECK-X64: *** Dumping AST Record Layout<br>
> +// CHECK-X64-NEXT: 0 | struct IB<br>
> +// CHECK-X64-NEXT: 0 | (IB vbtable pointer)<br>
> +// CHECK-X64-NEXT: 8 | struct IA (virtual base)<br>
> +// CHECK-X64-NEXT: 8 | (IA vftable pointer)<br>
> +// CHECK-X64-NEXT: | [sizeof=16, align=8<br>
> +// CHECK-X64-NEXT: | nvsize=8, nvalign=8]<br>
> +<br>
> int a[<br>
> sizeof(A)+<br>
> sizeof(C)+<br>
> @@ -428,4 +453,5 @@ sizeof(pragma_test3::C)+<br>
> sizeof(pragma_test4::C)+<br>
> sizeof(GD)+<br>
> sizeof(HC)+<br>
> +sizeof(IB)+<br>
> 0];<br>
><br>
><br>
> _______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>