With -fvisibility=hidden we currently produce a default visibility
symbol for f in
--------------------------
template <typename T>
struct X {
static void f() {}
};
class __attribute__((visibility("default"))) A;
void g() {
X<A> y;
y.f();
}
---------------------------
GCC is able to produce a hidden symbol.
What is happening is
1 We start with an implicit default visibility
2 We merge the visibility of A, getting an explicit default visibility
3 We merge the implicit hidden visibility from the command line, but
that does nothing since we already have an explicit visibility.
We cannot simply change the merge function to give preference to more
restrictive visibilities as that would for example make everything in A
hidden.
To fix this I added a specialized merge function that never increases
the visibility. It should be used in cases like the merging of the
template arguments in the above case. The are probably more cases not
yet covered by this patch where it should be used.
Just using this new function in step 2 above would not be sufficient, as
at that point we still have an default visibility. To fix this I simply
moved the handling of -fvisibility=hidden to the top. This now works as
we always keep track of the explicitness of the visibility and override
it as need.
Suggestions for a better name for mergeWithMin are welcome :-)
Cheers,
Rafael
diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 760e598..3ae02aa 100644
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -251,6 +251,7 @@ public:
mergeLinkage(Other.linkage());
}
+ // Merge the visibility V giving preference to explicit ones.
void mergeVisibility(Visibility V, bool E = false) {
// If one has explicit visibility and the other doesn't, keep the
// explicit one.
@@ -262,14 +263,32 @@ public:
// If both are explicit or both are implicit, keep the minimum.
setVisibility(minVisibility(visibility(), V), visibilityExplicit() || E);
}
+ // Merge the visibility V, keeping the most restrictive one.
+ void mergeVisibilityWithMin(Visibility V, bool E = false) {
+ // Never increase the visibility
+ if (visibility() < V)
+ return;
+
+ // If this visibility is explicit, keep it.
+ if (visibilityExplicit() && !E)
+ return;
+ setVisibility(V, E);
+ }
void mergeVisibility(LinkageInfo Other) {
mergeVisibility(Other.visibility(), Other.visibilityExplicit());
}
+ void mergeVisibilityWithMin(LinkageInfo Other) {
+ mergeVisibilityWithMin(Other.visibility(), Other.visibilityExplicit());
+ }
void merge(LinkageInfo Other) {
mergeLinkage(Other);
mergeVisibility(Other);
}
+ void mergeWithMin(LinkageInfo Other) {
+ mergeLinkage(Other);
+ mergeWithMin(Other);
+ }
friend LinkageInfo merge(LinkageInfo L, LinkageInfo R) {
L.merge(R);
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index d80e06b..ee00c3c 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -176,9 +176,9 @@ static LinkageInfo getLVForTemplateArgumentList(const
TemplateArgument *Args,
break;
case TemplateArgument::Pack:
- LV.merge(getLVForTemplateArgumentList(Args[I].pack_begin(),
- Args[I].pack_size(),
- F));
+ LV.mergeWithMin(getLVForTemplateArgumentList(Args[I].pack_begin(),
+ Args[I].pack_size(),
+ F));
break;
}
}
@@ -279,6 +279,7 @@ static LinkageInfo getLVForNamespaceScopeDecl(const
NamedDecl *D, LVFlags F) {
// scope and no storage-class specifier, its linkage is
// external.
LinkageInfo LV;
+ LV.mergeVisibility(Context.getLangOptions().getVisibilityMode());
if (F.ConsiderVisibilityAttributes) {
if (llvm::Optional<Visibility> Vis = D->getExplicitVisibility()) {
@@ -413,7 +414,7 @@ static LinkageInfo getLVForNamespaceScopeDecl(const
NamedDecl *D, LVFlags F) {
LV.merge(getLVForDecl(specInfo->getTemplate(),
F.onlyTemplateVisibility()));
const TemplateArgumentList &templateArgs =
*specInfo->TemplateArguments;
- LV.merge(getLVForTemplateArgumentList(templateArgs, F));
+ LV.mergeWithMin(getLVForTemplateArgumentList(templateArgs, F));
}
}
@@ -439,7 +440,7 @@ static LinkageInfo getLVForNamespaceScopeDecl(const
NamedDecl *D, LVFlags F) {
// The arguments at which the template was instantiated.
const TemplateArgumentList &TemplateArgs = spec->getTemplateArgs();
- LV.merge(getLVForTemplateArgumentList(TemplateArgs, F));
+ LV.mergeWithMin(getLVForTemplateArgumentList(TemplateArgs, F));
}
}
@@ -482,11 +483,6 @@ static LinkageInfo getLVForNamespaceScopeDecl(const
NamedDecl *D, LVFlags F) {
if (LV.linkage() != ExternalLinkage)
return LinkageInfo(LV.linkage(), DefaultVisibility, false);
- // If we didn't end up with hidden visibility, consider attributes
- // and -fvisibility.
- if (F.ConsiderGlobalVisibility)
- LV.mergeVisibility(Context.getLangOptions().getVisibilityMode());
-
return LV;
}
@@ -503,6 +499,7 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D,
LVFlags F) {
return LinkageInfo::none();
LinkageInfo LV;
+ LV.mergeVisibility(D->getASTContext().getLangOptions().getVisibilityMode());
// The flags we're going to use to compute the class's visibility.
LVFlags ClassF = F;
@@ -544,7 +541,8 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D,
LVFlags F) {
if (FunctionTemplateSpecializationInfo *spec
= MD->getTemplateSpecializationInfo()) {
if (shouldConsiderTemplateLV(MD, spec)) {
- LV.merge(getLVForTemplateArgumentList(*spec->TemplateArguments, F));
+ LV.mergeWithMin(getLVForTemplateArgumentList(*spec->TemplateArguments,
+ F));
if (F.ConsiderTemplateParameterTypes)
LV.merge(getLVForTemplateParameterList(
spec->getTemplate()->getTemplateParameters()));
@@ -583,7 +581,8 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D,
LVFlags F) {
if (shouldConsiderTemplateLV(spec)) {
// Merge template argument/parameter information for member
// class template specializations.
- LV.merge(getLVForTemplateArgumentList(spec->getTemplateArgs(), F));
+ LV.mergeWithMin(getLVForTemplateArgumentList(spec->getTemplateArgs(),
+ F));
if (F.ConsiderTemplateParameterTypes)
LV.merge(getLVForTemplateParameterList(
spec->getSpecializedTemplate()->getTemplateParameters()));
@@ -601,13 +600,6 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D,
LVFlags F) {
LV.mergeVisibility(TypeLV.visibility(), TypeLV.visibilityExplicit());
}
- F.ConsiderGlobalVisibility &= !LV.visibilityExplicit();
-
- // Apply -fvisibility if desired.
- if (F.ConsiderGlobalVisibility && LV.visibility() != HiddenVisibility) {
-
LV.mergeVisibility(D->getASTContext().getLangOptions().getVisibilityMode());
- }
-
return LV;
}
diff --git a/test/CodeGenCXX/visibility.cpp b/test/CodeGenCXX/visibility.cpp
index 67ac098..8bf34eb 100644
--- a/test/CodeGenCXX/visibility.cpp
+++ b/test/CodeGenCXX/visibility.cpp
@@ -5,6 +5,21 @@
#define PROTECTED __attribute__((visibility("protected")))
#define DEFAULT __attribute__((visibility("default")))
+namespace test25 {
+ template<typename T>
+ struct X {
+ template<typename U>
+ struct definition {
+ };
+ };
+
+ class DEFAULT A { };
+
+ X<int>::definition<A> a;
+ // CHECK: @_ZN6test251aE = global
+ // CHECK-HIDDEN: @_ZN6test251aE = hidden global
+}
+
// CHECK: @_ZN5Test425VariableInHiddenNamespaceE = hidden global i32 10
// CHECK: @_ZN5Test71aE = hidden global
// CHECK: @_ZN5Test71bE = global
@@ -500,3 +515,52 @@ namespace PR11690_2 {
// CHECK: define weak_odr void
@_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv
// CHECK-HIDDEN: define weak_odr void
@_ZN9PR11690_23foo3zedINS_3bazENS0_3barEE3barEv
}
+
+namespace test23 {
+ // Having a template argument that is explicitly visible should not make
+ // the template instantiation visible.
+ template <typename T>
+ struct X {
+ static void f() {
+ }
+ };
+
+ class DEFAULT A;
+
+ void g() {
+ X<A> y;
+ y.f();
+ }
+ // CHECK: define linkonce_odr void @_ZN6test231XINS_1AEE1fEv
+ // CHECK-HIDDEN: define linkonce_odr hidden void @_ZN6test231XINS_1AEE1fEv
+}
+
+namespace PR12001 {
+ template <typename P1>
+ void Bind(const P1& p1) {
+ }
+
+ class DEFAULT Version { };
+
+ void f() {
+ Bind(Version());
+ }
+ // CHECK: define linkonce_odr void @_ZN7PR120014BindINS_7VersionEEEvRKT_
+ // CHECK-HIDDEN: define linkonce_odr hidden void
@_ZN7PR120014BindINS_7VersionEEEvRKT_
+}
+
+namespace test24 {
+ class DEFAULT A { };
+
+ struct S {
+ template <typename T>
+ void mem() {}
+ };
+
+ void test() {
+ S s;
+ s.mem<A>();
+ }
+ // CHECK: define linkonce_odr void @_ZN6test241S3memINS_1AEEEvv
+ // CHECK-HIDDEN: define linkonce_odr hidden void @_ZN6test241S3memINS_1AEEEvv
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
