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