[cfe-commits] r155103 - in /cfe/trunk: include/clang/AST/Decl.h lib/AST/Decl.cpp test/CodeGenCXX/visibility.cpp

[Rafael Espindola]

Author: rafael
Date: Thu Apr 19 00:50:08 2012
New Revision: 155103

URL: http://llvm.org/viewvc/llvm-project?rev=155103&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/AST/Decl.h
    cfe/trunk/lib/AST/Decl.cpp
    cfe/trunk/test/CodeGenCXX/visibility.cpp

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=155103&r1=155102&r2=155103&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Thu Apr 19 00:50:08 2012
@@ -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

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=155103&r1=155102&r2=155103&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Apr 19 00:50:08 2012
@@ -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;

Modified: cfe/trunk/test/CodeGenCXX/visibility.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/visibility.cpp?rev=155103&r1=155102&r2=155103&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/visibility.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/visibility.cpp Thu Apr 19 00:50:08 2012
@@ -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