<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Thanks everyone! I landed this in r224651 since it seems to be behavior-neutral on visibility (my chromium components build also completed successfully).</div><div class="gmail_quote"><br></div><div class="gmail_quote">Please let me know if this causes problems for someone.</div>
<div class="gmail_quote"><br></div><div class="gmail_quote">On Fri, Dec 19, 2014 at 2:25 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On Fri, Dec 19, 2014 at 12:58 PM, Reid Kleckner <span dir="ltr"><<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">I think it's more interesting when the specialization lacks attributes:<div><br><div><span><div> template <int></div><div> struct A { void __attribute__((visibility("hidden"))) foo() {} };</div></span><div> template <> void A<0>::foo() {} // implicitly default visibility</div><div> <br></div></div><div> // compile with -fvisibility=hidden</div><div><div> template <int></div><div> struct A { void __attribute__((visibility("default"))) foo() {} };</div><div> template <> void A<0>::foo() {} // implicitly hidden visibility</div><div><br></div></div><div>I'm fine with this if there's no behavior change here.</div></div></div></blockquote><div><br></div></span><div>In both GCC and Clang, with and without this patch, A<0>::foo takes its visibility from the attribute.</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div><div class="gmail_extra"><div class="gmail_quote"><div><div>On Thu, Dec 18, 2014 at 11:42 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div>On Thu, Dec 18, 2014 at 9:58 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div><blockquote type="cite"><div>On Dec 18, 2014, at 6:18 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Dec 17, 2014 at 6:58 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>Don't drop attributes when checking explicit specializations.</div><div><br></div><div>Consider a template class with attributes on a method, and an explicit</div><div>specialization of that method:</div><div><br></div><div> template <int></div><div> struct A {</div><div> void foo() final;</div><div> };</div><div><br></div><div> template <></div><div> void A<0>::foo() {}</div><div><br></div><div>In this example, the attribute is `final`, but it might also be an</div><div>__attribute__((visibility("foo"))), noreturn, inline, etc. clang's current</div><div>behavior is to strip all attributes, which for some attributes is wrong</div><div>(the snippet above allows a subclass of A<0> to override the final method, for</div><div>example) and for others disagrees with gcc (__attribute__((visibility()))).</div><div><br></div><div>So stop dropping attributes. r95845 added this code without a test case, and</div><div>r176728 added the code for dropping attributes on parameters (with tests, but</div><div>they still pass).</div><div><br></div><div>As an additional wrinkle, do drop dllimport and dllexport, since that's how these two</div><div>attributes work. (This is covered by existing tests.)</div><div><br></div><div>Fixes PR21942.</div><div><br></div><div>The approach is by Richard Smith, initial analysis and typing was done by me.</div></div></blockquote><div><br></div><div>Naturally, this makes sense to me =) It also matches GCC and EDG on all attributes I tested.</div><div><br></div><div>John, do you remember why you added this code (a long long time ago)?</div></div></div></div>
</div></blockquote></div><br></div></div><div>The behavior on attribute((visibility)) is intentional, I think. At the very least, you have to allow for explicit specializations to override the visibility of the template, using a general most-specific-rule-wins logic.</div></div></blockquote><div><br></div></div></div><div>In practice, the attribute((visibility)) behavior seems unchanged, probably because it's propagated across redeclarations. I tried building the following program and compared `nm -m` output:</div><div><br></div><div> template <int></div><div> struct A { void __attribute__((visibility("hidden"))) foo() {} };</div><div> template <> void __attribute__((visibility("default"))) A<0>::foo() {}</div><div><br></div><div> void f() {</div><div> A<0> a0; A<1> a1;</div><div> a0.foo(); a1.foo();</div><div> }</div><div> </div><div>I toggled each of the two attributes on and off, the nm output with and without the patch looked identical.</div><div><br></div><div>I'm also doing a component build of Chromium with the patch, so far things look good.</div></div></div></div>
<br></div></div>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">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></blockquote></div></div></div></div>
</blockquote></div></div></div></div></div>
</blockquote></div></div></div>