[patch] Don't drop attributes when checking explicit specializations.

[Nico Weber]

On Thu, Dec 18, 2014 at 9:58 PM, John McCall <rjmccall at apple.com> wrote:
>
> On Dec 18, 2014, at 6:18 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Wed, Dec 17, 2014 at 6:58 PM, Nico Weber <thakis at chromium.org> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141218/90402091/attachment.html>