Patchwork C++/c-common PATCH for c++/35688 (wrong visibility of template instantiation)

login
register
mail settings
Submitter Jason Merrill
Date Nov. 7, 2011, 5:42 p.m.
Message ID <4EB81892.4030502@redhat.com>
Download mbox | patch
Permalink /patch/124153/
State New
Headers show

Comments

Jason Merrill - Nov. 7, 2011, 5:42 p.m.
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.

Patch

commit 145941ce3947108c9572379a33b3f48e5c539146
Author: Jason Merrill <jason@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>());
+}