commit c7e606086a8d44c4e84ac3d47289288534c79bc6
author Rafael Espindola <rafael.espindola@gmail.com> Thu Apr 19 05:50:08 2012 +0000
committer Rafael Espindola <rafael.espindola@gmail.com> Thu Apr 19 05:50:08 2012 +0000
tree 3151eaf97133cc018c81b439e5fa599e7378c5cb
parent 1c7eed7e6ea2b357aa94581a90018ddbdcb41cbf

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

include/clang/AST/Decl.h

diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h
index 026be08..4dff03e 100644
--- a/include/clang/AST/Decl.h
+++ b/include/clang/AST/Decl.h
@@ -262,15 +262,11 @@
       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

lib/AST/Decl.cpp

diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index f4c0aa3..8649ba2 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -490,8 +490,10 @@
     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 @@
       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 @@
     LinkageInfo TypeLV = getLVForType(VD->getType());
     if (TypeLV.linkage() != ExternalLinkage)
       LV.mergeLinkage(UniqueExternalLinkage);
-    if (!LV.visibilityExplicit())
-      LV.mergeVisibility(TypeLV);
+    LV.mergeVisibility(TypeLV);
   }
 
   return LV;

test/CodeGenCXX/visibility.cpp

diff --git a/test/CodeGenCXX/visibility.cpp b/test/CodeGenCXX/visibility.cpp
index 59d02d6..b3c6132 100644
--- a/test/CodeGenCXX/visibility.cpp
+++ b/test/CodeGenCXX/visibility.cpp
@@ -28,6 +28,18 @@
   // 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