[cfe-commits] [PATCH] Implement '#pragma GCC visibility' (WIP)

[Eli Friedman]

On Sun, Jul 18, 2010 at 9:36 AM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Sun, Jul 18, 2010 at 5:08 AM, Douglas Gregor <dgregor at apple.com> wrote:
>>
>> On Jul 17, 2010, at 9:09 PM, Eli Friedman wrote:
>>
>>> Attached.  Review comments welcome.  This doesn't interact quite
>>> correctly with classes and namespaces; any guidance on how to
>>> implement the correct rules there would be appreciated.
>>
>> Thanks for working on this! A few comments:
>>
>> Index: lib/Sema/SemaDeclAttr.cpp
>> ===================================================================
>> --- lib/Sema/SemaDeclAttr.cpp   (revision 108632)
>> +++ lib/Sema/SemaDeclAttr.cpp   (working copy)
>> @@ -678,7 +678,7 @@
>>   else if (TypeStr == "protected")
>>     type = VisibilityAttr::ProtectedVisibility;
>>   else {
>> -    S.Diag(Attr.getLoc(), diag::warn_attribute_unknown_visibility) << TypeStr;
>> +    S.Diag(Attr.getLoc(), diag::warn_attribute_unknown_visibility) << Str;
>>     return;
>>   }
>>
>> @@ -2172,6 +2172,9 @@
>>   // Finally, apply any attributes on the decl itself.
>>   if (const AttributeList *Attrs = PD.getAttributes())
>>     ProcessDeclAttributeList(S, D, Attrs);
>> +
>> +  // Tack on a visibility attribute from '#pragma GCC visibility', if necessary.
>> +  AddPushedVisibilityAttribute(D);
>>  }
>>
>> Since this isn't supposed to apply to class members, function-local variables, instance members, etc.,, the AddPushedVisibilityAttribute call should only occur when the declaration has external linkage and !D->getDeclContext()->isRecord().
>
> Ah, I guess that'll cover the issue's I've spotted with class members.

And now I've discovered another issue... consider the following:
#pragma GCC visibility push(hidden)
__attribute((visibility("default"))) extern int x;
int x = 1;

My current patch makes x hidden, gcc doesn't.  Any suggestions for
where to put the call to AddPushedVisibilityAttribute to deal with
this?

-E