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 <[email protected]> >> 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. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
