* [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization @ 2007-06-15 18:19 Simon Baldwin 2007-06-15 19:32 ` Mark Mitchell 0 siblings, 1 reply; 12+ messages in thread From: Simon Baldwin @ 2007-06-15 18:19 UTC (permalink / raw) To: gcc-patches; +Cc: mark 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 + } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization 2007-06-15 18:19 [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization Simon Baldwin @ 2007-06-15 19:32 ` Mark Mitchell 2007-06-15 20:08 ` Simon Baldwin 0 siblings, 1 reply; 12+ messages in thread From: Mark Mitchell @ 2007-06-15 19:32 UTC (permalink / raw) To: Simon Baldwin; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization 2007-06-15 19:32 ` Mark Mitchell @ 2007-06-15 20:08 ` Simon Baldwin 2007-06-15 22:44 ` Mark Mitchell 2007-08-15 18:50 ` [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) Jakub Jelinek 0 siblings, 2 replies; 12+ messages in thread From: Simon Baldwin @ 2007-06-15 20:08 UTC (permalink / raw) To: Mark Mitchell; +Cc: gcc-patches [-- 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 + } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization 2007-06-15 20:08 ` Simon Baldwin @ 2007-06-15 22:44 ` Mark Mitchell 2007-06-18 22:33 ` Simon Baldwin 2007-08-15 18:50 ` [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) Jakub Jelinek 1 sibling, 1 reply; 12+ messages in thread From: Mark Mitchell @ 2007-06-15 22:44 UTC (permalink / raw) To: Simon Baldwin; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization 2007-06-15 22:44 ` Mark Mitchell @ 2007-06-18 22:33 ` Simon Baldwin 0 siblings, 0 replies; 12+ messages in thread From: Simon Baldwin @ 2007-06-18 22:33 UTC (permalink / raw) To: Mark Mitchell; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) 2007-06-15 20:08 ` Simon Baldwin 2007-06-15 22:44 ` Mark Mitchell @ 2007-08-15 18:50 ` Jakub Jelinek 2007-08-15 19:08 ` Paolo Carlini ` (2 more replies) 1 sibling, 3 replies; 12+ messages in thread From: Jakub Jelinek @ 2007-08-15 18:50 UTC (permalink / raw) To: Mark Mitchell, Simon Baldwin; +Cc: gcc-patches 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) 2007-08-15 18:50 ` [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) Jakub Jelinek @ 2007-08-15 19:08 ` Paolo Carlini 2007-08-17 4:20 ` Mark Mitchell 2007-08-27 16:02 ` Jason Merrill 2 siblings, 0 replies; 12+ messages in thread From: Paolo Carlini @ 2007-08-15 19:08 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Mark Mitchell, Simon Baldwin, gcc-patches ... by the way, c++/32400 seems closely related, and indeed Jakub your patch fixes both! Thanks, Paolo. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) 2007-08-15 18:50 ` [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) Jakub Jelinek 2007-08-15 19:08 ` Paolo Carlini @ 2007-08-17 4:20 ` Mark Mitchell 2007-08-17 16:03 ` Jakub Jelinek 2007-08-27 16:02 ` Jason Merrill 2 siblings, 1 reply; 12+ messages in thread From: Mark Mitchell @ 2007-08-17 4:20 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Simon Baldwin, gcc-patches 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) 2007-08-17 4:20 ` Mark Mitchell @ 2007-08-17 16:03 ` Jakub Jelinek 0 siblings, 0 replies; 12+ messages in thread From: Jakub Jelinek @ 2007-08-17 16:03 UTC (permalink / raw) To: Mark Mitchell; +Cc: Simon Baldwin, gcc-patches 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) 2007-08-15 18:50 ` [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) Jakub Jelinek 2007-08-15 19:08 ` Paolo Carlini 2007-08-17 4:20 ` Mark Mitchell @ 2007-08-27 16:02 ` Jason Merrill 2007-08-28 11:58 ` Jakub Jelinek 2 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2007-08-27 16:02 UTC (permalink / raw) To: Jakub Jelinek Cc: Mark Mitchell, Simon Baldwin, gcc-patches@gcc.gnu.org >> gcc-patches List 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) 2007-08-27 16:02 ` Jason Merrill @ 2007-08-28 11:58 ` Jakub Jelinek 2007-08-28 13:49 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Jakub Jelinek @ 2007-08-28 11:58 UTC (permalink / raw) To: Jason Merrill; +Cc: Mark Mitchell, Simon Baldwin, gcc-patches 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) 2007-08-28 11:58 ` Jakub Jelinek @ 2007-08-28 13:49 ` Jason Merrill 0 siblings, 0 replies; 12+ messages in thread From: Jason Merrill @ 2007-08-28 13:49 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Mark Mitchell, Simon Baldwin, gcc-patches OK. Jason ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2007-08-28 13:44 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2007-06-15 18:19 [REVISED PATCH][RFC] Fix PR c++/31923: Storage class with explicit template specialization Simon Baldwin 2007-06-15 19:32 ` Mark Mitchell 2007-06-15 20:08 ` Simon Baldwin 2007-06-15 22:44 ` Mark Mitchell 2007-06-18 22:33 ` Simon Baldwin 2007-08-15 18:50 ` [C++ PATCH] Fix check_explicit_specialization (PR c++/32596, regression caused by PR c++/31923) Jakub Jelinek 2007-08-15 19:08 ` Paolo Carlini 2007-08-17 4:20 ` Mark Mitchell 2007-08-17 16:03 ` Jakub Jelinek 2007-08-27 16:02 ` Jason Merrill 2007-08-28 11:58 ` Jakub Jelinek 2007-08-28 13:49 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).