r206124 - MS ABI: #pragma vtordisp(0) only disables new vtordisps
Keith Walker
kwalker at arm.com
Mon Apr 14 07:02:55 PDT 2014
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
More information about the cfe-commits
mailing list