Visibility attribute ignored on a type that's used in a function prototype earlier #12373
Comments
|
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). |
|
half-assed patch 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)
|
|
Fixed in r151236. |
Extended Description
$ cat repro-weak-private-external.ii
template
struct BindState;
template
struct BindState<void(P1)> {
};
template
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)
The text was updated successfully, but these errors were encountered: