Re: C++ PATCH for c++/51930 (instantiation hidden despite visibility attribute)

2012-03-05 Thread Jason Merrill
While looking at the class variant of this issue, I noticed that some of 
the code in determine_visibility was wrong; template_class_depth only 
considers unbound template parameters, and the number we want is the 
total number of levels.  I've also adjusted the diagnostic for misplaced 
class attributes as manu requested.


Tested x86_64-pc-linux-pc, applying to trunk.


Re: C++ PATCH for c++/51930 (instantiation hidden despite visibility attribute)

2012-03-05 Thread Jason Merrill

On 03/05/2012 01:05 PM, Jason Merrill wrote:

While looking at the class variant of this issue, I noticed that some of
the code in determine_visibility was wrong; template_class_depth only
considers unbound template parameters, and the number we want is the
total number of levels. I've also adjusted the diagnostic for misplaced
class attributes as manu requested.

Tested x86_64-pc-linux-pc, applying to trunk.


...and here's the patch.
commit b3b1728e0f4f9d565b33f7d8c07f64b60d3503fe
Author: Jason Merrill ja...@redhat.com
Date:   Tue Feb 14 17:36:48 2012 -0800

	PR c++/51930
	* decl2.c (determine_visibility): Correct calculation of class
	args depth.
	* decl.c (check_tag_decl): Adjust warning.

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index c47f87c..a18b312 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4216,17 +4216,20 @@ check_tag_decl (cp_decl_specifier_seq *declspecs)
 error (%constexpr% cannot be used for type declarations);
 }
 
-  if (declspecs-attributes)
+  if (declspecs-attributes  warn_attributes)
 {
-  location_t loc = input_location;
+  location_t loc;
   if (!CLASSTYPE_TEMPLATE_INSTANTIATION (declared_type))
-	/* For a non-template class, use the name location; for a template
-	   class (an explicit instantiation), use the current location.  */
-	input_location = location_of (declared_type);
-  warning (0, attribute ignored in declaration of %q#T, declared_type);
-  warning (0, attribute for %q#T must follow the %qs keyword,
-	   declared_type, class_key_or_enum_as_string (declared_type));
-  input_location = loc;
+	/* For a non-template class, use the name location.  */
+	loc = location_of (declared_type);
+  else
+	/* For a template class (an explicit instantiation), use the
+	   current location.  */
+	loc = input_location;
+  warning_at (loc, OPT_Wattributes, attribute ignored in declaration 
+		  of %q#T, declared_type);
+  inform (loc, attribute for %q#T must follow the %qs keyword,
+	  declared_type, class_key_or_enum_as_string (declared_type));
 }
 
   return declared_type;
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index bdc962a..7eccf67 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -2181,12 +2181,8 @@ determine_visibility (tree decl)
 		  ? TYPE_ATTRIBUTES (TREE_TYPE (decl))
 		  : DECL_ATTRIBUTES (decl));
   
-  if (args != error_mark_node
-	  /* Template argument visibility outweighs #pragma or namespace
-	 visibility, but not an explicit attribute.  */
-	   !lookup_attribute (visibility, attribs))
+  if (args != error_mark_node)
 	{
-	  int depth = TMPL_ARGS_DEPTH (args);
 	  tree pattern = DECL_TEMPLATE_RESULT (TI_TEMPLATE (tinfo));
 
 	  if (!DECL_VISIBILITY_SPECIFIED (decl))
@@ -2202,10 +2198,31 @@ determine_visibility (tree decl)
 		}
 	}
 
-	  /* FIXME should TMPL_ARGS_DEPTH really return 1 for null input? */
-	  if (args  depth  template_class_depth (class_type))
-	/* Limit visibility based on its template arguments.  */
-	constrain_visibility_for_template (decl, args);
+	  if (args
+	  /* Template argument visibility outweighs #pragma or namespace
+		 visibility, but not an explicit attribute.  */
+	   !lookup_attribute (visibility, attribs))
+	{
+	  int depth = TMPL_ARGS_DEPTH (args);
+	  int class_depth = 0;
+	  if (class_type  CLASSTYPE_TEMPLATE_INFO (class_type))
+		class_depth = TMPL_ARGS_DEPTH (CLASSTYPE_TI_ARGS (class_type));
+	  if (DECL_VISIBILITY_SPECIFIED (decl))
+		{
+		  /* A class template member with explicit visibility
+		 overrides the class visibility, so we need to apply
+		 all the levels of template args directly.  */
+		  int i;
+		  for (i = 1; i = depth; ++i)
+		{
+		  tree lev = TMPL_ARGS_LEVEL (args, i);
+		  constrain_visibility_for_template (decl, lev);
+		}
+		}
+	  else if (depth  class_depth)
+		/* Limit visibility based on its template arguments.  */
+		constrain_visibility_for_template (decl, args);
+	}
 	}
 }
 
diff --git a/gcc/testsuite/g++.dg/ext/visibility/template11.C b/gcc/testsuite/g++.dg/ext/visibility/template11.C
new file mode 100644
index 000..fb47fe2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/visibility/template11.C
@@ -0,0 +1,20 @@
+// PR c++/51930
+// { dg-require-visibility  }
+// { dg-options -fvisibility=hidden }
+// { dg-final { scan-not-hidden _ZN13template_testI4testE8functionEv } }
+
+struct test { };
+
+templatetypename T
+struct template_test
+{
+  __attribute__((visibility(default)))
+  void function();
+};
+
+templatetypename T
+void template_testT::function() { }
+
+template
+struct __attribute__((visibility(default)))
+template_testtest;