r175326 - Rework the visibility computation algorithm in preparation

[Nico Weber]

On Wed, Feb 27, 2013 at 8:05 PM, John McCall <rjmccall at apple.com> wrote:
> On Feb 27, 2013, at 7:19 AM, Nico Weber <thakis at chromium.org> wrote:
>> On Wed, Feb 27, 2013 at 3:32 PM, Nico Weber <thakis at chromium.org> wrote:
>>> On Tue, Feb 26, 2013 at 11:37 PM, John McCall <rjmccall at apple.com> wrote:
>>>> On Feb 26, 2013, at 2:36 PM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>>>>> We should just not cache visibility for types.
>>>>>
>>>>> Can do. I will send a patch.
>>>>
>>>> Thanks!  As you're doing so, please change it to use the same LinkageInfo structure that NamedDecl uses.  You'll need to make it a non-member type and possibly pull it into its own header.
>>>
>>> As of clang r176164 chromium's components build works again, with just
>>> 6 additional explicit EXPORTs on predeclared types used in template
>>> instantiations. Thanks for the help!
>>
>> …while the clang build is happy with these changes, the gcc build
>> breaks with them because gcc warns
>>
>> test.cc:6: warning: type attributes ignored after type is already defined
>>
>> on code like
>>
>> $ cat test.cc
>> #define EXPORT __attribute__((visibility("default")))
>>
>> class Layer;
>> class EXPORT Layer {
>> };
>> class EXPORT Layer;
>>
>> Due to the template instantiation requiring the EXPORT on the
>> declaration of Layer happening in one header and the definition of
>> Layer happening in another, it depends on header include order if gcc
>> is happy or not.
>>
>> Do you have a better suggestion than putting the EXPORTed declaration
>> of Layer into an #ifdef __clang__ block? Requiring this would be
>> pretty ugly. (It's only 3 files for us so we'll live, but the user
>> experience is a lot worse than in gcc / msvc.)
>
> Maybe use an EXPORT_FORWARD_DECL macro?
>
> That seems like a good bug to file against GCC, by the way.  There's
> no reason for them to warn about a *redundant* attribute on a
> redeclaration.

Since I know that you're all on the edge of your seats: As it turns
out, everything works without any local chromium changes with clang
r176164+, it just didn't occur to me to test that. (And if I revert
r176164 locally, the problem reappears.)

The reason is that we have explicit instantiation declarations in our
headers for the three classes that caused issues, and an explicit
instantiation definition in one cc file. By the time the instantiation
definition in the cc file is compiled, the real declaration of the
template types has been included and its EXPORT annotation is picked
up correctly since clang r176164.

Thanks for bearing with me here :-)

Nico