r206124 - MS ABI: #pragma vtordisp(0) only disables new vtordisps
Reid Kleckner
rnk at google.com
Mon Apr 14 17:06:19 PDT 2014
Thanks, fixed in r206224.
On Mon, Apr 14, 2014 at 7:02 AM, Keith Walker <kwalker at arm.com> wrote:
> David,
>
> The changes in the test ms_x86-vtordisp.cpp
>
> > +#pragma vtordisp(pop, 2)
> > +#pragma vtordisp(pop, 0)
>
> are causing this test to fail on our build system because clang is
> generating the following warnings which the cause the FileCheck to fail as
> this is not expected output.
>
> > llvm/tools/clang/test/Layout/ms-x86-vtordisp.cpp:389:9: warning: missing
> ')' after '#pragma vtordisp' - ignoring
> > #pragma vtordisp(pop, 2)
> > ^
> > llvm/tools/clang/test/Layout/ms-x86-vtordisp.cpp:392:9: warning: missing
> ')' after '#pragma vtordisp' - ignoring
> > #pragma vtordisp(pop, 0)
> > ^
> > 2 warnings generated.
>
> I assume that removing the 2nd parameter is the correct fix.
>
> Keith
>
> > -----Original Message-----
> > From: cfe-commits-bounces at cs.uiuc.edu [mailto:cfe-commits-
> > bounces at cs.uiuc.edu] On Behalf Of David Majnemer
> > Sent: 13 April 2014 03:28
> > To: cfe-commits at cs.uiuc.edu
> > Subject: r206124 - MS ABI: #pragma vtordisp(0) only disables new
> > vtordisps
> >
> > Author: majnemer
> > Date: Sat Apr 12 21:27:32 2014
> > New Revision: 206124
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=206124&view=rev
> > Log:
> > MS ABI: #pragma vtordisp(0) only disables new vtordisps
> >
> > Previously, it was believed that #pragma vtordisp(0) would prohibit the
> > generation of any and all vtordisps.
> >
> > In actuality, it only disables the generation of additional vtordisps.
> >
> > This fixes PR19413.
> >
> > 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=206124&r1=206123&
> > r2=206124&view=diff
> > =======================================================================
> > =======
> > --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
> > +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Sat Apr 12 21:27:32 2014
> > @@ -2674,10 +2674,6 @@ llvm::SmallPtrSet<const CXXRecordDecl *,
> > MicrosoftRecordLayoutBuilder::computeVtorDispSet(const CXXRecordDecl
> > *RD) {
> > llvm::SmallPtrSet<const CXXRecordDecl *, 2> HasVtordispSet;
> >
> > - // /vd0 or #pragma vtordisp(0): Never use vtordisps when used as a
> > vbase.
> > - if (RD->getMSVtorDispMode() == MSVtorDispAttr::Never)
> > - return HasVtordispSet;
> > -
> > // /vd2 or #pragma vtordisp(2): Always use vtordisps for virtual
> > bases with
> > // vftables.
> > if (RD->getMSVtorDispMode() == MSVtorDispAttr::ForVFTable) {
> > @@ -2690,11 +2686,6 @@ MicrosoftRecordLayoutBuilder::computeVto
> > return HasVtordispSet;
> > }
> >
> > - // /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.
> > - assert(RD->getMSVtorDispMode() == MSVtorDispAttr::ForVBaseOverride);
> > -
> > // If any of our bases need a vtordisp for this type, so do we.
> > Check our
> > // direct bases for vtordisp requirements.
> > for (const auto &I : RD->bases()) {
> > @@ -2704,10 +2695,16 @@ MicrosoftRecordLayoutBuilder::computeVto
> > if (bi.second.hasVtorDisp())
> > HasVtordispSet.insert(bi.first);
> > }
> > - // If we do not have a user declared constructor or destructor then
> > we don't
> > - // introduce any additional vtordisps.
> > - if (!RD->hasUserDeclaredConstructor() && !RD-
> > >hasUserDeclaredDestructor())
> > + // We don't introduce any additional vtordisps if either:
> > + // * A user declared constructor or destructor aren't declared.
> > + // * #pragma vtordisp(0) or the /vd0 flag are in use.
> > + if ((!RD->hasUserDeclaredConstructor() && !RD-
> > >hasUserDeclaredDestructor()) ||
> > + RD->getMSVtorDispMode() == MSVtorDispAttr::Never)
> > return HasVtordispSet;
> > + // /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.
> > + assert(RD->getMSVtorDispMode() == MSVtorDispAttr::ForVBaseOverride);
> > // Compute a set of base classes which define methods we override.
> > A virtual
> > // base in this set will require a vtordisp. A virtual base that
> > transitively
> > // contains one of these bases as a non-virtual base will also
> > require a
> >
> > 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=206124&r1=206123&r2=206124&view=diff
> > =======================================================================
> > =======
> > --- cfe/trunk/test/Layout/ms-x86-vtordisp.cpp (original)
> > +++ cfe/trunk/test/Layout/ms-x86-vtordisp.cpp Sat Apr 12 21:27:32 2014
> > @@ -381,6 +381,41 @@ struct GD: public virtual GC, public vir
> > // CHECK-X64-NEXT: | [sizeof=40, align=8
> > // CHECK-X64-NEXT: | nvsize=8, nvalign=8]
> >
> > +struct HA {
> > + virtual void fun() {}
> > +};
> > +#pragma vtordisp(push, 2)
> > +struct HB : virtual HA {};
> > +#pragma vtordisp(pop, 2)
> > +#pragma vtordisp(push, 0)
> > +struct HC : virtual HB {};
> > +#pragma vtordisp(pop, 0)
> > +
> > +// CHECK: *** Dumping AST Record Layout
> > +// CHECK: *** Dumping AST Record Layout
> > +// CHECK: *** Dumping AST Record Layout
> > +// CHECK-NEXT: 0 | struct HC
> > +// CHECK-NEXT: 0 | (HC vbtable pointer)
> > +// CHECK-NEXT: 4 | (vtordisp for vbase HA)
> > +// CHECK-NEXT: 8 | struct HA (virtual base)
> > +// CHECK-NEXT: 8 | (HA vftable pointer)
> > +// CHECK-NEXT: 12 | struct HB (virtual base)
> > +// CHECK-NEXT: 12 | (HB vbtable pointer)
> > +// CHECK-NEXT: | [sizeof=16, align=4
> > +// CHECK-NEXT: | nvsize=4, nvalign=4]
> > +// CHECK-X64: *** Dumping AST Record Layout
> > +// CHECK-X64: *** Dumping AST Record Layout
> > +// CHECK-X64: *** Dumping AST Record Layout
> > +// CHECK-X64-NEXT: 0 | struct HC
> > +// CHECK-X64-NEXT: 0 | (HC vbtable pointer)
> > +// CHECK-X64-NEXT: 12 | (vtordisp for vbase HA)
> > +// CHECK-X64-NEXT: 16 | struct HA (virtual base)
> > +// CHECK-X64-NEXT: 16 | (HA vftable pointer)
> > +// CHECK-X64-NEXT: 24 | struct HB (virtual base)
> > +// CHECK-X64-NEXT: 24 | (HB vbtable pointer)
> > +// CHECK-X64-NEXT: | [sizeof=32, align=8
> > +// CHECK-X64-NEXT: | nvsize=8, nvalign=8]
> > +
> > int a[
> > sizeof(A)+
> > sizeof(C)+
> > @@ -392,4 +427,5 @@ sizeof(pragma_test2::C)+
> > sizeof(pragma_test3::C)+
> > sizeof(pragma_test4::C)+
> > sizeof(GD)+
> > +sizeof(HC)+
> > 0];
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
>
> _______________________________________________
> 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/20140414/cb1cca54/attachment.html>
More information about the cfe-commits
mailing list