[cfe-commits] r133157 - in /cfe/trunk: lib/CodeGen/CodeGenModule.cpp test/CodeGen/private-extern-redef.c

[John McCall]

On Jun 16, 2011, at 10:03 AM, jahanian wrote:

> 
> On Jun 16, 2011, at 9:36 AM, Eli Friedman wrote:
> 
>> On Thu, Jun 16, 2011 at 7:49 AM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>>> Author: fjahanian
>>> Date: Thu Jun 16 09:49:42 2011
>>> New Revision: 133157
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=133157&view=rev
>>> Log:
>>> Set the visibility to 'hidden' when previous
>>> declaration of global var is __private_extern__.
>>> // rdar://9609649
>>> 
>>> Added:
>>>   cfe/trunk/test/CodeGen/private-extern-redef.c
>>> Modified:
>>>   cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>>> 
>>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=133157&r1=133156&r2=133157&view=diff
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Jun 16 09:49:42 2011
>>> @@ -206,8 +206,17 @@
>>> 
>>>  // Set visibility for definitions.
>>>  NamedDecl::LinkageInfo LV = D->getLinkageAndVisibility();
>>> -  if (LV.visibilityExplicit() || !GV->hasAvailableExternallyLinkage())
>>> -    GV->setVisibility(GetLLVMVisibility(LV.visibility()));
>>> +  if (LV.visibilityExplicit() || !GV->hasAvailableExternallyLinkage()) {
>> 
>> If getLinkageAndVisibility is returning the wrong visibility, it
>> should be fixed in the AST/Sema, not hacked around in CodeGen.
> 
> This is not a hack.
> Idea was not to change the AST. It should reflect user source which is one 'hidden' decl.  followed by another
> declaration with default visibility. I can certainly move this code to MergeDecl though. Doug, what do you think?

Whether we change the AST or not, the implementation of getLinkageAndVisibility already does this sort of merging of information from redeclarations.  I agree with Eli;  please revert and fix differently.

John.