In mergeVisibility, if we already have an explicit visibility, keep it.

This fixes the included testcase and lets us simplify the code a bit. It
does require using mergeWithMin when merging class information to its
members. Expand the comments to explain why that works.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@155103 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 026be082a2fb017ef7bc09d88379f7aa0d6f65f4..4dff03ed51995fb93125971a55020cd2e8757933 100644 (file)
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -262,15 +262,11 @@ public:
       if (visibility() < V)
         return;
 
-      // If one has explicit visibility and the other doesn't, keep the
-      // explicit one.
-      if (visibilityExplicit() && !E)
+      // If we have an explicit visibility, keep it
+      if (visibilityExplicit())
         return;
-      if (!visibilityExplicit() && E)
-        setVisibility(V, E);
 
-      // If both are explicit or both are implicit, keep the minimum.
-      setVisibility(minVisibility(visibility(), V), visibilityExplicit() || E);
+      setVisibility(V, E);
     }
     // Merge the visibility V, keeping the most restrictive one.
     // This is used for cases like merging the visibility of a template

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index f4c0aa3c6be84bce07b84f06e7b25e17b43fb0ed..8649ba26961825951d1bec602ad482438894b1a9 100644 (file)
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -490,8 +490,10 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, LVFlags F) {
     if (llvm::Optional<Visibility> Vis = D->getExplicitVisibility())
       LV.mergeVisibility(*Vis, true);
   }
-  // Ignore both global visibility and attributes when computing our
-  // parent's visibility if we already have an explicit one.
+
+  // If this class member has an explicit visibility attribute, the only
+  // thing that can change its visibility is the template arguments, so
+  // only look for them when processing the the class.
   LVFlags ClassF =  LV.visibilityExplicit() ?
     LVFlags::CreateOnlyDeclLinkage() : F;
 
@@ -523,9 +525,12 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, LVFlags F) {
       LV.mergeVisibility(HiddenVisibility, true);
   }
 
-  // Class members only have linkage if their class has external
-  // linkage.
-  LV.merge(getLVForDecl(cast<RecordDecl>(D->getDeclContext()), ClassF));
+  // If this member has an visibility attribute, ClassF will exclude
+  // attributes on the class or command line options, keeping only information
+  // about the template instantiation. If the member has no visibility
+  // attributes, mergeWithMin behaves like merge, so in both cases mergeWithMin
+  // produces the desired result.
+  LV.mergeWithMin(getLVForDecl(cast<RecordDecl>(D->getDeclContext()), ClassF));
   if (!isExternalLinkage(LV.linkage()))
     return LinkageInfo::none();
 
@@ -579,8 +584,7 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D, LVFlags F) {
     LinkageInfo TypeLV = getLVForType(VD->getType());
     if (TypeLV.linkage() != ExternalLinkage)
       LV.mergeLinkage(UniqueExternalLinkage);
-    if (!LV.visibilityExplicit())
-      LV.mergeVisibility(TypeLV);
+    LV.mergeVisibility(TypeLV);
   }
 
   return LV;

diff --git a/test/CodeGenCXX/visibility.cpp b/test/CodeGenCXX/visibility.cpp
index 59d02d615ddf3ee66b3901489d5e58b863c668b7..b3c61329afe07d3412e4f6825c5e4970c7f18ee8 100644 (file)
--- a/test/CodeGenCXX/visibility.cpp
+++ b/test/CodeGenCXX/visibility.cpp
@@ -28,6 +28,18 @@ namespace test28 {
   // CHECK-HIDDEN: @_ZN6test285myvecE = hidden global
 }
 
+namespace test29 {
+#pragma GCC visibility push(hidden)
+  struct RECT {
+    int top;
+  };
+  __attribute__ ((visibility ("default"))) extern RECT data_rect;
+  RECT data_rect = { -1};
+#pragma GCC visibility pop
+  // CHECK: @_ZN6test299data_rectE = global
+  // CHECK-HIDDEN: @_ZN6test299data_rectE = global
+}
+
 // CHECK: @_ZN5Test425VariableInHiddenNamespaceE = hidden global i32 10
 // CHECK: @_ZN5Test71aE = hidden global
 // CHECK: @_ZN5Test71bE = global