[REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization

[REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization

[Simon Baldwin]

Attached is a revised version of a pending patch first distributed as

  http://gcc.gnu.org/ml/gcc-patches/2007-05/msg02033.html

The attached patch implements section 7.1.1-1 of the C++ standard by
preventing storage class specifiers on explicit specializations of templated
functions.  It also adjusts the linkage of such explicit specializations that
now cannot specify a storage class so that they meet the recommendations of
the C++ Core Language Working Group for Defect Report 605:

  http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#605

The patch includes two new tests, to verify parsing and linkage of explicit
specializations of templated functions.

Regression tested with the full g++ testsuite.


gcc/cp/ChangeLog
2007-06-15  Simon Baldwin <simonb@google.com>

	PR c++/31923
	* parser.c (cp_parser_single_declaration): Added check for storage
	class other than sc_none in parsed declaration, and a flag to indicate
	if the call is part of an explicit template specialization parse.
	* (cp_parser_explicit_specialization): Specialization check flag added
	to call to cp_parser_single_declaration(), set true.
	* (cp_parser_template_declaration_after_export): Specialization check
	flag added to call to cp_parser_single_declaration(), set false.
	* pt.c (check_explicit_specialization): Added code to copy visiblity
	and linkage from the templated function to the explicit specialization.

gcc/testsuite/ChangeLog
2007-06-15  Simon Baldwin <simonb@google.com>

	PR c++/31923
	* g++.dg/template/error25.C: New.
	* g++.dg/template/spec35.C: New.


diff -Nrcp3 gcc_orig/gcc/cp/parser.c gcc/gcc/cp/parser.c
*** gcc_orig/gcc/cp/parser.c	Tue Jun 12 10:21:25 2007
--- gcc/gcc/cp/parser.c	Fri Jun 15 09:45:58 2007
*************** static void cp_parser_template_declarati
*** 1913,1919 ****
  static void cp_parser_perform_template_parameter_access_checks
    (VEC (deferred_access_check,gc)*);
  static tree cp_parser_single_declaration
!   (cp_parser *, VEC (deferred_access_check,gc)*, bool, bool *);
  static tree cp_parser_functional_cast
    (cp_parser *, tree);
  static tree cp_parser_save_member_function_body
--- 1913,1919 ----
  static void cp_parser_perform_template_parameter_access_checks
    (VEC (deferred_access_check,gc)*);
  static tree cp_parser_single_declaration
!   (cp_parser *, VEC (deferred_access_check,gc)*, bool, bool, bool *);
  static tree cp_parser_functional_cast
    (cp_parser *, tree);
  static tree cp_parser_save_member_function_body
*************** cp_parser_explicit_specialization (cp_pa
*** 10225,10230 ****
--- 10225,10231 ----
      cp_parser_single_declaration (parser,
  				  /*checks=*/NULL,
  				  /*member_p=*/false,
+                                   /*explicit_specialization_p=*/true,
  				  /*friend_p=*/NULL);
    /* We're done with the specialization.  */
    end_specialization ();
*************** cp_parser_template_declaration_after_exp
*** 16510,16515 ****
--- 16511,16517 ----
        decl = cp_parser_single_declaration (parser,
  					   checks,
  					   member_p,
+                                            /*explicit_specialization_p=*/false,
  					   &friend_p);
        pop_deferring_access_checks ();
  
*************** static tree
*** 16575,16580 ****
--- 16577,16583 ----
  cp_parser_single_declaration (cp_parser* parser,
  			      VEC (deferred_access_check,gc)* checks,
  			      bool member_p,
+                               bool explicit_specialization_p,
  			      bool* friend_p)
  {
    int declares_class_or_enum;
*************** cp_parser_single_declaration (cp_parser*
*** 16648,16660 ****
    if (!decl
        && (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON)
  	  || decl_specifiers.type != error_mark_node))
!     decl = cp_parser_init_declarator (parser,
! 				      &decl_specifiers,
! 				      checks,
! 				      /*function_definition_allowed_p=*/true,
! 				      member_p,
! 				      declares_class_or_enum,
! 				      &function_definition_p);
  
    pop_deferring_access_checks ();
  
--- 16651,16677 ----
    if (!decl
        && (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON)
  	  || decl_specifiers.type != error_mark_node))
!     {
!       decl = cp_parser_init_declarator (parser,
! 				        &decl_specifiers,
! 				        checks,
! 				        /*function_definition_allowed_p=*/true,
! 				        member_p,
! 				        declares_class_or_enum,
! 				        &function_definition_p);
! 
!     /* 7.1.1-1 [dcl.stc]
! 
!        A storage-class-specifier shall not be specified in an explicit
!        specialization...  */
!     if (decl
!         && explicit_specialization_p
!         && decl_specifiers.storage_class != sc_none)
!       {
!         error ("explicit template specialization cannot have a storage class");
!         decl = error_mark_node;
!       }
!     }
  
    pop_deferring_access_checks ();
  
diff -Nrcp3 gcc_orig/gcc/cp/pt.c gcc/gcc/cp/pt.c
*** gcc_orig/gcc/cp/pt.c	Tue Jun 12 10:21:25 2007
--- gcc/gcc/cp/pt.c	Fri Jun 15 10:40:03 2007
*************** check_explicit_specialization (tree decl
*** 2193,2198 ****
--- 2193,2227 ----
  	  TREE_PRIVATE (decl) = TREE_PRIVATE (gen_tmpl);
  	  TREE_PROTECTED (decl) = TREE_PROTECTED (gen_tmpl);
  
+           /* 7.1.1-1 [dcl.stc]
+ 
+              A storage-class-specifier shall not be specified in an
+              explicit specialization...
+ 
+              The parser rejects these, so unless action is taken here,
+              explicit function specializations will always appear with
+              global linkage.
+ 
+              The action recommended by the C++ CWG in response to C++
+              defect report 605 is to make the storage class and linkage
+              of the explicit specialization match the templated function:
+ 
+              http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#605
+            */
+           if (tsk == tsk_expl_spec
+               && TREE_CODE (decl) == FUNCTION_DECL
+               && DECL_TEMPLATE_INFO (decl) && DECL_USE_TEMPLATE (decl)
+               && DECL_FUNCTION_TEMPLATE_P (gen_tmpl))
+             {
+               tree tmpl_func = DECL_TEMPLATE_RESULT (gen_tmpl);
+               gcc_assert (TREE_CODE (tmpl_func) == FUNCTION_DECL);
+ 
+               /* This specialization has the same linkage and visiblity as
+                  the function template it specializes.  */
+               TREE_PUBLIC (decl) = TREE_PUBLIC (tmpl_func);
+               DECL_THIS_STATIC (decl) = DECL_THIS_STATIC (tmpl_func);
+             }
+ 
  	  /* If DECL is a friend declaration, declared using an
  	     unqualified name, the namespace associated with DECL may
  	     have been set incorrectly.  For example, in:
diff -Nrcp3 gcc_orig/gcc/testsuite/g++.dg/template/error25.C gcc/gcc/testsuite/g++.dg/template/error25.C
*** gcc_orig/gcc/testsuite/g++.dg/template/error25.C	Wed Dec 31 16:00:00 1969
--- gcc/gcc/testsuite/g++.dg/template/error25.C	Tue Jun 12 14:02:38 2007
***************
*** 0 ****
--- 1,16 ----
+ // PR c++/31923
+ 
+ template<class T>
+ static void f1 ();
+ 
+ template<>
+ static void f1<void> ();  // { dg-error "explicit template specialization cannot have a storage class" }
+ 
+ template<class T>
+ extern void f2 ();
+ 
+ template<>
+ extern void f2<void> ();  // { dg-error "explicit template specialization cannot have a storage class" }
+ 
+ export template<class T>  // { dg-warning "keyword 'export' not implemented" }
+ static void* f3 ();
diff -Nrcp3 gcc_orig/gcc/testsuite/g++.dg/template/spec35.C gcc/gcc/testsuite/g++.dg/template/spec35.C
*** gcc_orig/gcc/testsuite/g++.dg/template/spec35.C	Wed Dec 31 16:00:00 1969
--- gcc/gcc/testsuite/g++.dg/template/spec35.C	Fri Jun 15 10:35:33 2007
***************
*** 0 ****
--- 1,29 ----
+ // PR c++/31923
+ // C++ DR 605 -- "...the linkage of an explicit specialization must be that of
+ // the template."
+ 
+ // { dg-do compile { target i?86-*-* x86_64-*-* } }
+ // { dg-require-weak "" }
+ 
+ template<class T>
+ static void f1 (T) { }
+ 
+ // { dg-final { scan-assembler-not ".glob(a|)l\[\t \]*_Z2f1IfEvT_" } }
+ template<>
+ void f1<float> (float) { }  // Expected to have static linkage
+ 
+ template<class T>
+ void f2 (T) { }
+ 
+ // { dg-final { scan-assembler ".glob(a|)l\[\t \]*_Z2f2IfEvT_" } }
+ template<>
+ void f2<float> (float) { }  // Expected to have global linkage
+ 
+ void instantiator ()
+ {
+   // { dg-final { scan-assembler-not ".glob(a|)l\[\t \]*_Z2f1IiEvT_" } }
+   f1(0);  // Expected to have static linkage
+ 
+   // { dg-final { scan-assembler ".weak\[\t \]*_Z2f2IiEvT_" } }
+   f2(0);  // Expected to have weak global linkage
+ }

Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization

[Mark Mitchell]

Simon Baldwin wrote:

> gcc/cp/ChangeLog
> 2007-06-15  Simon Baldwin <simonb@google.com>
> 
> 	PR c++/31923
> 	* parser.c (cp_parser_single_declaration): Added check for storage
> 	class other than sc_none in parsed declaration, and a flag to indicate
> 	if the call is part of an explicit template specialization parse.
> 	* (cp_parser_explicit_specialization): Specialization check flag added
> 	to call to cp_parser_single_declaration(), set true.
> 	* (cp_parser_template_declaration_after_export): Specialization check
> 	flag added to call to cp_parser_single_declaration(), set false.
> 	* pt.c (check_explicit_specialization): Added code to copy visiblity
> 	and linkage from the templated function to the explicit specialization.

This looks good.  A couple of questions:

> +           if (tsk == tsk_expl_spec
> +               && TREE_CODE (decl) == FUNCTION_DECL
> +               && DECL_TEMPLATE_INFO (decl) && DECL_USE_TEMPLATE (decl)
> +               && DECL_FUNCTION_TEMPLATE_P (gen_tmpl))
> +             {
> +               tree tmpl_func = DECL_TEMPLATE_RESULT (gen_tmpl);
> +               gcc_assert (TREE_CODE (tmpl_func) == FUNCTION_DECL);
> + 
> +               /* This specialization has the same linkage and visiblity as
> +                  the function template it specializes.  */
> +               TREE_PUBLIC (decl) = TREE_PUBLIC (tmpl_func);
> +               DECL_THIS_STATIC (decl) = DECL_THIS_STATIC (tmpl_func);
> +             }

The condition seems complicated.  What are you trying to check exactly?
 Why isn't just

  tsk == tsk_expl_spec && DECL_FUNCTION_TEMPLATE_P (gen_tmpl)

enough?

Also, shouldn't we copy ELF symbol visibility here too?

> +   // { dg-final { scan-assembler-not ".glob(a|)l\[\t \]*_Z2f1IiEvT_" } }
> +   f1(0);  // Expected to have static linkage
> + 
> +   // { dg-final { scan-assembler ".weak\[\t \]*_Z2f2IiEvT_" } }
> +   f2(0);  // Expected to have weak global linkage

Do we have a DejaGNU function to check for weakness?  If not, we should.
 And, we may as well add one to check for external/global-ness too.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization

[Simon Baldwin]

[-- Attachment #1: Type: text/plain, Size: 2758 bytes --]

Thanks for the review; inlined stuff below...

Mark Mitchell wrote:

> ...
>
>>+           if (tsk == tsk_expl_spec
>>+               && TREE_CODE (decl) == FUNCTION_DECL
>>+               && DECL_TEMPLATE_INFO (decl) && DECL_USE_TEMPLATE (decl)
>>+               && DECL_FUNCTION_TEMPLATE_P (gen_tmpl))
>>+             {
>>+               tree tmpl_func = DECL_TEMPLATE_RESULT (gen_tmpl);
>>+               gcc_assert (TREE_CODE (tmpl_func) == FUNCTION_DECL);
>>+ 
>>+               /* This specialization has the same linkage and visiblity as
>>+                  the function template it specializes.  */
>>+               TREE_PUBLIC (decl) = TREE_PUBLIC (tmpl_func);
>>+               DECL_THIS_STATIC (decl) = DECL_THIS_STATIC (tmpl_func);
>>+             }
>>    
>>
>
>The condition seems complicated.  What are you trying to check exactly?
> Why isn't just
>
>  tsk == tsk_expl_spec && DECL_FUNCTION_TEMPLATE_P (gen_tmpl)
>
>enough?
>  
>

Thanks for the note.  I triangulated to this condition through a series 
of swipes at narrowing down problems with early versions, which may 
explain any redundancies.  I wanted complete certainty here that this 
code wouldn't be triggered for explicit specializations of class 
templates, but only for function templates, and I wasn't entirely sure 
from the surrounding code that this would reliably be the case without 
checking.

If we're sure that decl here is always a FUNCTION_DECL then the simpler 
condition ought to be fine.  I've retested with this change in place, 
and all regression tests continue to pass.

>Also, shouldn't we copy ELF symbol visibility here too?
>  
>

Added.  Thanks for the note.  Revised patch appended below.

>>+   // { dg-final { scan-assembler-not ".glob(a|)l\[\t \]*_Z2f1IiEvT_" } }
>>+   f1(0);  // Expected to have static linkage
>>+ 
>>+   // { dg-final { scan-assembler ".weak\[\t \]*_Z2f2IiEvT_" } }
>>+   f2(0);  // Expected to have weak global linkage
>>    
>>
>
>Do we have a DejaGNU function to check for weakness?  If not, we should.
> And, we may as well add one to check for external/global-ness too.
>  
>

I couldn't find anything in DejaGNU to check either of these 
explicitly.  Several existing tests currently use scan-assembler in a 
similar way (g++.dg/rtti/tinfo1.C, g++.dg/ext/visibility/anon{1,2}.C, 
obj-c++.dg/template-{5,6}.mm, and g++.dg/abi/rtti3.C), suggesting that 
there's nothing available at the moment.

I could perhaps add suitable DejaGNU functions, but TCL/expect aren't my 
strong points.  Adding these functions might also be tricky for multiple 
architectures.  I didn't want this g++ patch to get hung up on testing 
framework changes that I might not be able to complete.  Can we live 
with this test for now?

Thanks,

--S


[-- Attachment #2: pr31923.patch --]
[-- Type: text/x-patch, Size: 8726 bytes --]

Attached is a revised version of a pending patch first distributed as

  http://gcc.gnu.org/ml/gcc-patches/2007-05/msg02033.html

The attached patch implements section 7.1.1-1 of the C++ standard by
preventing storage class specifiers on explicit specializations of templated
functions.  It also adjusts the linkage of such explicit specializations that
now cannot specify a storage class so that they meet the recommendations of
the C++ Core Language Working Group for Defect Report 605:

  http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#605

The patch includes two new tests, to verify parsing and linkage of explicit
specializations of templated functions.

Regression tested with the full g++ testsuite.


gcc/cp/ChangeLog
2007-06-15  Simon Baldwin <simonb@google.com>

	PR c++/31923
	* parser.c (cp_parser_single_declaration): Added check for storage
	class other than sc_none in parsed declaration, and a flag to indicate
	if the call is part of an explicit template specialization parse.
	* (cp_parser_explicit_specialization): Specialization check flag added
	to call to cp_parser_single_declaration(), set true.
	* (cp_parser_template_declaration_after_export): Specialization check
	flag added to call to cp_parser_single_declaration(), set false.
	* pt.c (check_explicit_specialization): Added code to copy visiblity
	and linkage from the templated function to the explicit specialization.

gcc/testsuite/ChangeLog
2007-06-15  Simon Baldwin <simonb@google.com>

	PR c++/31923
	* g++.dg/template/error25.C: New.
	* g++.dg/template/spec35.C: New.


diff -Nrcp3 gcc_orig/gcc/cp/parser.c gcc/gcc/cp/parser.c
*** gcc_orig/gcc/cp/parser.c	Tue Jun 12 10:21:25 2007
--- gcc/gcc/cp/parser.c	Fri Jun 15 09:45:58 2007
*************** static void cp_parser_template_declarati
*** 1913,1919 ****
  static void cp_parser_perform_template_parameter_access_checks
    (VEC (deferred_access_check,gc)*);
  static tree cp_parser_single_declaration
!   (cp_parser *, VEC (deferred_access_check,gc)*, bool, bool *);
  static tree cp_parser_functional_cast
    (cp_parser *, tree);
  static tree cp_parser_save_member_function_body
--- 1913,1919 ----
  static void cp_parser_perform_template_parameter_access_checks
    (VEC (deferred_access_check,gc)*);
  static tree cp_parser_single_declaration
!   (cp_parser *, VEC (deferred_access_check,gc)*, bool, bool, bool *);
  static tree cp_parser_functional_cast
    (cp_parser *, tree);
  static tree cp_parser_save_member_function_body
*************** cp_parser_explicit_specialization (cp_pa
*** 10225,10230 ****
--- 10225,10231 ----
      cp_parser_single_declaration (parser,
  				  /*checks=*/NULL,
  				  /*member_p=*/false,
+                                   /*explicit_specialization_p=*/true,
  				  /*friend_p=*/NULL);
    /* We're done with the specialization.  */
    end_specialization ();
*************** cp_parser_template_declaration_after_exp
*** 16510,16515 ****
--- 16511,16517 ----
        decl = cp_parser_single_declaration (parser,
  					   checks,
  					   member_p,
+                                            /*explicit_specialization_p=*/false,
  					   &friend_p);
        pop_deferring_access_checks ();
  
*************** static tree
*** 16575,16580 ****
--- 16577,16583 ----
  cp_parser_single_declaration (cp_parser* parser,
  			      VEC (deferred_access_check,gc)* checks,
  			      bool member_p,
+                               bool explicit_specialization_p,
  			      bool* friend_p)
  {
    int declares_class_or_enum;
*************** cp_parser_single_declaration (cp_parser*
*** 16648,16660 ****
    if (!decl
        && (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON)
  	  || decl_specifiers.type != error_mark_node))
!     decl = cp_parser_init_declarator (parser,
! 				      &decl_specifiers,
! 				      checks,
! 				      /*function_definition_allowed_p=*/true,
! 				      member_p,
! 				      declares_class_or_enum,
! 				      &function_definition_p);
  
    pop_deferring_access_checks ();
  
--- 16651,16677 ----
    if (!decl
        && (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON)
  	  || decl_specifiers.type != error_mark_node))
!     {
!       decl = cp_parser_init_declarator (parser,
! 				        &decl_specifiers,
! 				        checks,
! 				        /*function_definition_allowed_p=*/true,
! 				        member_p,
! 				        declares_class_or_enum,
! 				        &function_definition_p);
! 
!     /* 7.1.1-1 [dcl.stc]
! 
!        A storage-class-specifier shall not be specified in an explicit
!        specialization...  */
!     if (decl
!         && explicit_specialization_p
!         && decl_specifiers.storage_class != sc_none)
!       {
!         error ("explicit template specialization cannot have a storage class");
!         decl = error_mark_node;
!       }
!     }
  
    pop_deferring_access_checks ();
  
diff -Nrcp3 gcc_orig/gcc/cp/pt.c gcc/gcc/cp/pt.c
*** gcc_orig/gcc/cp/pt.c	Tue Jun 12 10:21:25 2007
--- gcc/gcc/cp/pt.c	Fri Jun 15 12:06:21 2007
*************** check_explicit_specialization (tree decl
*** 2193,2198 ****
--- 2193,2229 ----
  	  TREE_PRIVATE (decl) = TREE_PRIVATE (gen_tmpl);
  	  TREE_PROTECTED (decl) = TREE_PROTECTED (gen_tmpl);
  
+           /* 7.1.1-1 [dcl.stc]
+ 
+              A storage-class-specifier shall not be specified in an
+              explicit specialization...
+ 
+              The parser rejects these, so unless action is taken here,
+              explicit function specializations will always appear with
+              global linkage.
+ 
+              The action recommended by the C++ CWG in response to C++
+              defect report 605 is to make the storage class and linkage
+              of the explicit specialization match the templated function:
+ 
+              http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#605
+            */
+           if (tsk == tsk_expl_spec && DECL_FUNCTION_TEMPLATE_P (gen_tmpl))
+             {
+               tree tmpl_func = DECL_TEMPLATE_RESULT (gen_tmpl);
+               gcc_assert (TREE_CODE (tmpl_func) == FUNCTION_DECL);
+ 
+               /* This specialization has the same linkage and visiblity as
+                  the function template it specializes.  */
+               TREE_PUBLIC (decl) = TREE_PUBLIC (tmpl_func);
+               DECL_THIS_STATIC (decl) = DECL_THIS_STATIC (tmpl_func);
+               if (DECL_VISIBILITY_SPECIFIED (tmpl_func))
+                 {
+                   DECL_VISIBILITY_SPECIFIED (decl) = 1;
+                   DECL_VISIBILITY (decl) = DECL_VISIBILITY (tmpl_func);
+                 }
+             }
+ 
  	  /* If DECL is a friend declaration, declared using an
  	     unqualified name, the namespace associated with DECL may
  	     have been set incorrectly.  For example, in:
diff -Nrcp3 gcc_orig/gcc/testsuite/g++.dg/template/error25.C gcc/gcc/testsuite/g++.dg/template/error25.C
*** gcc_orig/gcc/testsuite/g++.dg/template/error25.C	Wed Dec 31 16:00:00 1969
--- gcc/gcc/testsuite/g++.dg/template/error25.C	Tue Jun 12 14:02:38 2007
***************
*** 0 ****
--- 1,16 ----
+ // PR c++/31923
+ 
+ template<class T>
+ static void f1 ();
+ 
+ template<>
+ static void f1<void> ();  // { dg-error "explicit template specialization cannot have a storage class" }
+ 
+ template<class T>
+ extern void f2 ();
+ 
+ template<>
+ extern void f2<void> ();  // { dg-error "explicit template specialization cannot have a storage class" }
+ 
+ export template<class T>  // { dg-warning "keyword 'export' not implemented" }
+ static void* f3 ();
diff -Nrcp3 gcc_orig/gcc/testsuite/g++.dg/template/spec35.C gcc/gcc/testsuite/g++.dg/template/spec35.C
*** gcc_orig/gcc/testsuite/g++.dg/template/spec35.C	Wed Dec 31 16:00:00 1969
--- gcc/gcc/testsuite/g++.dg/template/spec35.C	Fri Jun 15 12:33:28 2007
***************
*** 0 ****
--- 1,29 ----
+ // PR c++/31923
+ // C++ DR 605 -- "...the linkage of an explicit specialization must be that of
+ // the template."
+ 
+ // { dg-require-weak "" }
+ // { dg-do compile { target i?86-*-* x86_64-*-* } }
+ 
+ template<class T>
+ static void f1 (T) { }
+ 
+ // { dg-final { scan-assembler-not ".glob(a|)l\[\t \]*_Z2f1IfEvT_" } }
+ template<>
+ void f1<float> (float) { }  // Expected to have static linkage
+ 
+ template<class T>
+ void f2 (T) { }
+ 
+ // { dg-final { scan-assembler ".glob(a|)l\[\t \]*_Z2f2IfEvT_" } }
+ template<>
+ void f2<float> (float) { }  // Expected to have global linkage
+ 
+ void instantiator ()
+ {
+   // { dg-final { scan-assembler-not ".glob(a|)l\[\t \]*_Z2f1IiEvT_" } }
+   f1(0);  // Expected to have static linkage
+ 
+   // { dg-final { scan-assembler ".weak\[\t \]*_Z2f2IiEvT_" } }
+   f2(0);  // Expected to have weak global linkage
+ }

Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization

[Mark Mitchell]

Simon Baldwin wrote:

> If we're sure that decl here is always a FUNCTION_DECL then the simpler
> condition ought to be fine.  I've retested with this change in place,
> and all regression tests continue to pass.

If GEN_TMPL (the most general template of which DECL is a
specialization) is DECL_FUNCTION_TEMPLATE_P then DECL had better be a
FUNCTION_DECL. :-)

> I could perhaps add suitable DejaGNU functions, but TCL/expect aren't my
> strong points.  Adding these functions might also be tricky for multiple
> architectures.  I didn't want this g++ patch to get hung up on testing
> framework changes that I might not be able to complete.  Can we live
> with this test for now?

This version is OK.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization

[Simon Baldwin]

Mark Mitchell wrote:

>Simon Baldwin wrote:
>  
>
> ...
>
>>I could perhaps add suitable DejaGNU functions, but TCL/expect aren't my
>>strong points.  Adding these functions might also be tricky for multiple
>>architectures.  I didn't want this g++ patch to get hung up on testing
>>framework changes that I might not be able to complete.  Can we live
>>with this test for now?
>>    
>>
>
>This version is OK.
>
>Thanks,
>  
>

Many thanks for the review.  Committed as revision 125829.

--S

[C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923)

[Jakub Jelinek]

Hi!

On Fri, Jun 15, 2007 at 12:55:54PM -0700, Simon Baldwin wrote:

2007-06-15  Simon Baldwin <simonb@google.com>

	PR c++/31923
	* parser.c (cp_parser_single_declaration): Added check for storage
	class other than sc_none in parsed declaration, and a flag to indicate
	if the call is part of an explicit template specialization parse.
	* (cp_parser_explicit_specialization): Specialization check flag added
	to call to cp_parser_single_declaration(), set true.
	* (cp_parser_template_declaration_after_export): Specialization check 
	flag added to call to cp_parser_single_declaration(), set false.
        * pt.c (check_explicit_specialization): Added code to copy visiblity
	and linkage from the templated function to the explicit specialization.

The above change causes PR c++/32596 regression.
When we copy TREE_PUBLIC from the template to the specialization and
the template is !TREE_PUBLIC because of constrain_visibility (xx, VISIBILITY_ANON),
it means the following determine_visibility will be a nop.  So we need
to ensure all other effects of constrain_visibility (xx, VISIBILITY_ANON)
are copied as well.
The following patch copies just DECL_INTERFACE_KNOWN which is enough to fix
this bug and didn't cause any make check-g++ or make check-libstdc++-v3
failure, but I wonder if we shouldn't copy DECL_NOT_REALLY_EXTERN as well,
or if DECL_INTERFACE_KNOWN can be copied over unconditionally.

BTW, the ChangeLog entry is badly formatted (replace "* (" with just "(").

2007-08-15  Jakub Jelinek  <jakub@redhat.com>

	PR c++/32596
	* pt.c (check_explicit_specialization): Set DECL_INTERFACE_KNOWN
	if tmpl_func is not public and has DECL_INTERFACE_KNOWN set.

	* g++.dg/ext/visibility/anon5.C: New test.

--- gcc/cp/pt.c.jj	2007-08-15 15:44:59.000000000 +0200
+++ gcc/cp/pt.c	2007-08-15 17:45:39.000000000 +0200
@@ -2215,6 +2215,8 @@ check_explicit_specialization (tree decl
               /* This specialization has the same linkage and visibility as
                  the function template it specializes.  */
               TREE_PUBLIC (decl) = TREE_PUBLIC (tmpl_func);
+	      if (! TREE_PUBLIC (decl) && DECL_INTERFACE_KNOWN (tmpl_func))
+		DECL_INTERFACE_KNOWN (decl) = 1;
               DECL_THIS_STATIC (decl) = DECL_THIS_STATIC (tmpl_func);
               if (DECL_VISIBILITY_SPECIFIED (tmpl_func))
                 {
--- gcc/testsuite/g++.dg/ext/visibility/anon5.C.jj	2007-08-15 17:47:03.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/visibility/anon5.C	2007-08-15 17:46:09.000000000 +0200
@@ -0,0 +1,8 @@
+// PR c++/32596
+// { dg-do compile }
+
+namespace
+{
+  template<class T> inline void char_less(void) { }
+  template<> inline void char_less<char>(void) { }
+}


	Jakub

Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923)

[Paolo Carlini]

... by the way, c++/32400 seems closely related, and indeed Jakub your 
patch fixes both!

Thanks,
Paolo.

Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923)

[Mark Mitchell]

Jakub Jelinek wrote:

> The above change causes PR c++/32596 regression.

> The following patch copies just DECL_INTERFACE_KNOWN which is enough to fix
> this bug and didn't cause any make check-g++ or make check-libstdc++-v3
> failure, but I wonder if we shouldn't copy DECL_NOT_REALLY_EXTERN as well,
> or if DECL_INTERFACE_KNOWN can be copied over unconditionally.

I don't think DECL_NOT_REALLY_EXTERN has any meaning for a template.
(It certainly does for an instantiation, but not for the template
itself.)  At least, I hope it doesn't have any meaning...

> 2007-08-15  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/32596
> 	* pt.c (check_explicit_specialization): Set DECL_INTERFACE_KNOWN
> 	if tmpl_func is not public and has DECL_INTERFACE_KNOWN set.
> 
> 	* g++.dg/ext/visibility/anon5.C: New test.

I think you should copy DECL_INTERFACE_KNOWN unconditionally.  If that
passes testing, this change is OK.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923)

[Jakub Jelinek]

On Thu, Aug 16, 2007 at 09:19:52PM -0700, Mark Mitchell wrote:
> > 2007-08-15  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR c++/32596
> > 	* pt.c (check_explicit_specialization): Set DECL_INTERFACE_KNOWN
> > 	if tmpl_func is not public and has DECL_INTERFACE_KNOWN set.
> > 
> > 	* g++.dg/ext/visibility/anon5.C: New test.
> 
> I think you should copy DECL_INTERFACE_KNOWN unconditionally.  If that
> passes testing, this change is OK.

Copying DECL_INTERFACE_KNOWN unconditionally, even when TREE_PUBLIC
(tmpl_func), causes many regressions.
As an example pretty3.C.

Here tmpl_func has TREE_PUBLIC set and DECL_INTERFACE_KNOWN set as well.
If we set DECL_INTERFACE_KNOWN on decl and reach
start_preparsed_function around line 11030:

  if (DECL_INTERFACE_KNOWN (decl1))
    {
      tree ctx = decl_function_context (decl1);

      if (DECL_NOT_REALLY_EXTERN (decl1))
        DECL_EXTERNAL (decl1) = 0;

      if (ctx != NULL_TREE && DECL_DECLARED_INLINE_P (ctx)
          && TREE_PUBLIC (ctx))
        /* This is a function in a local class in an extern inline
           function.  */
        comdat_linkage (decl1);
    }
  else if ...
  else if ...
  else
    {
      /* This is a definition, not a reference.
         So clear DECL_EXTERNAL.  */
      DECL_EXTERNAL (decl1) = 0;

      if ((DECL_DECLARED_INLINE_P (decl1)
           || DECL_TEMPLATE_INSTANTIATION (decl1))
          && ! DECL_INTERFACE_KNOWN (decl1)
          /* Don't try to defer nested functions for now.  */
          && ! decl_function_context (decl1))
        DECL_DEFER_OUTPUT (decl1) = 1;
      else
        DECL_INTERFACE_KNOWN (decl1) = 1;
    }

Without the unconditional copy of DECL_INTERFACE_KNOWN in
check_explicit_specialization this will clear DECL_EXTERNAL,
but as DECL_NOT_REALLY_EXTERN is not set (and no, tmpl_func
didn't have it set either, so copying that over wouldn't help)
if we set there DECL_INTERFACE_KNOWN, DECL_EXTERNAL will still
be set and thus the explicit instantiation is not emitted.
Other regressions:

FAIL: g++.dg/ext/pretty1.C scan-assembler int bar\\(T\\).*with T = int
FAIL: g++.dg/ext/pretty2.C (test for excess errors)
FAIL: g++.dg/ext/visibility/template4.C scan-hidden hidden[ \t_]*_Z3fooIdEvT_
FAIL: g++.dg/ext/visibility/template4.C scan-hidden hidden[ \t_]*_Z3fooIcEvT_
FAIL: g++.dg/ext/visibility/template4.C scan-hidden hidden[ \t_]*_Z3barIdEvT_
FAIL: g++.dg/ext/visibility/template4.C scan-hidden hidden[ \t_]*_Z3barIcEvT_
FAIL: g++.dg/template/spec29.C (test for excess errors)
FAIL: g++.dg/template/spec35.C scan-assembler .glob(a|)l[\t ]*_Z2f2IfEvT_
FAIL: g++.old-deja/g++.ext/pretty3.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/explicit23.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/explicit28.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/explicit51.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/explicit57.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/explicit64.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/explicit65.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/spec33.C (test for excess errors)
FAIL: g++.old-deja/g++.pt/ttp28.C (test for excess errors)

	Jakub

Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923)

[Jason Merrill]

Jakub Jelinek wrote:
> The following patch copies just DECL_INTERFACE_KNOWN which is enough to fix
> this bug and didn't cause any make check-g++ or make check-libstdc++-v3
> failure, but I wonder if we shouldn't copy DECL_NOT_REALLY_EXTERN as well,
> or if DECL_INTERFACE_KNOWN can be copied over unconditionally.

> +	      if (! TREE_PUBLIC (decl) && DECL_INTERFACE_KNOWN (tmpl_func))
> +		DECL_INTERFACE_KNOWN (decl) = 1;

The right fix is to do what grokfndecl does: if !TREE_PUBLIC, set 
DECL_INTERFACE_KNOWN and DECL_NOT_REALLY_EXTERN.  No need to check 
DECL_INTERFACE_KNOWN on the template.

Incidentally, it seems wrong to me to be propagating DECL_VISIBILITY 
here, since a specialization can have less visibility than its template.

Jason

Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923)

[Jakub Jelinek]

On Mon, Aug 27, 2007 at 11:56:38AM -0400, Jason Merrill wrote:
> Jakub Jelinek wrote:
> >The following patch copies just DECL_INTERFACE_KNOWN which is enough to fix
> >this bug and didn't cause any make check-g++ or make check-libstdc++-v3
> >failure, but I wonder if we shouldn't copy DECL_NOT_REALLY_EXTERN as well,
> >or if DECL_INTERFACE_KNOWN can be copied over unconditionally.
> 
> >+	      if (! TREE_PUBLIC (decl) && DECL_INTERFACE_KNOWN (tmpl_func))
> >+		DECL_INTERFACE_KNOWN (decl) = 1;
> 
> The right fix is to do what grokfndecl does: if !TREE_PUBLIC, set 
> DECL_INTERFACE_KNOWN and DECL_NOT_REALLY_EXTERN.  No need to check 
> DECL_INTERFACE_KNOWN on the template.

Following bootstraps on x86_64-linux and ia64-linux with no regressions.

2007-08-28  Jakub Jelinek  <jakub@redhat.com>

	PR c++/32596
	* pt.c (check_explicit_specialization): Set DECL_INTERFACE_KNOWN
	and DECL_NOT_REALLY_EXTERN if tmpl_func is not public.

	* g++.dg/ext/visibility/anon5.C: New test.

--- gcc/cp/pt.c.jj	2007-08-27 20:56:18.000000000 +0200
+++ gcc/cp/pt.c	2007-08-27 20:56:19.000000000 +0200
@@ -2217,6 +2217,11 @@ check_explicit_specialization (tree decl
               /* This specialization has the same linkage and visibility as
                  the function template it specializes.  */
               TREE_PUBLIC (decl) = TREE_PUBLIC (tmpl_func);
+	      if (! TREE_PUBLIC (decl))
+		{
+		  DECL_INTERFACE_KNOWN (decl) = 1;
+		  DECL_NOT_REALLY_EXTERN (decl) = 1;
+		}
               DECL_THIS_STATIC (decl) = DECL_THIS_STATIC (tmpl_func);
               if (DECL_VISIBILITY_SPECIFIED (tmpl_func))
                 {
--- gcc/testsuite/g++.dg/ext/visibility/anon5.C.jj	2007-08-15 17:47:03.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/visibility/anon5.C	2007-08-15 17:46:09.000000000 +0200
@@ -0,0 +1,8 @@
+// PR c++/32596
+// { dg-do compile }
+
+namespace
+{
+  template<class T> inline void char_less(void) { }
+  template<> inline void char_less<char>(void) { }
+}

> Incidentally, it seems wrong to me to be propagating DECL_VISIBILITY 
> here, since a specialization can have less visibility than its template.

If you want to change this instead, guess I'd pass the buck to you...

	Jakub

Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923)

[Jason Merrill]

OK.

Jason