$ cat repro-weak-private-external.ii template <typename BoundArgsType> struct BindState; template <typename P1> struct BindState<void(P1)> { }; template <typename P1> void Bind(const P1& p1) { BindState<void(P1)>(); } class Version; void CheckPnaclComponentManifest(Version* version_out); // With this line: private. Without: not. class __attribute__((visibility("default"))) Version { }; void f() { Bind(Version()); } $ Release+Asserts/bin/clang -c repro-weak-private-external.ii -fvisibility=hidden && nm -m repro-weak-private-external.o | grep __Z4BindI7VersionEvRKT_ 0000000000000020 (__TEXT,__textcoal_nt) weak private external __Z4BindI7VersionEvRKT_ 0000000000000070 (__TEXT,__eh_frame) weak private external __Z4BindI7VersionEvRKT_.eh Note that the symbol is "weak private external. If the CheckPnaclComponentManifest() line is removed however, it beomces "weak external". This is (I think) because the CheckPnacl... function decl calls computeCachedProperties() in Type.cpp for the function type, which calls computeCachedProperties() or the argument type. When f() is being compiled, it then uses the cached value, which doesn't know about the explicit visibility yet. (This prevents the chromium component build from working on mac, see http://crbug.com/113934)
Changing the forward declaration of Version to: class __attribute__((visibility("default"))) Version; always makes the symbol weak external (no private). But requiring this would mean that changing the visibility of a type requires updating all forward declarations of the type, which is a bit unfortunate (and gcc and msvc seem to not require it).
gcc4.2 always makes the symbol weak private external. I guess there's no reason not to – just because Version is visibility("default") doesn't mean that BindState's constructor needs to be default-visible (it's not explicitly marked visible in any form).
Created attachment 8070 [details] half-assed patch The problem seems to be that the function visibility is set from parameter visibility, which doesn't make much sense. Just because a parameter type is explicitly declared visible, a function taking that type shouldn't become visible. The attached patch (missing test, comments) fixes this; I'm evaluating it now.
Actually, this patch. (Same idea, though.) Index: lib/AST/Decl.cpp =================================================================== --- lib/AST/Decl.cpp (revision 150357) +++ lib/AST/Decl.cpp (working copy) @@ -146,6 +146,8 @@ static LinkageInfo getLVForTemplateArgumentList(const TemplateArgument *Args, unsigned NumArgs, LVFlags &F) { + LinkageInfo TypeLV; + //bool oldE = false; LinkageInfo LV(ExternalLinkage, DefaultVisibility, false); for (unsigned I = 0; I != NumArgs; ++I) { @@ -156,7 +158,10 @@ break; case TemplateArgument::Type: - LV.merge(getLVForType(Args[I].getAsType())); + TypeLV = getLVForType(Args[I].getAsType()); + TypeLV.setVisibility(TypeLV.visibility(), false); + LV.merge(TypeLV); break; case TemplateArgument::Declaration:
Patch: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120213/053618.html
Fixed in r151236.