On Fri, Dec 19, 2014 at 12:58 PM, Reid Kleckner <[email protected]> wrote: > > I think it's more interesting when the specialization lacks attributes: > > template <int> > struct A { void __attribute__((visibility("hidden"))) foo() {} }; > template <> void A<0>::foo() {} // implicitly default visibility > > // compile with -fvisibility=hidden > template <int> > struct A { void __attribute__((visibility("default"))) foo() {} }; > template <> void A<0>::foo() {} // implicitly hidden visibility > > I'm fine with this if there's no behavior change here. >
In both GCC and Clang, with and without this patch, A<0>::foo takes its visibility from the attribute. > On Thu, Dec 18, 2014 at 11:42 PM, Nico Weber <[email protected]> wrote: > >> On Thu, Dec 18, 2014 at 9:58 PM, John McCall <[email protected]> wrote: >>> >>> On Dec 18, 2014, at 6:18 PM, Richard Smith <[email protected]> >>> wrote: >>> On Wed, Dec 17, 2014 at 6:58 PM, Nico Weber <[email protected]> wrote: >>>> >>>> Don't drop attributes when checking explicit specializations. >>>> >>>> Consider a template class with attributes on a method, and an explicit >>>> specialization of that method: >>>> >>>> template <int> >>>> struct A { >>>> void foo() final; >>>> }; >>>> >>>> template <> >>>> void A<0>::foo() {} >>>> >>>> In this example, the attribute is `final`, but it might also be an >>>> __attribute__((visibility("foo"))), noreturn, inline, etc. clang's >>>> current >>>> behavior is to strip all attributes, which for some attributes is wrong >>>> (the snippet above allows a subclass of A<0> to override the final >>>> method, for >>>> example) and for others disagrees with gcc >>>> (__attribute__((visibility()))). >>>> >>>> So stop dropping attributes. r95845 added this code without a test >>>> case, and >>>> r176728 added the code for dropping attributes on parameters (with >>>> tests, but >>>> they still pass). >>>> >>>> As an additional wrinkle, do drop dllimport and dllexport, since that's >>>> how these two >>>> attributes work. (This is covered by existing tests.) >>>> >>>> Fixes PR21942. >>>> >>>> The approach is by Richard Smith, initial analysis and typing was done >>>> by me. >>>> >>> >>> Naturally, this makes sense to me =) It also matches GCC and EDG on all >>> attributes I tested. >>> >>> John, do you remember why you added this code (a long long time ago)? >>> >>> >>> 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. >>> >> >> 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: >> >> template <int> >> struct A { void __attribute__((visibility("hidden"))) foo() {} }; >> template <> void __attribute__((visibility("default"))) A<0>::foo() {} >> >> void f() { >> A<0> a0; A<1> a1; >> a0.foo(); a1.foo(); >> } >> >> I toggled each of the two attributes on and off, the nm output with and >> without the patch looked identical. >> >> I'm also doing a component build of Chromium with the patch, so far >> things look good. >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
