On 11/06/2011 11:31 PM, Jason Merrill wrote:
The function constrain_visibility_for_template tries to set the
visibility of a template instantiation properly by giving it the minimum
visibility of the template itself and the template arguments. But this
PR points out that we were failing to do that in the case that the
template is within a namespace with a visibility attribute, because then
it gets DECL_VISIBILITY_SPECIFIED. This patch fixes that by re-using
some of the C front end's visibility code, so that we can further
constrain visibility on a decl with DECL_VISIBILITY_SPECIFIED so long as
it doesn't actually have a visibility attribute.

Vincenzo pointed out that this still didn't fix the visibility of class template instantiations, and on further reflection I think we want lesser template argument visibility to override even an explicit attribute on the template. That's also what the documentation says. So I've reverted the previous approach and now just give template argument visibility higher priority.

Tested x86_64-pc-linux-gnu, applying to trunk.
commit 145941ce3947108c9572379a33b3f48e5c539146
Author: Jason Merrill <ja...@redhat.com>
Date:   Mon Nov 7 12:16:11 2011 -0500

    	PR c++/35688
    	* decl2.c (constrain_visibility): Return void.  Add tmpl parm
    	which gives the constraint priority over an attribute.
    	(constrain_visibility_for_template, determine_visibility): Adjust.
    	* pt.c (instantiate_class_template_1): Call determine_visibility.

diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 80fb0c3..17be3ad 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1954,10 +1954,12 @@ type_visibility (tree type)
 }
 
 /* Limit the visibility of DECL to VISIBILITY, if not explicitly
-   specified (or if VISIBILITY is static).  */
+   specified (or if VISIBILITY is static).  If TMPL is true, this
+   constraint is for a template argument, and takes precedence
+   over explicitly-specified visibility on the template.  */
 
-static bool
-constrain_visibility (tree decl, int visibility)
+static void
+constrain_visibility (tree decl, int visibility, bool tmpl)
 {
   if (visibility == VISIBILITY_ANON)
     {
@@ -1974,16 +1976,11 @@ constrain_visibility (tree decl, int visibility)
 	    DECL_NOT_REALLY_EXTERN (decl) = 1;
 	}
     }
-  /* We check decl_has_visibility_attr rather than
-     DECL_VISIBILITY_SPECIFIED here because we want other considerations
-     to override visibility from a namespace or #pragma.  */
   else if (visibility > DECL_VISIBILITY (decl)
-	   && !decl_has_visibility_attr (decl))
+	   && (tmpl || !DECL_VISIBILITY_SPECIFIED (decl)))
     {
       DECL_VISIBILITY (decl) = (enum symbol_visibility) visibility;
-      return true;
     }
-  return false;
 }
 
 /* Constrain the visibility of DECL based on the visibility of its template
@@ -2019,7 +2016,7 @@ constrain_visibility_for_template (tree decl, tree targs)
 	    }
 	}
       if (vis)
-	constrain_visibility (decl, vis);
+	constrain_visibility (decl, vis, true);
     }
 }
 
@@ -2132,7 +2129,7 @@ determine_visibility (tree decl)
 	  if (underlying_vis == VISIBILITY_ANON
 	      || (CLASS_TYPE_P (underlying_type)
 		  && CLASSTYPE_VISIBILITY_SPECIFIED (underlying_type)))
-	    constrain_visibility (decl, underlying_vis);
+	    constrain_visibility (decl, underlying_vis, false);
 	  else
 	    DECL_VISIBILITY (decl) = VISIBILITY_DEFAULT;
 	}
@@ -2140,7 +2137,7 @@ determine_visibility (tree decl)
 	{
 	  /* tinfo visibility is based on the type it's for.  */
 	  constrain_visibility
-	    (decl, type_visibility (TREE_TYPE (DECL_NAME (decl))));
+	    (decl, type_visibility (TREE_TYPE (DECL_NAME (decl))), false);
 
 	  /* Give the target a chance to override the visibility associated
 	     with DECL.  */
@@ -2207,14 +2204,14 @@ determine_visibility (tree decl)
   if (decl_anon_ns_mem_p (decl))
     /* Names in an anonymous namespace get internal linkage.
        This might change once we implement export.  */
-    constrain_visibility (decl, VISIBILITY_ANON);
+    constrain_visibility (decl, VISIBILITY_ANON, false);
   else if (TREE_CODE (decl) != TYPE_DECL)
     {
       /* Propagate anonymity from type to decl.  */
       int tvis = type_visibility (TREE_TYPE (decl));
       if (tvis == VISIBILITY_ANON
 	  || ! DECL_VISIBILITY_SPECIFIED (decl))
-	constrain_visibility (decl, tvis);
+	constrain_visibility (decl, tvis, false);
     }
   else if (no_linkage_check (TREE_TYPE (decl), /*relaxed_p=*/true))
     /* DR 757: A type without linkage shall not be used as the type of a
@@ -2225,7 +2222,7 @@ determine_visibility (tree decl)
 
        Since non-extern "C" decls need to be defined in the same
        translation unit, we can make the type internal.  */
-    constrain_visibility (decl, VISIBILITY_ANON);
+    constrain_visibility (decl, VISIBILITY_ANON, false);
 
   /* If visibility changed and DECL already has DECL_RTL, ensure
      symbol flags are updated.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 493e3e6..52f4d47 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -8600,6 +8600,8 @@ instantiate_class_template_1 (tree type)
     {
       CLASSTYPE_VISIBILITY_SPECIFIED (type) = 1;
       CLASSTYPE_VISIBILITY (type) = CLASSTYPE_VISIBILITY (pattern);
+      /* Adjust visibility for template arguments.  */
+      determine_visibility (TYPE_MAIN_DECL (type));
     }
   CLASSTYPE_FINAL (type) = CLASSTYPE_FINAL (pattern);
 
diff --git a/gcc/testsuite/g++.dg/ext/visibility/template8.C b/gcc/testsuite/g++.dg/ext/visibility/template8.C
new file mode 100644
index 0000000..e491882
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/visibility/template8.C
@@ -0,0 +1,26 @@
+// PR c++/35688
+// { dg-require-visibility "" }
+// { dg-options "-fvisibility=hidden" }
+
+// { dg-final { scan-hidden "_Z1gI1BEvT_" } }
+// { dg-final { scan-hidden "_Z1gI1AI1BEEvT_" } }
+
+// Test that template argument visibility takes priority even over an
+// explicit visibility attribute on a template.
+
+template <class T>
+struct __attribute ((visibility ("default"))) A { };
+template <class T>
+void g(T) __attribute ((visibility ("default")));
+
+struct B { };
+
+template <class T>
+void g(T)
+{ }
+
+int main()
+{
+  g(B());
+  g(A<B>());
+}

Reply via email to