[RFC] c++: Print function template parms when relevant (was: [PATCH v4] c++: Add gnu::diagnose_as attribute)

Message ID 1694479.jsd7nNDzyu@excalibur
State New
Headers
Series [RFC] c++: Print function template parms when relevant (was: [PATCH v4] c++: Add gnu::diagnose_as attribute) |

Commit Message

Matthias Kretz Nov. 8, 2021, 4:40 p.m. UTC
  On Tuesday, 17 August 2021 20:31:54 CET Jason Merrill wrote:
> > 2. Given a DECL_TI_ARGS tree, can I query whether an argument was deduced
> > or explicitly specified? I'm asking because I still consider diagnostics
> > of function templates unfortunate. `template <class T> void f()` is fine,
> > as is `void f(T) [with T = float]`, but `void f() [with T = float]` could
> > be better. I.e. if the template parameter appears somewhere in the
> > function parameter list, dump_template_parms would only produce noise.
> > If, however, the template parameter was given explicitly, it would be
> > nice if it could show up accordingly in diagnostics.
> 
> NON_DEFAULT_TEMPLATE_ARGS_COUNT has that information, though there are
> some issues with it.  Attached is my WIP from May to improve it
> somewhat, if that's interesting.

It is interesting. I used your patch to come up with the attached. Patch. I 
must say, I didn't try to read through all the cp/pt.c code to understand all 
of what you did there (which is why my ChangeLog entry says "Jason?"), but it 
works for me (and all of `make check`).

Anyway, I'd like to propose the following before finishing my diagnose_as 
patch. I believe it's useful to fix this part first. The diagnostic/default-
template-args-[12].C tests show a lot of examples of the intent of this patch. 
And the remaining changes to the testsuite show how it changes diagnostic 
output.

---------------------- 8< --------------------

The choice when to print a function template parameter was still
suboptimal. That's because sometimes the function template parameter
list only adds noise, while in other situations the lack of a function
template parameter list makes diagnostic messages hard to understand.

The general idea of this change is to print template parms wherever they
would appear in the source code as well. Thus, the diagnostics code
needs to know whether any template parameter was given explicitly.

Signed-off-by: Matthias Kretz <m.kretz@gsi.de>

gcc/testsuite/ChangeLog:

        * g++.dg/debug/dwarf2/template-params-12n.C: Optionally, allow
        DW_AT_default_value.
        * g++.dg/diagnostic/default-template-args-1.C: New.
        * g++.dg/diagnostic/default-template-args-2.C: New.
        * g++.dg/diagnostic/param-type-mismatch-2.C: Expect template
        parms in diagnostic.
        * g++.dg/ext/pretty1.C: Expect function template specialization
        to not pretty-print template parms.
        * g++.old-deja/g++.ext/pretty3.C: Ditto.
        * g++.old-deja/g++.pt/memtemp77.C: Ditto.
        * g++.dg/goacc/template.C: Expect function template parms for
        explicit arguments.
        * g++.dg/gomp/declare-variant-7.C: Expect no function template
        parms for deduced arguments.
        * g++.dg/template/error40.C: Expect only non-default template
        arguments in diagnostic.

gcc/cp/ChangeLog:

        * cp-tree.h (GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT): Return
        absolute value of stored constant.
        (EXPLICIT_TEMPLATE_ARGS_P): New.
        (SET_EXPLICIT_TEMPLATE_ARGS_P): New.
        (TFF_AS_PRIMARY): New constant.
        * error.c (get_non_default_template_args_count): Avoid
        GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT if
        NON_DEFAULT_TEMPLATE_ARGS_COUNT is a NULL_TREE. Make independent
        of flag_pretty_templates.
        (dump_template_bindings): Add flags parameter to be passed to
        get_non_default_template_args_count. Print only non-default
        template arguments.
        (dump_function_decl): Call dump_function_name and dump_type of
        the DECL_CONTEXT with specialized template and set
        TFF_AS_PRIMARY for their flags.
        (dump_function_name): Add and document conditions for calling
        dump_template_parms.
        (dump_template_parms): Print only non-default template
        parameters.
        * pt.c (determine_specialization): Jason?
        (template_parms_level_to_args): Jason?
        (copy_template_args): Jason?
        (fn_type_unification): Set EXPLICIT_TEMPLATE_ARGS_P on the
        template arguments tree if any template parameter was explicitly
        given.
        (type_unification_real): Jason?
        (get_partial_spec_bindings): Jason?
        (tsubst_template_args): Determine number of defaulted arguments
        from new argument vector, if possible.
---
 gcc/cp/cp-tree.h                              | 18 +++-
 gcc/cp/error.c                                | 83 ++++++++++++++-----
 gcc/cp/pt.c                                   | 58 +++++++++----
 .../g++.dg/debug/dwarf2/template-params-12n.C |  2 +-
 .../diagnostic/default-template-args-1.C      | 73 ++++++++++++++++
 .../diagnostic/default-template-args-2.C      | 37 +++++++++
 .../g++.dg/diagnostic/param-type-mismatch-2.C |  2 +-
 gcc/testsuite/g++.dg/ext/pretty1.C            |  2 +-
 gcc/testsuite/g++.dg/goacc/template.C         |  8 +-
 gcc/testsuite/g++.dg/gomp/declare-variant-7.C |  4 +-
 gcc/testsuite/g++.dg/template/error40.C       |  6 +-
 gcc/testsuite/g++.old-deja/g++.ext/pretty3.C  |  2 +-
 gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C |  2 +-
 13 files changed, 242 insertions(+), 55 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/default-template-args-1.C
 create mode 100644 gcc/testsuite/g++.dg/diagnostic/default-template-args-2.C
  

Comments

Matthias Kretz Nov. 8, 2021, 8 p.m. UTC | #1
I forgot to mention why I tagged it [RFC]: I needed one more bit of 
information on the template args TREE_VEC to encode EXPLICIT_TEMPLATE_ARGS_P. 
Its TREE_CHAIN already points to an integer constant denoting the number of 
non-default arguments, so I couldn't trivially replace that. Therefore, I used 
the sign of that integer. I was hoping to find a cleaner solution, though.

-Matthias

On Monday, 8 November 2021 17:40:44 CET Matthias Kretz wrote:
> On Tuesday, 17 August 2021 20:31:54 CET Jason Merrill wrote:
> > > 2. Given a DECL_TI_ARGS tree, can I query whether an argument was
> > > deduced
> > > or explicitly specified? I'm asking because I still consider diagnostics
> > > of function templates unfortunate. `template <class T> void f()` is
> > > fine,
> > > as is `void f(T) [with T = float]`, but `void f() [with T = float]`
> > > could
> > > be better. I.e. if the template parameter appears somewhere in the
> > > function parameter list, dump_template_parms would only produce noise.
> > > If, however, the template parameter was given explicitly, it would be
> > > nice if it could show up accordingly in diagnostics.
> > 
> > NON_DEFAULT_TEMPLATE_ARGS_COUNT has that information, though there are
> > some issues with it.  Attached is my WIP from May to improve it
> > somewhat, if that's interesting.
> 
> It is interesting. I used your patch to come up with the attached. Patch. I
> must say, I didn't try to read through all the cp/pt.c code to understand
> all of what you did there (which is why my ChangeLog entry says "Jason?"),
> but it works for me (and all of `make check`).
> 
> Anyway, I'd like to propose the following before finishing my diagnose_as
> patch. I believe it's useful to fix this part first. The diagnostic/default-
> template-args-[12].C tests show a lot of examples of the intent of this
> patch. And the remaining changes to the testsuite show how it changes
> diagnostic output.
> 
> ---------------------- 8< --------------------
> 
> The choice when to print a function template parameter was still
> suboptimal. That's because sometimes the function template parameter
> list only adds noise, while in other situations the lack of a function
> template parameter list makes diagnostic messages hard to understand.
> 
> The general idea of this change is to print template parms wherever they
> would appear in the source code as well. Thus, the diagnostics code
> needs to know whether any template parameter was given explicitly.
> 
> Signed-off-by: Matthias Kretz <m.kretz@gsi.de>
> 
> gcc/testsuite/ChangeLog:
> 
>         * g++.dg/debug/dwarf2/template-params-12n.C: Optionally, allow
>         DW_AT_default_value.
>         * g++.dg/diagnostic/default-template-args-1.C: New.
>         * g++.dg/diagnostic/default-template-args-2.C: New.
>         * g++.dg/diagnostic/param-type-mismatch-2.C: Expect template
>         parms in diagnostic.
>         * g++.dg/ext/pretty1.C: Expect function template specialization
>         to not pretty-print template parms.
>         * g++.old-deja/g++.ext/pretty3.C: Ditto.
>         * g++.old-deja/g++.pt/memtemp77.C: Ditto.
>         * g++.dg/goacc/template.C: Expect function template parms for
>         explicit arguments.
>         * g++.dg/gomp/declare-variant-7.C: Expect no function template
>         parms for deduced arguments.
>         * g++.dg/template/error40.C: Expect only non-default template
>         arguments in diagnostic.
> 
> gcc/cp/ChangeLog:
> 
>         * cp-tree.h (GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT): Return
>         absolute value of stored constant.
>         (EXPLICIT_TEMPLATE_ARGS_P): New.
>         (SET_EXPLICIT_TEMPLATE_ARGS_P): New.
>         (TFF_AS_PRIMARY): New constant.
>         * error.c (get_non_default_template_args_count): Avoid
>         GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT if
>         NON_DEFAULT_TEMPLATE_ARGS_COUNT is a NULL_TREE. Make independent
>         of flag_pretty_templates.
>         (dump_template_bindings): Add flags parameter to be passed to
>         get_non_default_template_args_count. Print only non-default
>         template arguments.
>         (dump_function_decl): Call dump_function_name and dump_type of
>         the DECL_CONTEXT with specialized template and set
>         TFF_AS_PRIMARY for their flags.
>         (dump_function_name): Add and document conditions for calling
>         dump_template_parms.
>         (dump_template_parms): Print only non-default template
>         parameters.
>         * pt.c (determine_specialization): Jason?
>         (template_parms_level_to_args): Jason?
>         (copy_template_args): Jason?
>         (fn_type_unification): Set EXPLICIT_TEMPLATE_ARGS_P on the
>         template arguments tree if any template parameter was explicitly
>         given.
>         (type_unification_real): Jason?
>         (get_partial_spec_bindings): Jason?
>         (tsubst_template_args): Determine number of defaulted arguments
>         from new argument vector, if possible.
> ---
>  gcc/cp/cp-tree.h                              | 18 +++-
>  gcc/cp/error.c                                | 83 ++++++++++++++-----
>  gcc/cp/pt.c                                   | 58 +++++++++----
>  .../g++.dg/debug/dwarf2/template-params-12n.C |  2 +-
>  .../diagnostic/default-template-args-1.C      | 73 ++++++++++++++++
>  .../diagnostic/default-template-args-2.C      | 37 +++++++++
>  .../g++.dg/diagnostic/param-type-mismatch-2.C |  2 +-
>  gcc/testsuite/g++.dg/ext/pretty1.C            |  2 +-
>  gcc/testsuite/g++.dg/goacc/template.C         |  8 +-
>  gcc/testsuite/g++.dg/gomp/declare-variant-7.C |  4 +-
>  gcc/testsuite/g++.dg/template/error40.C       |  6 +-
>  gcc/testsuite/g++.old-deja/g++.ext/pretty3.C  |  2 +-
>  gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C |  2 +-
>  13 files changed, 242 insertions(+), 55 deletions(-)
>  create mode 100644
> gcc/testsuite/g++.dg/diagnostic/default-template-args-1.C create mode
> 100644 gcc/testsuite/g++.dg/diagnostic/default-template-args-2.C
  
Jason Merrill Nov. 16, 2021, 8:25 p.m. UTC | #2
On 11/8/21 15:00, Matthias Kretz wrote:
> I forgot to mention why I tagged it [RFC]: I needed one more bit of
> information on the template args TREE_VEC to encode EXPLICIT_TEMPLATE_ARGS_P.
> Its TREE_CHAIN already points to an integer constant denoting the number of
> non-default arguments, so I couldn't trivially replace that. Therefore, I used
> the sign of that integer. I was hoping to find a cleaner solution, though.

It seems that we aren't using any TREE_LANG_FLAG_n on TREE_VEC, so that 
would be a cleaner solution.

> On Monday, 8 November 2021 17:40:44 CET Matthias Kretz wrote:
>> On Tuesday, 17 August 2021 20:31:54 CET Jason Merrill wrote:
>>>> 2. Given a DECL_TI_ARGS tree, can I query whether an argument was
>>>> deduced
>>>> or explicitly specified? I'm asking because I still consider diagnostics
>>>> of function templates unfortunate. `template <class T> void f()` is
>>>> fine,
>>>> as is `void f(T) [with T = float]`, but `void f() [with T = float]`
>>>> could
>>>> be better. I.e. if the template parameter appears somewhere in the
>>>> function parameter list, dump_template_parms would only produce noise.
>>>> If, however, the template parameter was given explicitly, it would be
>>>> nice if it could show up accordingly in diagnostics.
>>>
>>> NON_DEFAULT_TEMPLATE_ARGS_COUNT has that information, though there are
>>> some issues with it.  Attached is my WIP from May to improve it
>>> somewhat, if that's interesting.
>>
>> It is interesting. I used your patch to come up with the attached. Patch. I
>> must say, I didn't try to read through all the cp/pt.c code to understand
>> all of what you did there (which is why my ChangeLog entry says "Jason?"),
>> but it works for me (and all of `make check`).
>>
>> Anyway, I'd like to propose the following before finishing my diagnose_as
>> patch. I believe it's useful to fix this part first. The diagnostic/default-
>> template-args-[12].C tests show a lot of examples of the intent of this
>> patch. And the remaining changes to the testsuite show how it changes
>> diagnostic output.
>>
>> ---------------------- 8< --------------------
>>
>> The choice when to print a function template parameter was still
>> suboptimal. That's because sometimes the function template parameter
>> list only adds noise, while in other situations the lack of a function
>> template parameter list makes diagnostic messages hard to understand.
>>
>> The general idea of this change is to print template parms wherever they
>> would appear in the source code as well. Thus, the diagnostics code
>> needs to know whether any template parameter was given explicitly.
>>
>> Signed-off-by: Matthias Kretz <m.kretz@gsi.de>
>>
>> gcc/testsuite/ChangeLog:
>>
>>          * g++.dg/debug/dwarf2/template-params-12n.C: Optionally, allow
>>          DW_AT_default_value.
>>          * g++.dg/diagnostic/default-template-args-1.C: New.
>>          * g++.dg/diagnostic/default-template-args-2.C: New.
>>          * g++.dg/diagnostic/param-type-mismatch-2.C: Expect template
>>          parms in diagnostic.
>>          * g++.dg/ext/pretty1.C: Expect function template specialization
>>          to not pretty-print template parms.
>>          * g++.old-deja/g++.ext/pretty3.C: Ditto.
>>          * g++.old-deja/g++.pt/memtemp77.C: Ditto.
>>          * g++.dg/goacc/template.C: Expect function template parms for
>>          explicit arguments.
>>          * g++.dg/gomp/declare-variant-7.C: Expect no function template
>>          parms for deduced arguments.
>>          * g++.dg/template/error40.C: Expect only non-default template
>>          arguments in diagnostic.
>>
>> gcc/cp/ChangeLog:
>>
>>          * cp-tree.h (GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT): Return
>>          absolute value of stored constant.
>>          (EXPLICIT_TEMPLATE_ARGS_P): New.
>>          (SET_EXPLICIT_TEMPLATE_ARGS_P): New.
>>          (TFF_AS_PRIMARY): New constant.
>>          * error.c (get_non_default_template_args_count): Avoid
>>          GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT if
>>          NON_DEFAULT_TEMPLATE_ARGS_COUNT is a NULL_TREE. Make independent
>>          of flag_pretty_templates.
>>          (dump_template_bindings): Add flags parameter to be passed to
>>          get_non_default_template_args_count. Print only non-default
>>          template arguments.
>>          (dump_function_decl): Call dump_function_name and dump_type of
>>          the DECL_CONTEXT with specialized template and set
>>          TFF_AS_PRIMARY for their flags.
>>          (dump_function_name): Add and document conditions for calling
>>          dump_template_parms.
>>          (dump_template_parms): Print only non-default template
>>          parameters.
>>          * pt.c (determine_specialization): Jason?
>>          (template_parms_level_to_args): Jason?
>>          (copy_template_args): Jason?
>>          (fn_type_unification): Set EXPLICIT_TEMPLATE_ARGS_P on the
>>          template arguments tree if any template parameter was explicitly
>>          given.
>>          (type_unification_real): Jason?
>>          (get_partial_spec_bindings): Jason?
>>          (tsubst_template_args): Determine number of defaulted arguments
>>          from new argument vector, if possible.
>> ---
>>   gcc/cp/cp-tree.h                              | 18 +++-
>>   gcc/cp/error.c                                | 83 ++++++++++++++-----
>>   gcc/cp/pt.c                                   | 58 +++++++++----
>>   .../g++.dg/debug/dwarf2/template-params-12n.C |  2 +-
>>   .../diagnostic/default-template-args-1.C      | 73 ++++++++++++++++
>>   .../diagnostic/default-template-args-2.C      | 37 +++++++++
>>   .../g++.dg/diagnostic/param-type-mismatch-2.C |  2 +-
>>   gcc/testsuite/g++.dg/ext/pretty1.C            |  2 +-
>>   gcc/testsuite/g++.dg/goacc/template.C         |  8 +-
>>   gcc/testsuite/g++.dg/gomp/declare-variant-7.C |  4 +-
>>   gcc/testsuite/g++.dg/template/error40.C       |  6 +-
>>   gcc/testsuite/g++.old-deja/g++.ext/pretty3.C  |  2 +-
>>   gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C |  2 +-
>>   13 files changed, 242 insertions(+), 55 deletions(-)
>>   create mode 100644
>> gcc/testsuite/g++.dg/diagnostic/default-template-args-1.C create mode
>> 100644 gcc/testsuite/g++.dg/diagnostic/default-template-args-2.C
> 
>
  
Matthias Kretz Nov. 16, 2021, 8:42 p.m. UTC | #3
On Tuesday, 16 November 2021 21:25:33 CET Jason Merrill wrote:
> On 11/8/21 15:00, Matthias Kretz wrote:
> > I forgot to mention why I tagged it [RFC]: I needed one more bit of
> > information on the template args TREE_VEC to encode
> > EXPLICIT_TEMPLATE_ARGS_P. Its TREE_CHAIN already points to an integer
> > constant denoting the number of non-default arguments, so I couldn't
> > trivially replace that. Therefore, I used the sign of that integer. I was
> > hoping to find a cleaner solution, though.
> It seems that we aren't using any TREE_LANG_FLAG_n on TREE_VEC, so that
> would be a cleaner solution.

I tried that first but realized that TREE_VEC doesn't allow any 
TREE_LANG_FLAGs (it uses those bits for the length IIRC). And setting the 
TREE_LANG_FLAGs on the TREE_CHAIN of the TREE_VEC can't work either (since the 
int constants are shared between many trees).

Should I maybe turn the TREE_CHAIN into a TREE_LIST using TREE_PURPOSE and 
TREE_VALUE for EXPLICIT_TEMPLATE_ARGS_P and non-default arguments, 
respectively? (And where would I document this?)
  
Jason Merrill Nov. 16, 2021, 8:49 p.m. UTC | #4
On 11/16/21 15:42, Matthias Kretz wrote:
> On Tuesday, 16 November 2021 21:25:33 CET Jason Merrill wrote:
>> On 11/8/21 15:00, Matthias Kretz wrote:
>>> I forgot to mention why I tagged it [RFC]: I needed one more bit of
>>> information on the template args TREE_VEC to encode
>>> EXPLICIT_TEMPLATE_ARGS_P. Its TREE_CHAIN already points to an integer
>>> constant denoting the number of non-default arguments, so I couldn't
>>> trivially replace that. Therefore, I used the sign of that integer. I was
>>> hoping to find a cleaner solution, though.
>> It seems that we aren't using any TREE_LANG_FLAG_n on TREE_VEC, so that
>> would be a cleaner solution.
> 
> I tried that first but realized that TREE_VEC doesn't allow any
> TREE_LANG_FLAGs (it uses those bits for the length IIRC). And setting the
> TREE_LANG_FLAGs on the TREE_CHAIN of the TREE_VEC can't work either (since the
> int constants are shared between many trees).
> 
> Should I maybe turn the TREE_CHAIN into a TREE_LIST using TREE_PURPOSE and
> TREE_VALUE for EXPLICIT_TEMPLATE_ARGS_P and non-default arguments,
> respectively? (And where would I document this?)

Maybe a TREE_LIST if there are explicit template arguments to a function 
template, where TREE_PURPOSE is the number of explicit arguments and 
TREE_VALUE is the number of non-default arguments.

I'd document it at the definition of NON_DEFAULT_TEMPLATE_ARGS_COUNT. 
The SET/GET macros should become functions.

Jason
  
Matthias Kretz Nov. 16, 2021, 8:51 p.m. UTC | #5
On Tuesday, 16 November 2021 21:49:31 CET Jason Merrill wrote:
> On 11/16/21 15:42, Matthias Kretz wrote:
> > On Tuesday, 16 November 2021 21:25:33 CET Jason Merrill wrote:
> >> On 11/8/21 15:00, Matthias Kretz wrote:
> >>> I forgot to mention why I tagged it [RFC]: I needed one more bit of
> >>> information on the template args TREE_VEC to encode
> >>> EXPLICIT_TEMPLATE_ARGS_P. Its TREE_CHAIN already points to an integer
> >>> constant denoting the number of non-default arguments, so I couldn't
> >>> trivially replace that. Therefore, I used the sign of that integer. I
> >>> was
> >>> hoping to find a cleaner solution, though.
> >> 
> >> It seems that we aren't using any TREE_LANG_FLAG_n on TREE_VEC, so that
> >> would be a cleaner solution.
> > 
> > I tried that first but realized that TREE_VEC doesn't allow any
> > TREE_LANG_FLAGs (it uses those bits for the length IIRC). And setting the
> > TREE_LANG_FLAGs on the TREE_CHAIN of the TREE_VEC can't work either (since
> > the int constants are shared between many trees).
> > 
> > Should I maybe turn the TREE_CHAIN into a TREE_LIST using TREE_PURPOSE and
> > TREE_VALUE for EXPLICIT_TEMPLATE_ARGS_P and non-default arguments,
> > respectively? (And where would I document this?)
> 
> Maybe a TREE_LIST if there are explicit template arguments to a function
> template, where TREE_PURPOSE is the number of explicit arguments and
> TREE_VALUE is the number of non-default arguments.
> 
> I'd document it at the definition of NON_DEFAULT_TEMPLATE_ARGS_COUNT.
> The SET/GET macros should become functions.

Sounds good. I'll come up with a new patch ASAP.
  
Jason Merrill Nov. 17, 2021, 6:09 a.m. UTC | #6
On 11/8/21 11:40, Matthias Kretz wrote:
> On Tuesday, 17 August 2021 20:31:54 CET Jason Merrill wrote:
>>> 2. Given a DECL_TI_ARGS tree, can I query whether an argument was deduced
>>> or explicitly specified? I'm asking because I still consider diagnostics
>>> of function templates unfortunate. `template <class T> void f()` is fine,
>>> as is `void f(T) [with T = float]`, but `void f() [with T = float]` could
>>> be better. I.e. if the template parameter appears somewhere in the
>>> function parameter list, dump_template_parms would only produce noise.
>>> If, however, the template parameter was given explicitly, it would be
>>> nice if it could show up accordingly in diagnostics.
>>
>> NON_DEFAULT_TEMPLATE_ARGS_COUNT has that information, though there are
>> some issues with it.  Attached is my WIP from May to improve it
>> somewhat, if that's interesting.
> 
> It is interesting. I used your patch to come up with the attached. Patch. I
> must say, I didn't try to read through all the cp/pt.c code to understand all
> of what you did there (which is why my ChangeLog entry says "Jason?"), but it
> works for me (and all of `make check`).
> 
> Anyway, I'd like to propose the following before finishing my diagnose_as
> patch. I believe it's useful to fix this part first. The diagnostic/default-
> template-args-[12].C tests show a lot of examples of the intent of this patch.
> And the remaining changes to the testsuite show how it changes diagnostic
> output.
> 
> ---------------------- 8< --------------------
> 
> The choice when to print a function template parameter was still
> suboptimal. That's because sometimes the function template parameter
> list only adds noise, while in other situations the lack of a function
> template parameter list makes diagnostic messages hard to understand.
> 
> The general idea of this change is to print template parms wherever they
> would appear in the source code as well. Thus, the diagnostics code
> needs to know whether any template parameter was given explicitly.
> 
> Signed-off-by: Matthias Kretz <m.kretz@gsi.de>
> 
> gcc/testsuite/ChangeLog:
> 
>          * g++.dg/debug/dwarf2/template-params-12n.C: Optionally, allow
>          DW_AT_default_value.
>          * g++.dg/diagnostic/default-template-args-1.C: New.
>          * g++.dg/diagnostic/default-template-args-2.C: New.
>          * g++.dg/diagnostic/param-type-mismatch-2.C: Expect template
>          parms in diagnostic.
>          * g++.dg/ext/pretty1.C: Expect function template specialization
>          to not pretty-print template parms.
>          * g++.old-deja/g++.ext/pretty3.C: Ditto.
>          * g++.old-deja/g++.pt/memtemp77.C: Ditto.
>          * g++.dg/goacc/template.C: Expect function template parms for
>          explicit arguments.
>          * g++.dg/gomp/declare-variant-7.C: Expect no function template
>          parms for deduced arguments.
>          * g++.dg/template/error40.C: Expect only non-default template
>          arguments in diagnostic.
> 
> gcc/cp/ChangeLog:
> 
>          * cp-tree.h (GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT): Return
>          absolute value of stored constant.
>          (EXPLICIT_TEMPLATE_ARGS_P): New.
>          (SET_EXPLICIT_TEMPLATE_ARGS_P): New.
>          (TFF_AS_PRIMARY): New constant.
>          * error.c (get_non_default_template_args_count): Avoid
>          GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT if
>          NON_DEFAULT_TEMPLATE_ARGS_COUNT is a NULL_TREE. Make independent
>          of flag_pretty_templates.
>          (dump_template_bindings): Add flags parameter to be passed to
>          get_non_default_template_args_count. Print only non-default
>          template arguments.
>          (dump_function_decl): Call dump_function_name and dump_type of
>          the DECL_CONTEXT with specialized template and set
>          TFF_AS_PRIMARY for their flags.
>          (dump_function_name): Add and document conditions for calling
>          dump_template_parms.
>          (dump_template_parms): Print only non-default template
>          parameters.
>          * pt.c (determine_specialization): Jason?

Also copy the inner TREE_VECs.

>          (template_parms_level_to_args): Jason?

Always count non-default args.  Also, I think

> -  if (CHECKING_P)
> -    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
> +  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);

should have been

if (CHECKING_P || nondefault != TREE_VEC_LENGTH (a))
   SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);

>          (copy_template_args): Jason?

Only copy the non-default template args count on TREE_VECs that should 
have it.

>          (fn_type_unification): Set EXPLICIT_TEMPLATE_ARGS_P on the
>          template arguments tree if any template parameter was explicitly
>          given.
>          (type_unification_real): Jason?

Count non-default args sooner.

>          (get_partial_spec_bindings): Jason?

Set non-default args count.

>          (tsubst_template_args): Determine number of defaulted arguments
>          from new argument vector, if possible.
> ---
>   gcc/cp/cp-tree.h                              | 18 +++-
>   gcc/cp/error.c                                | 83 ++++++++++++++-----
>   gcc/cp/pt.c                                   | 58 +++++++++----
>   .../g++.dg/debug/dwarf2/template-params-12n.C |  2 +-
>   .../diagnostic/default-template-args-1.C      | 73 ++++++++++++++++
>   .../diagnostic/default-template-args-2.C      | 37 +++++++++
>   .../g++.dg/diagnostic/param-type-mismatch-2.C |  2 +-
>   gcc/testsuite/g++.dg/ext/pretty1.C            |  2 +-
>   gcc/testsuite/g++.dg/goacc/template.C         |  8 +-
>   gcc/testsuite/g++.dg/gomp/declare-variant-7.C |  4 +-
>   gcc/testsuite/g++.dg/template/error40.C       |  6 +-
>   gcc/testsuite/g++.old-deja/g++.ext/pretty3.C  |  2 +-
>   gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C |  2 +-
>   13 files changed, 242 insertions(+), 55 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/diagnostic/default-template-args-1.C
>   create mode 100644 gcc/testsuite/g++.dg/diagnostic/default-template-args-2.C
> 

> +  /* Pretty print only template instantiations. Don't pretty print explicit
> +     specializations like 'template <> void fun<int> (int)'.  */

This seems like a significant change of behavior unrelated to printing 
default template arguments.  What's the rationale for handling 
specializations differently from instantiations?

I also don't understand the purpose of TFF_AS_PRIMARY.

> +/* Print function template parameters if:
> +   1. t is template, and
> +   2. flags did not request "show only template-name", and
> +   3. t is a specialization of a template (Why is this needed? This was present
> +      since 1999 via !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION: "Don't crash if
> +      given a friend pseudo-instantiation". The DECL_USE_TEMPLATE should likely
> +      inform the PRIMARY parameter of dump_template_parms.), and

Good question.  It seems that the existing 
!DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION has mostly been excluding the 
most general template; removing that line changes things like

int bar(T) [with T = int]

to

int bar<T>(T) [with T = int]

> +   4. either
> +      - flags requests to show no function arguments, in which case deduced
> +        types could be hidden, or
> +      - at least one function template argument was given explicitly, or
> +      - we're printing a DWARF name,

...but perhaps this can replace the above.  Though, why do we want 
different behavior here when printing a DWARF name?

> +   5. either
> +      - t is a member friend template of a template class (see DECL_TI_TEMPLATE
> +        documentation), or
> +      - 

Missing the last item.  :)

> + */

*/ doesn't get its own line.

>    if (DECL_TEMPLATE_INFO (t)
>        && !(flags & TFF_TEMPLATE_NAME)
> -      && !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t)
> +      && DECL_USE_TEMPLATE (t)
> +      && ((flags & TFF_NO_FUNCTION_ARGUMENTS)
> +	    || (DECL_TI_ARGS (t)
> +		  && EXPLICIT_TEMPLATE_ARGS_P (INNERMOST_TEMPLATE_ARGS
> +						 (DECL_TI_ARGS (t))))
> +	    || (pp->flags & pp_c_flag_gnu_v3) != 0)
>        && (TREE_CODE (DECL_TI_TEMPLATE (t)) != TEMPLATE_DECL
>  	  || PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (t))))
> -    dump_template_parms (pp, DECL_TEMPLATE_INFO (t), !DECL_USE_TEMPLATE (t),
> -                         flags);
> +    dump_template_parms (pp, DECL_TEMPLATE_INFO (t), false, flags);


Jason
  
Matthias Kretz Nov. 17, 2021, 9:04 a.m. UTC | #7
On Wednesday, 17 November 2021 07:09:18 CET Jason Merrill wrote:
> > -  if (CHECKING_P)
> > -    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
> > +  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
> 
> should have been
> 
> if (CHECKING_P || nondefault != TREE_VEC_LENGTH (a))
>    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);

TBH, I don't understand the purpose of CHECKING_P here, or rather it makes me 
nervous because AFAIU I'm only testing with CHECKING_P enabled. Why make 
behavior dependent on CHECKING_P? I expected CHECKING_P to basically only add 
more assertions.

> > (copy_template_args): Jason?
> 
> Only copy the non-default template args count on TREE_VECs that should
> have it.

Why not simply set the count on all args? Is it a performance concern? The 
INTEGER_CST the TREE_CHAIN has to point to exists anyway, so it's not wasting 
any memory, right?

> 
> > +  /* Pretty print only template instantiations. Don't pretty print
> > explicit
> > +     specializations like 'template <> void fun<int> (int)'. 
> 
> This seems like a significant change of behavior unrelated to printing
> default template arguments.  What's the rationale for handling
> specializations differently from instantiations?

Right, this is about "The general idea of this change is to print template 
parms wherever they would appear in the source code as well".

Initially, the change to print function template arguments/parameters only if 
the args were explicitly specified lead to printing 'void fun (T) [with T = 
...]' or 'template <> void fun (int)'. Both are not telling the full story, 
even if the former is how the function would be called. But if the reader 
should quickly recognize what code is getting called, it is helpful to see 
right away that a function template specialization is called. (It might also 
reveal an implementation detail of a library, so it's not 100% obvious how to 
choose here.) Also, saying 'T = int' is kind of wrong. Yes, 'int' was deduced. 
But there's no T in fun<int>:

template <class T> void fun (T);
template <> void fun<int> (int);

__FUNCTION__ was 'fun<int>' all the time, but __PRETTY_FUNCTION__ was 'void 
fun(T) [with T = int]'. It's more consistent that __PRETTY_FUNCTION__ contains 
__FUNCTION__, IMHO, so it would have to be at least 'void fun<int>(T) [with T 
= int]'. But that's strange: How it uses T and int for the same type. So I 
settled on 'void fun<int>(int)'.

> I also don't understand the purpose of TFF_AS_PRIMARY.

dump_function_decl generalizes the TEMPLATE_DECL (if flag_pretty_templates is 
true) and, before this change, passes the generalized TEMPLATE_DECL to 
dump_type (... DECL_CONTEXT (t) ...) and dump_function_name (... t ...). 
That's why the whole template is printed as primary template (i.e. with 
template parms instead of template args, as is needed for 
flag_pretty_templates). But this drops the count of non-default template args. 
To retain the count, dump_type and dump_function_name need to be called with 
the original TEMPLATE_DECL. But if I do this, pretty-templates is broken.
'template <class T> struct A { template <class U> void f(T, U); };' would 
print as 'A<int>::f<float>(T, U) [with U = float, T = int]'. To get back to 
'A<T>::f<U>(T, U) [with U = float, T = int]' I needed to tell 
dump_template_parms that even though the template args are there, it should 
print only the template parms. The most obvious way to do that was to carry it 
through via flags.

Note that this creates another problem. Given

template <class T0, class T1 = int> struct Outer {
  template <class T, class U> struct A;
  template <class X> struct A<X, int> {
    void f();
  };
};

we want to print e.g. 'void Outer<T0>::A<X, int>::f() [with X = int, T0 = 
int]', but certainly not 'void Outer<T0>::A<T, U>::f() [with X = int, T0 = 
int]'. However, specialized_t holds A<int, int> which is printed as A<T, U> 
with TFF_AS_PRIMARY. Only most_general_template of the function's 
TEMPLATE_DECL can give us A<X, int> as DECL_CONTEXT.

I have a solution in the diagnose_as patch, where I had to solve a similar 
problem because for the diagnose_as attribute (dump_template_scope).

> > +/* Print function template parameters if:
> > +   1. t is template, and
> > +   2. flags did not request "show only template-name", and
> > +   3. t is a specialization of a template (Why is this needed? This was
> > present +      since 1999 via !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION:
> > "Don't crash if +      given a friend pseudo-instantiation". The
> > DECL_USE_TEMPLATE should likely +      inform the PRIMARY parameter of
> > dump_template_parms.), and
> 
> Good question.  It seems that the existing
> !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION has mostly been excluding the
> most general template; removing that line changes things like
> 
> int bar(T) [with T = int]
> 
> to
> 
> int bar<T>(T) [with T = int]
> 
> 
> 
> > +   4. either
> > +      - flags requests to show no function arguments, in which case
> > deduced +        types could be hidden, or
> > +      - at least one function template argument was given explicitly, or
> > +      - we're printing a DWARF name,
> 
> ...but perhaps this can replace the above.

I'll rerun the tests with DECL_USE_TEMPLATE moved to the PRIMARY parameter of 
dump_template_parms.

> Though, why do we want
> different behavior here when printing a DWARF name?

Sorry, I should have asked ... I also added the same issue as an open question 
on the diagnose_as patch. When I ran the whole testsuite I had failures in the 
DWARF tests. This change resolved them. I don't know enough about how those 
strings are used and whether they may change between GCC versions. Anyway, the 
DWARF strings in that test had only the function name and template argument 
list (i.e. no function arguments). If the template argument list was removed 
(as was the case without the condition here), the DWARF strings lost important 
information, giving several different functions the same name. That seemed 
like a regression. So either the DWARF strings need to include the function 
arguments, or we need this condition to keep showing the template arguments 
independent of whether they were explicitly given or not.

> > +   5. either
> > +      - t is a member friend template of a template class (see
> > DECL_TI_TEMPLATE +        documentation), or
> > +      -
> 
> Missing the last item.  :)

Oh, I got distracted while trying to figure this out. :) I'll try to 
understand why PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (t)) is needed and 
document it.
  
Jason Merrill Nov. 17, 2021, 6:25 p.m. UTC | #8
On 11/17/21 04:04, Matthias Kretz wrote:
> On Wednesday, 17 November 2021 07:09:18 CET Jason Merrill wrote:
>>> -  if (CHECKING_P)
>>> -    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
>>> +  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
>>
>> should have been
>>
>> if (CHECKING_P || nondefault != TREE_VEC_LENGTH (a))
>>     SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
> 
> TBH, I don't understand the purpose of CHECKING_P here, or rather it makes me
> nervous because AFAIU I'm only testing with CHECKING_P enabled. Why make
> behavior dependent on CHECKING_P? I expected CHECKING_P to basically only add
> more assertions.

The idea when NON_DEFAULT_TEMPLATE_ARGS_COUNT was added years back was 
to leave the TREE_CHAIN null when !CHECKING_P and treat that as 
equivalent to TREE_VEC_LENGTH (args).  But perhaps you're right that 
it's not a savings worth the complexity.

>>> (copy_template_args): Jason?
>>
>> Only copy the non-default template args count on TREE_VECs that should
>> have it.
> 
> Why not simply set the count on all args? Is it a performance concern? The
> INTEGER_CST the TREE_CHAIN has to point to exists anyway, so it's not wasting
> any memory, right?

In this case the TREE_VEC we're excluding is the one wrapping multiple 
levels of template args; it doesn't contain args directly, so setting 
NON_DEFAULT_ARGS_COUNT on it doesn't make sense.

>>> +  /* Pretty print only template instantiations. Don't pretty print
>>> explicit
>>> +     specializations like 'template <> void fun<int> (int)'.
>>
>> This seems like a significant change of behavior unrelated to printing
>> default template arguments.  What's the rationale for handling
>> specializations differently from instantiations?
> 
> Right, this is about "The general idea of this change is to print template
> parms wherever they would appear in the source code as well".
> 
> Initially, the change to print function template arguments/parameters only if
> the args were explicitly specified lead to printing 'void fun (T) [with T =
> ...]' or 'template <> void fun (int)'. Both are not telling the full story,
> even if the former is how the function would be called.

and the latter is how I expect the specialization to be declared, not 
with the deducible template argument made explicit.

> But if the reader
> should quickly recognize what code is getting called, it is helpful to see
> right away that a function template specialization is called. (It might also
> reveal an implementation detail of a library, so it's not 100% obvious how to
> choose here.) Also, saying 'T = int' is kind of wrong. Yes, 'int' was deduced.
> But there's no T in fun<int>:
> 
> template <class T> void fun (T);
> template <> void fun<int> (int);

There's a T in the template, and as you said above, that's how it's 
called (and mangled).

> __FUNCTION__ was 'fun<int>' all the time, but __PRETTY_FUNCTION__ was 'void
> fun(T) [with T = int]'.

Isn't that true for instantiations, as well?

> It's more consistent that __PRETTY_FUNCTION__ contains __FUNCTION__, IMHO

I suppose, but I don't see that as a strong enough motivation to mix 
this up.

> so it would have to be at least 'void fun<int>(T) [with T
> = int]'. But that's strange: How it uses T and int for the same type. So I
> settled on 'void fun<int>(int)'.
> 
>> I also don't understand the purpose of TFF_AS_PRIMARY.
> 
> dump_function_decl generalizes the TEMPLATE_DECL (if flag_pretty_templates is
> true) and, before this change, passes the generalized TEMPLATE_DECL to
> dump_type (... DECL_CONTEXT (t) ...) and dump_function_name (... t ...).
> That's why the whole template is printed as primary template (i.e. with
> template parms instead of template args, as is needed for
> flag_pretty_templates). But this drops the count of non-default template args.

Ah, you're trying to omit defaulted parms from the <list>?  I'm not sure 
that's necessary, leaving them out of the [with ...] list should be 
sufficient.

> To retain the count, dump_type and dump_function_name need to be called with
> the original TEMPLATE_DECL. But if I do this, pretty-templates is broken.
> 'template <class T> struct A { template <class U> void f(T, U); };' would
> print as 'A<int>::f<float>(T, U) [with U = float, T = int]'. To get back to
> 'A<T>::f<U>(T, U) [with U = float, T = int]' I needed to tell
> dump_template_parms that even though the template args are there, it should
> print only the template parms. The most obvious way to do that was to carry it
> through via flags.
> 
> Note that this creates another problem. Given
> 
> template <class T0, class T1 = int> struct Outer {
>    template <class T, class U> struct A;
>    template <class X> struct A<X, int> {
>      void f();
>    };
> };
> 
> we want to print e.g. 'void Outer<T0>::A<X, int>::f() [with X = int, T0 =
> int]', but certainly not 'void Outer<T0>::A<T, U>::f() [with X = int, T0 =
> int]'. However, specialized_t holds A<int, int> which is printed as A<T, U>
> with TFF_AS_PRIMARY. Only most_general_template of the function's
> TEMPLATE_DECL can give us A<X, int> as DECL_CONTEXT.
> 
> I have a solution in the diagnose_as patch, where I had to solve a similar
> problem because for the diagnose_as attribute (dump_template_scope).
> 
>>> +/* Print function template parameters if:
>>> +   1. t is template, and
>>> +   2. flags did not request "show only template-name", and
>>> +   3. t is a specialization of a template (Why is this needed? This was
>>> present +      since 1999 via !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION:
>>> "Don't crash if +      given a friend pseudo-instantiation". The
>>> DECL_USE_TEMPLATE should likely +      inform the PRIMARY parameter of
>>> dump_template_parms.), and
>>
>> Good question.  It seems that the existing
>> !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION has mostly been excluding the
>> most general template; removing that line changes things like
>>
>> int bar(T) [with T = int]
>>
>> to
>>
>> int bar<T>(T) [with T = int]
>>
>>
>>
>>> +   4. either
>>> +      - flags requests to show no function arguments, in which case
>>> deduced +        types could be hidden, or
>>> +      - at least one function template argument was given explicitly, or
>>> +      - we're printing a DWARF name,
>>
>> ...but perhaps this can replace the above.
> 
> I'll rerun the tests with DECL_USE_TEMPLATE moved to the PRIMARY parameter of
> dump_template_parms.
> 
>> Though, why do we want
>> different behavior here when printing a DWARF name?
> 
> Sorry, I should have asked ... I also added the same issue as an open question
> on the diagnose_as patch. When I ran the whole testsuite I had failures in the
> DWARF tests. This change resolved them. I don't know enough about how those
> strings are used and whether they may change between GCC versions. Anyway, the
> DWARF strings in that test had only the function name and template argument
> list (i.e. no function arguments).

If they only have arguments, isn't TFF_NO_FUNCTION_ARGUMENTS set?

> If the template argument list was removed
> (as was the case without the condition here), the DWARF strings lost important
> information, giving several different functions the same name. That seemed
> like a regression. So either the DWARF strings need to include the function
> arguments, or we need this condition to keep showing the template arguments
> independent of whether they were explicitly given or not.
> 
>>> +   5. either
>>> +      - t is a member friend template of a template class (see
>>> DECL_TI_TEMPLATE +        documentation), or
>>> +      -
>>
>> Missing the last item.  :)
> 
> Oh, I got distracted while trying to figure this out. :) I'll try to
> understand why PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (t)) is needed and
> document it.
>
  
Matthias Kretz Nov. 17, 2021, 10:51 p.m. UTC | #9
On Wednesday, 17 November 2021 19:25:46 CET Jason Merrill wrote:
> On 11/17/21 04:04, Matthias Kretz wrote:
> > On Wednesday, 17 November 2021 07:09:18 CET Jason Merrill wrote:
> >>> -  if (CHECKING_P)
> >>> -    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
> >>> +  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
> >> 
> >> should have been
> >> 
> >> if (CHECKING_P || nondefault != TREE_VEC_LENGTH (a))
> >> 
> >>     SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
> > 
> > TBH, I don't understand the purpose of CHECKING_P here, or rather it makes
> > me nervous because AFAIU I'm only testing with CHECKING_P enabled. Why
> > make behavior dependent on CHECKING_P? I expected CHECKING_P to basically
> > only add more assertions.
> 
> The idea when NON_DEFAULT_TEMPLATE_ARGS_COUNT was added years back was
> to leave the TREE_CHAIN null when !CHECKING_P and treat that as
> equivalent to TREE_VEC_LENGTH (args).  But perhaps you're right that
> it's not a savings worth the complexity.

Thanks, now I understand.

> >>> (copy_template_args): Jason?
> >> 
> >> Only copy the non-default template args count on TREE_VECs that should
> >> have it.
> > 
> > Why not simply set the count on all args? Is it a performance concern? The
> > INTEGER_CST the TREE_CHAIN has to point to exists anyway, so it's not
> > wasting any memory, right?
> 
> In this case the TREE_VEC we're excluding is the one wrapping multiple
> levels of template args; it doesn't contain args directly, so setting
> NON_DEFAULT_ARGS_COUNT on it doesn't make sense.

Right, I had already added a `gcc_assert (!TMPL_ARGS_HAVE_MULTIPLE_LEVELS 
(args))` to my new set_non_default_template_args_count function and found cp/
constraint.cc:2896 (get_mapped_args), which calls 
SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT on the outer TREE_VEC. Was this supposed 
to apply to all inner TREE_VECs? Or is deleting the line the correct fix?

> >>> +  /* Pretty print only template instantiations. Don't pretty print
> >>> explicit
> >>> +     specializations like 'template <> void fun<int> (int)'.
> >> 
> >> This seems like a significant change of behavior unrelated to printing
> >> default template arguments.  What's the rationale for handling
> >> specializations differently from instantiations?
> > 
> > Right, this is about "The general idea of this change is to print template
> > parms wherever they would appear in the source code as well".
> > 
> > Initially, the change to print function template arguments/parameters only
> > if the args were explicitly specified lead to printing 'void fun (T)
> > [with T = ...]' or 'template <> void fun (int)'. Both are not telling the
> > full story, even if the former is how the function would be called.
> 
> and the latter is how I expect the specialization to be declared, not
> with the deducible template argument made explicit.

You're right. From my tests:

template <class a>
  [[deprecated]] void f4(a);

template <>
  [[deprecated]] void f4<int>(int);

template <>
  [[deprecated]] void f4(float);

  f4(1.);          // { dg-warning "'void f4\\(a\\) .with a = double.'" }
  f4(1);           // { dg-warning "'void f4<int>\\(int\\)'" }
  f4(1.f);         // { dg-warning "'void f4\\(float\\)'" }

So how it's printed depends on how the specialization is declared. It just 
falls out that way and it didn't seem awfully wrong... ;)

> > But if the reader
> > should quickly recognize what code is getting called, it is helpful to see
> > right away that a function template specialization is called. (It might
> > also reveal an implementation detail of a library, so it's not 100%
> > obvious how to choose here.) Also, saying 'T = int' is kind of wrong.
> > Yes, 'int' was deduced. But there's no T in fun<int>:
> > 
> > template <class T> void fun (T);
> > template <> void fun<int> (int);
> 
> There's a T in the template, and as you said above, that's how it's
> called (and mangled).
> 
> > __FUNCTION__ was 'fun<int>' all the time, but __PRETTY_FUNCTION__ was
> > 'void
> > fun(T) [with T = int]'.
> 
> Isn't that true for instantiations, as well?

No, instantiations don't have template args/parms in __FUNCTION__.

> > It's more consistent that __PRETTY_FUNCTION__ contains __FUNCTION__, IMHO
> 
> I suppose, but I don't see that as a strong enough motivation to mix
> this up.

What about

template <class T> void f();
template <> void f<int>();

With -fpretty-templates shouldn't it print as 'void f<T>() [with T = float]' 
and 'void f<int>()'? Yes, it's probably too subtle for most users to notice 
the difference. But I find it's more consistent this way.

> > so it would have to be at least 'void fun<int>(T) [with T
> > = int]'. But that's strange: How it uses T and int for the same type. So I
> > settled on 'void fun<int>(int)'.
> > 
> >> I also don't understand the purpose of TFF_AS_PRIMARY.
> > 
> > dump_function_decl generalizes the TEMPLATE_DECL (if flag_pretty_templates
> > is true) and, before this change, passes the generalized TEMPLATE_DECL to
> > dump_type (... DECL_CONTEXT (t) ...) and dump_function_name (... t ...).
> > That's why the whole template is printed as primary template (i.e. with
> > template parms instead of template args, as is needed for
> > flag_pretty_templates). But this drops the count of non-default template
> > args.
> Ah, you're trying to omit defaulted parms from the <list>?  I'm not sure
> that's necessary, leaving them out of the [with ...] list should be
> sufficient.

I was thinking about all the std::allocator defaults in the standard library. 
I don't want to see them. E.g. vector<int>::clear() on const object:

error: passing 'const std::vector<int>' as 'this' argument discards qualifiers
[...]/stl_vector.h:1498:7: note:   in call to 'void std::vector<_Tp, 
_Alloc>::clear() [with _Tp = int; _Alloc = std::allocator<int>]'

With my patch the last line becomes
[...]/stl_vector.h:1498:7: note:   in call to 'void std::vector<_Tp>::clear() 
[with _Tp = int]'


Another case I didn't consider before:

template <class T, class U = int> struct A {
  [[deprecated]] void f(U);
};

A<float> a; a.f(1);

With my patch it prints 'void A<T>::f(U) [with T = float]', with your 
suggestion 'void A<T, U>::f(U) [with T = float]'. Both are missing important 
information in the substitution list, IMHO. Would 'void A<T, U = int>::f(U) 
[with T = float]' be an improvement? Or should find_typenames (in cp/error.c) 
find defaulted template parms and add them to its list? IIUC find_typenames 
would find all template parms and couldn't know whether they're defaulted.

> >>> +   4. either
> >>> +      - flags requests to show no function arguments, in which case
> >>> deduced +        types could be hidden, or
> >>> +      - at least one function template argument was given explicitly,
> >>> or
> >>> +      - we're printing a DWARF name,
> >> 
> >> Though, why do we want
> >> different behavior here when printing a DWARF name?
> > 
> > Sorry, I should have asked ... I also added the same issue as an open
> > question on the diagnose_as patch. When I ran the whole testsuite I had
> > failures in the DWARF tests. This change resolved them. I don't know
> > enough about how those strings are used and whether they may change
> > between GCC versions. Anyway, the DWARF strings in that test had only the
> > function name and template argument list (i.e. no function arguments).
> 
> If they only have arguments, isn't TFF_NO_FUNCTION_ARGUMENTS set?

No, it isn't set. I could try to set it for DWARF names. It sounds like the 
right solution here.
  
Jason Merrill Nov. 18, 2021, 7:24 p.m. UTC | #10
On 11/17/21 17:51, Matthias Kretz wrote:
> On Wednesday, 17 November 2021 19:25:46 CET Jason Merrill wrote:
>> On 11/17/21 04:04, Matthias Kretz wrote:
>>> On Wednesday, 17 November 2021 07:09:18 CET Jason Merrill wrote:
>>>>> -  if (CHECKING_P)
>>>>> -    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
>>>>> +  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
>>>>
>>>> should have been
>>>>
>>>> if (CHECKING_P || nondefault != TREE_VEC_LENGTH (a))
>>>>
>>>>      SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
>>>
>>> TBH, I don't understand the purpose of CHECKING_P here, or rather it makes
>>> me nervous because AFAIU I'm only testing with CHECKING_P enabled. Why
>>> make behavior dependent on CHECKING_P? I expected CHECKING_P to basically
>>> only add more assertions.
>>
>> The idea when NON_DEFAULT_TEMPLATE_ARGS_COUNT was added years back was
>> to leave the TREE_CHAIN null when !CHECKING_P and treat that as
>> equivalent to TREE_VEC_LENGTH (args).  But perhaps you're right that
>> it's not a savings worth the complexity.
> 
> Thanks, now I understand.
> 
>>>>> (copy_template_args): Jason?
>>>>
>>>> Only copy the non-default template args count on TREE_VECs that should
>>>> have it.
>>>
>>> Why not simply set the count on all args? Is it a performance concern? The
>>> INTEGER_CST the TREE_CHAIN has to point to exists anyway, so it's not
>>> wasting any memory, right?
>>
>> In this case the TREE_VEC we're excluding is the one wrapping multiple
>> levels of template args; it doesn't contain args directly, so setting
>> NON_DEFAULT_ARGS_COUNT on it doesn't make sense.
> 
> Right, I had already added a `gcc_assert (!TMPL_ARGS_HAVE_MULTIPLE_LEVELS
> (args))` to my new set_non_default_template_args_count function and found cp/
> constraint.cc:2896 (get_mapped_args), which calls
> SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT on the outer TREE_VEC. Was this supposed
> to apply to all inner TREE_VECs? Or is deleting the line the correct fix?

That should apply to the inner TREE_VECs (and probably use list.length)

>>>>> +  /* Pretty print only template instantiations. Don't pretty print
>>>>> explicit
>>>>> +     specializations like 'template <> void fun<int> (int)'.
>>>>
>>>> This seems like a significant change of behavior unrelated to printing
>>>> default template arguments.  What's the rationale for handling
>>>> specializations differently from instantiations?
>>>
>>> Right, this is about "The general idea of this change is to print template
>>> parms wherever they would appear in the source code as well".
>>>
>>> Initially, the change to print function template arguments/parameters only
>>> if the args were explicitly specified lead to printing 'void fun (T)
>>> [with T = ...]' or 'template <> void fun (int)'. Both are not telling the
>>> full story, even if the former is how the function would be called.
>>
>> and the latter is how I expect the specialization to be declared, not
>> with the deducible template argument made explicit.
> 
> You're right. From my tests:
> 
> template <class a>
>    [[deprecated]] void f4(a);
> 
> template <>
>    [[deprecated]] void f4<int>(int);
> 
> template <>
>    [[deprecated]] void f4(float);
> 
>    f4(1.);          // { dg-warning "'void f4\\(a\\) .with a = double.'" }
>    f4(1);           // { dg-warning "'void f4<int>\\(int\\)'" }
>    f4(1.f);         // { dg-warning "'void f4\\(float\\)'" }
> 
> So how it's printed depends on how the specialization is declared. It just
> falls out that way and it didn't seem awfully wrong... ;)
> 
>>> But if the reader
>>> should quickly recognize what code is getting called, it is helpful to see
>>> right away that a function template specialization is called. (It might
>>> also reveal an implementation detail of a library, so it's not 100%
>>> obvious how to choose here.) Also, saying 'T = int' is kind of wrong.
>>> Yes, 'int' was deduced. But there's no T in fun<int>:
>>>
>>> template <class T> void fun (T);
>>> template <> void fun<int> (int);
>>
>> There's a T in the template, and as you said above, that's how it's
>> called (and mangled).
>>
>>> __FUNCTION__ was 'fun<int>' all the time, but __PRETTY_FUNCTION__ was
>>> 'void
>>> fun(T) [with T = int]'.
>>
>> Isn't that true for instantiations, as well?
> 
> No, instantiations don't have template args/parms in __FUNCTION__.

Hmm, that inconsistency seems like a bug, though I'm not sure whether it 
should have the template args or not; I lean toward not.  The standard 
says that the value of __func__ is implementation-defined, the GCC 
manual says it's "the unadorned name of the function".

>>> It's more consistent that __PRETTY_FUNCTION__ contains __FUNCTION__, IMHO
>>
>> I suppose, but I don't see that as a strong enough motivation to mix
>> this up.
> 
> What about
> 
> template <class T> void f();
> template <> void f<int>();
> 
> With -fpretty-templates shouldn't it print as 'void f<T>() [with T = float]'
> and 'void f<int>()'? Yes, it's probably too subtle for most users to notice
> the difference. But I find it's more consistent this way.

I disagree; the function signature is the same whether a particular 
function is an explicit specialization or an instantiation.

>>> so it would have to be at least 'void fun<int>(T) [with T
>>> = int]'. But that's strange: How it uses T and int for the same type. So I
>>> settled on 'void fun<int>(int)'.
>>>
>>>> I also don't understand the purpose of TFF_AS_PRIMARY.
>>>
>>> dump_function_decl generalizes the TEMPLATE_DECL (if flag_pretty_templates
>>> is true) and, before this change, passes the generalized TEMPLATE_DECL to
>>> dump_type (... DECL_CONTEXT (t) ...) and dump_function_name (... t ...).
>>> That's why the whole template is printed as primary template (i.e. with
>>> template parms instead of template args, as is needed for
>>> flag_pretty_templates). But this drops the count of non-default template
>>> args.
>> Ah, you're trying to omit defaulted parms from the <list>?  I'm not sure
>> that's necessary, leaving them out of the [with ...] list should be
>> sufficient.
> 
> I was thinking about all the std::allocator defaults in the standard library.
> I don't want to see them. E.g. vector<int>::clear() on const object:
> 
> error: passing 'const std::vector<int>' as 'this' argument discards qualifiers
> [...]/stl_vector.h:1498:7: note:   in call to 'void std::vector<_Tp,
> _Alloc>::clear() [with _Tp = int; _Alloc = std::allocator<int>]'
> 
> With my patch the last line becomes
> [...]/stl_vector.h:1498:7: note:   in call to 'void std::vector<_Tp>::clear()
> [with _Tp = int]'
> 
> 
> Another case I didn't consider before:
> 
> template <class T, class U = int> struct A {
>    [[deprecated]] void f(U);
> };
> 
> A<float> a; a.f(1);
> 
> With my patch it prints 'void A<T>::f(U) [with T = float]', with your
> suggestion 'void A<T, U>::f(U) [with T = float]'. Both are missing important
> information in the substitution list, IMHO. Would 'void A<T, U = int>::f(U)
> [with T = float]' be an improvement? Or should find_typenames (in cp/error.c)
> find defaulted template parms and add them to its list? IIUC find_typenames
> would find all template parms and couldn't know whether they're defaulted.

That sounds good: omit defaulted parms only if they don't appear in the 
signature (other than as another default template argument).

>>>>> +   4. either
>>>>> +      - flags requests to show no function arguments, in which case
>>>>> deduced +        types could be hidden, or
>>>>> +      - at least one function template argument was given explicitly,
>>>>> or
>>>>> +      - we're printing a DWARF name,
>>>>
>>>> Though, why do we want
>>>> different behavior here when printing a DWARF name?
>>>
>>> Sorry, I should have asked ... I also added the same issue as an open
>>> question on the diagnose_as patch. When I ran the whole testsuite I had
>>> failures in the DWARF tests. This change resolved them. I don't know
>>> enough about how those strings are used and whether they may change
>>> between GCC versions. Anyway, the DWARF strings in that test had only the
>>> function name and template argument list (i.e. no function arguments).
>>
>> If they only have arguments, isn't TFF_NO_FUNCTION_ARGUMENTS set?
> 
> No, it isn't set. I could try to set it for DWARF names. It sounds like the
> right solution here.

I agree.

Jason
  
Matthias Kretz Nov. 19, 2021, 9:53 a.m. UTC | #11
On Thursday, 18 November 2021 20:24:36 CET Jason Merrill wrote:
> On 11/17/21 17:51, Matthias Kretz wrote:
> > Right, I had already added a `gcc_assert (!TMPL_ARGS_HAVE_MULTIPLE_LEVELS
> > (args))` to my new set_non_default_template_args_count function and found
> > cp/ constraint.cc:2896 (get_mapped_args), which calls
> > SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT on the outer TREE_VEC. Was this
> > supposed to apply to all inner TREE_VECs? Or is deleting the line the
> > correct fix?
>
> That should apply to the inner TREE_VECs (and probably use list.length)

Like this?

@@ -2890,10 +2890,11 @@ get_mapped_args (tree map)
       tree level = make_tree_vec (list.length ());
       for (unsigned j = 0; j < list.length(); ++j)
         TREE_VEC_ELT (level, j) = list[j];
+      /* None of the args at any level are defaulted.  */
+      SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (level, list.length());
       SET_TMPL_ARGS_LEVEL (args, i + 1, level);
       list.release ();
     }
-  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args, 0);

   return args;
 }

> >>> __FUNCTION__ was 'fun<int>' all the time, but __PRETTY_FUNCTION__ was
> >>> 'void fun(T) [with T = int]'.
> >> 
> >> Isn't that true for instantiations, as well?
> > 
> > No, instantiations don't have template args/parms in __FUNCTION__.
> 
> Hmm, that inconsistency seems like a bug, though I'm not sure whether it
> should have the template args or not; I lean toward not.  The standard
> says that the value of __func__ is implementation-defined, the GCC
> manual says it's "the unadorned name of the function".

So you think f1<int> in testsuite/g++.old-deja/g++.ext/pretty3.C needs to test 
for

  if (strcmp (function, "f1"))
    bad = true;
  if (strcmp (pretty, "void f1(T) [with T = int]"))
    bad = true;

Or should the latter be "void f1(int)"?

> >>> It's more consistent that __PRETTY_FUNCTION__ contains __FUNCTION__,
> >>> IMHO
> >> 
> >> I suppose, but I don't see that as a strong enough motivation to mix
> >> this up.
> > 
> > What about
> > 
> > template <class T> void f();
> > template <> void f<int>();
> > 
> > With -fpretty-templates shouldn't it print as 'void f<T>() [with T =
> > float]' and 'void f<int>()'? Yes, it's probably too subtle for most users
> > to notice the difference. But I find it's more consistent this way.
> 
> I disagree; the function signature is the same whether a particular
> function is an explicit specialization or an instantiation.

Yes, the call signature is the same. But what it calls is different. There's 
no obvious answer for my stated goal "print template parms wherever they
would appear in the source code as well", since it depends on whether the user 
is interested in recognizing the exact function body that was called.

My motivation for printing a function template specialization differently is:

1. It's a different function definition that's being called. The user (caller) 
might be surprised to realize this is the case as he forgot about the 
specialization and was expecting his change to the general template to make a 
difference.

2. There's no T in

template <> void f<int>() {
  // no T here, but of course I can define one:
  using T = int;
}

so presenting the function that was called as 'void f<T>() [with T = int]' is 
not exactly correct. In this case it wasn't even the case that T was deduced 
to be 'int', which we could argue to be useful information that might get 
lost.

For

template <class T> void f(T);
template <> void f(int);

the whole story is "'void f(int)' was called for 'template <class T> void f(T) 
[with T = int]'". What the user wants to see depends on what is more important 
to fix the bug: that T was deduced to be int, or that the specialization of f 
was called instead of the general template. I'd still go with 'void f(int)', 
though I'd be happier if I had some indication that a template was involved.

> >> Ah, you're trying to omit defaulted parms from the <list>?  I'm not sure
> >> that's necessary, leaving them out of the [with ...] list should be
> >> sufficient.
> > 
> > I was thinking about all the std::allocator defaults in the standard
> > library. I don't want to see them. E.g. vector<int>::clear() on const
> > object:
> > 
> > error: passing 'const std::vector<int>' as 'this' argument discards
> > qualifiers [...]/stl_vector.h:1498:7: note:   in call to 'void
> > std::vector<_Tp, _Alloc>::clear() [with _Tp = int; _Alloc =
> > std::allocator<int>]'
> > 
> > With my patch the last line becomes
> > [...]/stl_vector.h:1498:7: note:   in call to 'void
> > std::vector<_Tp>::clear() [with _Tp = int]'
> > 
> > 
> > Another case I didn't consider before:
> > 
> > template <class T, class U = int> struct A {
> > 
> >    [[deprecated]] void f(U);
> > 
> > };
> > 
> > A<float> a; a.f(1);
> > 
> > With my patch it prints 'void A<T>::f(U) [with T = float]', with your
> > suggestion 'void A<T, U>::f(U) [with T = float]'. Both are missing
> > important information in the substitution list, IMHO. Would 'void A<T, U
> > = int>::f(U) [with T = float]' be an improvement? Or should
> > find_typenames (in cp/error.c) find defaulted template parms and add them
> > to its list? IIUC find_typenames would find all template parms and
> > couldn't know whether they're defaulted.
>
> That sounds good: omit defaulted parms only if they don't appear in the
> signature (other than as another default template argument).

Let me check whether I have the right idea:

I could extend find_typenames (which walks the complete) tree to look for 
TEMPLATE_TYPE_PARM (and the 3 others I don't recall right now). But since that 
walks the *complete* tree, it'll simply find all parms with no indication 
whether they appear in the signature. Ideas:

1. Count occurrences: with 2 occurrences, one of them must be a use in the 
signature.

2. Walk only over TYPE_ARG_TYPES (TREE_TYPE (DECL_TEMPLATE_RESULT (fn))) to 
collect TEMPLATE_TYPE_PARMs.
  
Matthias Kretz Nov. 19, 2021, 12:02 p.m. UTC | #12
On Friday, 19 November 2021 10:53:27 CET Matthias Kretz wrote:
> > >> Ah, you're trying to omit defaulted parms from the <list>?  I'm not
> > >> sure
> > >> that's necessary, leaving them out of the [with ...] list should be
> > >> sufficient.
> > >
> > > I was thinking about all the std::allocator defaults in the standard
> > > library. I don't want to see them. E.g. vector<int>::clear() on const
> > > object:
> > > 
> > > error: passing 'const std::vector<int>' as 'this' argument discards
> > > qualifiers [...]/stl_vector.h:1498:7: note:   in call to 'void
> > > std::vector<_Tp, _Alloc>::clear() [with _Tp = int; _Alloc =
> > > std::allocator<int>]'
> > > 
> > > With my patch the last line becomes
> > > [...]/stl_vector.h:1498:7: note:   in call to 'void
> > > std::vector<_Tp>::clear() [with _Tp = int]'
> > > 
> > > 
> > > Another case I didn't consider before:
> > > 
> > > template <class T, class U = int> struct A {
> > > 
> > > [[deprecated]] void f(U);
> > > 
> > > };
> > > 
> > > A<float> a; a.f(1);
> > > 
> > > With my patch it prints 'void A<T>::f(U) [with T = float]', with your
> > > suggestion 'void A<T, U>::f(U) [with T = float]'. Both are missing
> > > important information in the substitution list, IMHO. Would 'void A<T, U
> > > = int>::f(U) [with T = float]' be an improvement? Or should
> > > find_typenames (in cp/error.c) find defaulted template parms and add
> > > them
> > > to its list? IIUC find_typenames would find all template parms and
> > > couldn't know whether they're defaulted.
> > 
> > That sounds good: omit defaulted parms only if they don't appear in the
> > signature (other than as another default template argument).
> 
> Let me check whether I have the right idea:
> 
> I could extend find_typenames (which walks the complete) tree to look for
> TEMPLATE_TYPE_PARM (and the 3 others I don't recall right now). But since
> that walks the *complete* tree, it'll simply find all parms with no
> indication whether they appear in the signature. Ideas:
> 
> 1. Count occurrences: with 2 occurrences, one of them must be a use in the
> signature.
> 
> 2. Walk only over TYPE_ARG_TYPES (TREE_TYPE (DECL_TEMPLATE_RESULT (fn))) to
> collect TEMPLATE_TYPE_PARMs.

I tried the latter:

@@ -1641,8 +1652,11 @@ dump_substitution (cxx_pretty_printer *pp,                              
       && !(flags & TFF_NO_TEMPLATE_BINDINGS))                                                 
     {
       vec<tree, va_gc> *typenames = t ? find_typenames (t) : NULL;                            
-      dump_template_bindings (pp, template_parms, template_args, typenames,                   
-			      flags);                                                          
+      tree fn_arguments = TYPE_ARG_TYPES (TREE_TYPE (DECL_TEMPLATE_RESULT 
(t)));              
+      tree used_template_parms = find_template_parameters (fn_arguments,                      
+							   template_parms);                    
+      dump_template_bindings (pp, template_parms, template_args,                              
+			      used_template_parms, typenames, flags);                          
     }
 }

Now in dump_template_bindings it skips all defaulted template_parms that are 
not in used_template_parms. Makes this test pass:

template <class T>                                                              
  struct id                                                                     
  { using type = T; };                                                          
                                                                                
template <class T0, class T1 = int>                                             
  struct A                                                                      
  {                                                                             
    template <class U0 = const T1&>                                             
      [[deprecated]] static void                                                
      f(typename id<U0>::type);                                                 
  };                                                                            
                                                                                
int main()                                                                      
{                                                                               
  A<int>::f(0); // { dg-warning "'static void A<T0>::f\\(typename 
id<U0>::type\\) .with U0 = const int&; T0 = int; typename id<U0>::type = const 
int&.'" }
}
  
Jason Merrill Nov. 19, 2021, 10:26 p.m. UTC | #13
On 11/19/21 04:53, Matthias Kretz wrote:
> On Thursday, 18 November 2021 20:24:36 CET Jason Merrill wrote:
>> On 11/17/21 17:51, Matthias Kretz wrote:
>>> Right, I had already added a `gcc_assert (!TMPL_ARGS_HAVE_MULTIPLE_LEVELS
>>> (args))` to my new set_non_default_template_args_count function and found
>>> cp/ constraint.cc:2896 (get_mapped_args), which calls
>>> SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT on the outer TREE_VEC. Was this
>>> supposed to apply to all inner TREE_VECs? Or is deleting the line the
>>> correct fix?
>>
>> That should apply to the inner TREE_VECs (and probably use list.length)
> 
> Like this?

Yes.

> @@ -2890,10 +2890,11 @@ get_mapped_args (tree map)
>         tree level = make_tree_vec (list.length ());
>         for (unsigned j = 0; j < list.length(); ++j)
>           TREE_VEC_ELT (level, j) = list[j];
> +      /* None of the args at any level are defaulted.  */
> +      SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (level, list.length());
>         SET_TMPL_ARGS_LEVEL (args, i + 1, level);
>         list.release ();
>       }
> -  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args, 0);
> 
>     return args;
>   }
> 
>>>>> __FUNCTION__ was 'fun<int>' all the time, but __PRETTY_FUNCTION__ was
>>>>> 'void fun(T) [with T = int]'.
>>>>
>>>> Isn't that true for instantiations, as well?
>>>
>>> No, instantiations don't have template args/parms in __FUNCTION__.
>>
>> Hmm, that inconsistency seems like a bug, though I'm not sure whether it
>> should have the template args or not; I lean toward not.  The standard
>> says that the value of __func__ is implementation-defined, the GCC
>> manual says it's "the unadorned name of the function".
> 
> So you think f1<int> in testsuite/g++.old-deja/g++.ext/pretty3.C needs to test
> for
> 
>    if (strcmp (function, "f1"))
>      bad = true;
>    if (strcmp (pretty, "void f1(T) [with T = int]"))
>      bad = true;

I think so.

>>>>> It's more consistent that __PRETTY_FUNCTION__ contains __FUNCTION__,
>>>>> IMHO
>>>>
>>>> I suppose, but I don't see that as a strong enough motivation to mix
>>>> this up.
>>>
>>> What about
>>>
>>> template <class T> void f();
>>> template <> void f<int>();
>>>
>>> With -fpretty-templates shouldn't it print as 'void f<T>() [with T =
>>> float]' and 'void f<int>()'? Yes, it's probably too subtle for most users
>>> to notice the difference. But I find it's more consistent this way.
>>
>> I disagree; the function signature is the same whether a particular
>> function is an explicit specialization or an instantiation.
> 
> Yes, the call signature is the same. But what it calls is different. There's
> no obvious answer for my stated goal "print template parms wherever they
> would appear in the source code as well", since it depends on whether the user
> is interested in recognizing the exact function body that was called.
> 
> My motivation for printing a function template specialization differently is:
> 
> 1. It's a different function definition that's being called. The user (caller)
> might be surprised to realize this is the case as he forgot about the
> specialization and was expecting his change to the general template to make a
> difference.
>
> 2. There's no T in
> 
> template <> void f<int>() {
>    // no T here, but of course I can define one:
>    using T = int;
> }
> 
> so presenting the function that was called as 'void f<T>() [with T = int]' is
> not exactly correct. In this case it wasn't even the case that T was deduced
> to be 'int', which we could argue to be useful information that might get
> lost.

On the other hand, this tells us what template this specialization is 
specializing, which could be unclear if there are multiple overloaded 
function templates.

There's always -fno-pretty-templates if you want the form without 
template args.

Incidentally, the contents of __PRETTY_FUNCTION__ probably shouldn't 
vary with that flag...

> For
> 
> template <class T> void f(T);
> template <> void f(int);
> 
> the whole story is "'void f(int)' was called for 'template <class T> void f(T)
> [with T = int]'". What the user wants to see depends on what is more important
> to fix the bug: that T was deduced to be int, or that the specialization of f
> was called instead of the general template. I'd still go with 'void f(int)',
> though I'd be happier if I had some indication that a template was involved.

The current form tells you about the template, and the line number 
points you at the declaration.

>>>> Ah, you're trying to omit defaulted parms from the <list>?  I'm not sure
>>>> that's necessary, leaving them out of the [with ...] list should be
>>>> sufficient.
>>>
>>> I was thinking about all the std::allocator defaults in the standard
>>> library. I don't want to see them. E.g. vector<int>::clear() on const
>>> object:
>>>
>>> error: passing 'const std::vector<int>' as 'this' argument discards
>>> qualifiers [...]/stl_vector.h:1498:7: note:   in call to 'void
>>> std::vector<_Tp, _Alloc>::clear() [with _Tp = int; _Alloc =
>>> std::allocator<int>]'
>>>
>>> With my patch the last line becomes
>>> [...]/stl_vector.h:1498:7: note:   in call to 'void
>>> std::vector<_Tp>::clear() [with _Tp = int]'
>>>
>>>
>>> Another case I didn't consider before:
>>>
>>> template <class T, class U = int> struct A {
>>>
>>>     [[deprecated]] void f(U);
>>>
>>> };
>>>
>>> A<float> a; a.f(1);
>>>
>>> With my patch it prints 'void A<T>::f(U) [with T = float]', with your
>>> suggestion 'void A<T, U>::f(U) [with T = float]'. Both are missing
>>> important information in the substitution list, IMHO. Would 'void A<T, U
>>> = int>::f(U) [with T = float]' be an improvement? Or should
>>> find_typenames (in cp/error.c) find defaulted template parms and add them
>>> to its list? IIUC find_typenames would find all template parms and
>>> couldn't know whether they're defaulted.
>>
>> That sounds good: omit defaulted parms only if they don't appear in the
>> signature (other than as another default template argument).
> 
> Let me check whether I have the right idea:
> 
> I could extend find_typenames (which walks the complete) tree to look for
> TEMPLATE_TYPE_PARM (and the 3 others I don't recall right now). But since that
> walks the *complete* tree, it'll simply find all parms with no indication
> whether they appear in the signature. Ideas:

Hmm, since it walks DECL_TEMPLATE_RESULT, I wouldn't expect it to find 
template parms that aren't in the function signature.

> 1. Count occurrences: with 2 occurrences, one of them must be a use in the
> signature.
> 
> 2. Walk only over TYPE_ARG_TYPES (TREE_TYPE (DECL_TEMPLATE_RESULT (fn))) to
> collect TEMPLATE_TYPE_PARMs.

Jason
  
Matthias Kretz Nov. 19, 2021, 11:11 p.m. UTC | #14
On Friday, 19 November 2021 23:26:57 CET Jason Merrill wrote:
> On 11/19/21 04:53, Matthias Kretz wrote:
> > My motivation for printing a function template specialization differently
> > is:
> > 
> > 1. It's a different function definition that's being called. The user
> > (caller) might be surprised to realize this is the case as he forgot
> > about the specialization and was expecting his change to the general
> > template to make a difference.
> > 
> > 2. There's no T in
> > 
> > template <> void f<int>() {
> > // no T here, but of course I can define one:
> > using T = int;
> > }
> > 
> > so presenting the function that was called as 'void f<T>() [with T = int]'
> > is not exactly correct. In this case it wasn't even the case that T was
> > deduced to be 'int', which we could argue to be useful information that
> > might get lost.
> 
> On the other hand, this tells us what template this specialization is
> specializing, which could be unclear if there are multiple overloaded
> function templates.

I don't disagree that printing the primary template + substitution has value. 
I just believe that being more explicit about a function specialization 
getting called has more value. But I guess we'll just have to live with our 
disagreement.

I'll re-add the condition for "pretty-printing" specialized functions for the 
next patch revision, unless someone else wants to & can convince you. :)

> There's always -fno-pretty-templates if you want the form without
> template args.
> 
> Incidentally, the contents of __PRETTY_FUNCTION__ probably shouldn't
> vary with that flag...

That ship is sailing since -fpretty-templates was implemented AFAICS. Do you 
want it to come back? ;)

> >> That sounds good: omit defaulted parms only if they don't appear in the
> >> signature (other than as another default template argument).
> >
> > Let me check whether I have the right idea:
> > 
> > I could extend find_typenames (which walks the complete) tree to look for
> > TEMPLATE_TYPE_PARM (and the 3 others I don't recall right now). But since
> > that walks the *complete* tree, it'll simply find all parms with no
> > indication
> > whether they appear in the signature. Ideas:
>
> Hmm, since it walks DECL_TEMPLATE_RESULT, I wouldn't expect it to find
> template parms that aren't in the function signature.

Maybe I made an error elsewhere, but that's what I saw when I tried it. I'll 
try again (next week, I think).
  

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f387b5036d2..27f11e12812 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3686,7 +3686,8 @@  struct GTY(()) lang_decl {
   TREE_LANG_FLAG_1 (TEMPLATE_INFO_CHECK (NODE))
 /* For a given TREE_VEC containing a template argument list,
    this property contains the number of arguments that are not
-   defaulted.  */
+   defaulted. The sign of the number is negative for function templates with
+   explicitly given template arguments (i.e. neither deduced nor defaulted).  */
 #define NON_DEFAULT_TEMPLATE_ARGS_COUNT(NODE) \
   TREE_CHAIN (TREE_VEC_CHECK (NODE))
 
@@ -3696,14 +3697,21 @@  struct GTY(()) lang_decl {
   NON_DEFAULT_TEMPLATE_ARGS_COUNT(NODE) = build_int_cst (NULL_TREE, INT_VALUE)
 #if CHECKING_P
 #define GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT(NODE) \
-    int_cst_value (NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE))
+  abs (int_cst_value (NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE)))
 #else
 #define GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT(NODE) \
   NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE) \
-  ? int_cst_value (NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE)) \
+  ? abs (int_cst_value (NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE))) \
   : TREE_VEC_LENGTH (INNERMOST_TEMPLATE_ARGS (NODE))
 #endif
 
+#define EXPLICIT_TEMPLATE_ARGS_P(NODE) \
+  (int_cst_value (NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE)) < 0)
+
+#define SET_EXPLICIT_TEMPLATE_ARGS_P(NODE) \
+  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT      \
+    (NODE, -GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE))
+
 /* The list of access checks that were deferred during parsing
    which need to be performed at template instantiation time.
 
@@ -5938,7 +5946,8 @@  enum auto_deduction_context
        identical to their defaults.
    TFF_NO_TEMPLATE_BINDINGS: do not print information about the template
        arguments for a function template specialization.
-   TFF_POINTER: we are printing a pointer type.  */
+   TFF_POINTER: we are printing a pointer type.
+   TFF_AS_PRIMARY: show the template like a primary template.  */
 
 #define TFF_PLAIN_IDENTIFIER			(0)
 #define TFF_SCOPE				(1)
@@ -5956,6 +5965,7 @@  enum auto_deduction_context
 #define TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS	(1 << 12)
 #define TFF_NO_TEMPLATE_BINDINGS		(1 << 13)
 #define TFF_POINTER		                (1 << 14)
+#define TFF_AS_PRIMARY		                (1 << 15)
 
 /* These constants can be used as bit flags to control strip_typedefs.
 
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 012a4ecddf4..86e9d12103a 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -87,7 +87,7 @@  static void dump_template_argument (cxx_pretty_printer *, tree, int);
 static void dump_template_argument_list (cxx_pretty_printer *, tree, int);
 static void dump_template_parameter (cxx_pretty_printer *, tree, int);
 static void dump_template_bindings (cxx_pretty_printer *, tree, tree,
-                                    vec<tree, va_gc> *);
+                                    vec<tree, va_gc> *, int);
 static void dump_scope (cxx_pretty_printer *, tree, int);
 static void dump_template_parms (cxx_pretty_printer *, tree, int, int);
 static int get_non_default_template_args_count (tree, int);
@@ -278,7 +278,8 @@  dump_template_argument (cxx_pretty_printer *pp, tree arg, int flags)
 static int
 get_non_default_template_args_count (tree args, int flags)
 {
-  int n = TREE_VEC_LENGTH (INNERMOST_TEMPLATE_ARGS (args));
+  args = INNERMOST_TEMPLATE_ARGS (args);
+  int n = TREE_VEC_LENGTH (args);
 
   if (/* We use this flag when generating debug information.  We don't
 	 want to expand templates at this point, for this may generate
@@ -286,10 +287,10 @@  get_non_default_template_args_count (tree args, int flags)
 	 turn cause codegen differences between compilations with and
 	 without -g.  */
       (flags & TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS) != 0
-      || !flag_pretty_templates)
+      || !NON_DEFAULT_TEMPLATE_ARGS_COUNT (args))
     return n;
 
-  return GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (INNERMOST_TEMPLATE_ARGS (args));
+  return GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args);
 }
 
 /* Dump a template-argument-list ARGS (always a TREE_VEC) under control
@@ -369,7 +370,7 @@  dump_template_parameter (cxx_pretty_printer *pp, tree parm, int flags)
 
 static void
 dump_template_bindings (cxx_pretty_printer *pp, tree parms, tree args,
-                        vec<tree, va_gc> *typenames)
+                        vec<tree, va_gc> *typenames, int flags)
 {
   /* Print "[with" and ']', conditional on whether anything is printed at all.
      This is tied to whether a semicolon is needed to separate multiple template
@@ -411,11 +412,16 @@  dump_template_bindings (cxx_pretty_printer *pp, tree parms, tree args,
       int i;
       tree lvl_args = NULL_TREE;
 
+      int len = TREE_VEC_LENGTH (p);
       /* Don't crash if we had an invalid argument list.  */
       if (TMPL_ARGS_DEPTH (args) >= lvl)
-	lvl_args = TMPL_ARGS_LEVEL (args, lvl);
+	{
+	  lvl_args = TMPL_ARGS_LEVEL (args, lvl);
+	  len = MIN (len,
+		     get_non_default_template_args_count (lvl_args, flags));
+	}
 
-      for (i = 0; i < TREE_VEC_LENGTH (p); ++i)
+      for (i = 0; i < len; ++i)
 	{
 	  tree arg = NULL_TREE;
 
@@ -1635,7 +1641,8 @@  dump_substitution (cxx_pretty_printer *pp,
       && !(flags & TFF_NO_TEMPLATE_BINDINGS))
     {
       vec<tree, va_gc> *typenames = t ? find_typenames (t) : NULL;
-      dump_template_bindings (pp, template_parms, template_args, typenames);
+      dump_template_bindings (pp, template_parms, template_args, typenames,
+			      flags);
     }
 }
 
@@ -1688,8 +1695,15 @@  dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
   /* Likewise for the constexpr specifier, in case t is a specialization.  */
   constexpr_p = DECL_DECLARED_CONSTEXPR_P (t);
 
-  /* Pretty print template instantiations only.  */
-  if (DECL_USE_TEMPLATE (t) && DECL_TEMPLATE_INFO (t)
+  /* Keep t before the following branch makes t point to a more general
+     template. Without the specialized template, the information about defaulted
+     template arguments is lost.  */
+  tree specialized_t = t;
+  int specialized_flags = 0;
+
+  /* Pretty print only template instantiations. Don't pretty print explicit
+     specializations like 'template <> void fun<int> (int)'.  */
+  if (DECL_TEMPLATE_INSTANTIATION (t) && DECL_TEMPLATE_INFO (t)
       && !(flags & TFF_NO_TEMPLATE_BINDINGS)
       && flag_pretty_templates)
     {
@@ -1701,6 +1715,9 @@  dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
 	{
 	  template_parms = DECL_TEMPLATE_PARMS (tmpl);
 	  t = tmpl;
+	  /* The "[with ...]" clause is printed, thus dump functions printing
+	     SPECIALIZED_T need to add TFF_AS_PRIMARY to their flags.  */
+	  specialized_flags = TFF_AS_PRIMARY;
 	}
     }
 
@@ -1710,8 +1727,8 @@  dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
   fntype = TREE_TYPE (t);
   parmtypes = FUNCTION_FIRST_USER_PARMTYPE (t);
 
-  if (DECL_CLASS_SCOPE_P (t))
-    cname = DECL_CONTEXT (t);
+  if (DECL_CLASS_SCOPE_P (specialized_t))
+    cname = DECL_CONTEXT (specialized_t);
   /* This is for partially instantiated template methods.  */
   else if (TREE_CODE (fntype) == METHOD_TYPE)
     cname = TREE_TYPE (TREE_VALUE (parmtypes));
@@ -1749,13 +1766,14 @@  dump_function_decl (cxx_pretty_printer *pp, tree t, int flags)
     /* Nothing.  */;
   else if (cname)
     {
-      dump_type (pp, cname, flags);
+      dump_type (pp, cname, flags | specialized_flags);
       pp_cxx_colon_colon (pp);
     }
   else
     dump_scope (pp, CP_DECL_CONTEXT (t), flags);
 
-  dump_function_name (pp, t, dump_function_name_flags);
+  dump_function_name (pp, specialized_t,
+		      dump_function_name_flags | specialized_flags);
 
   if (!(flags & TFF_NO_FUNCTION_ARGUMENTS))
     {
@@ -1968,13 +1986,35 @@  dump_function_name (cxx_pretty_printer *pp, tree t, int flags)
 
   dump_module_suffix (pp, t);
 
+/* Print function template parameters if:
+   1. t is template, and
+   2. flags did not request "show only template-name", and
+   3. t is a specialization of a template (Why is this needed? This was present
+      since 1999 via !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION: "Don't crash if
+      given a friend pseudo-instantiation". The DECL_USE_TEMPLATE should likely
+      inform the PRIMARY parameter of dump_template_parms.), and
+   4. either
+      - flags requests to show no function arguments, in which case deduced
+        types could be hidden, or
+      - at least one function template argument was given explicitly, or
+      - we're printing a DWARF name,
+      and
+   5. either
+      - t is a member friend template of a template class (see DECL_TI_TEMPLATE
+        documentation), or
+      - 
+ */
   if (DECL_TEMPLATE_INFO (t)
       && !(flags & TFF_TEMPLATE_NAME)
-      && !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t)
+      && DECL_USE_TEMPLATE (t)
+      && ((flags & TFF_NO_FUNCTION_ARGUMENTS)
+	    || (DECL_TI_ARGS (t)
+		  && EXPLICIT_TEMPLATE_ARGS_P (INNERMOST_TEMPLATE_ARGS
+						 (DECL_TI_ARGS (t))))
+	    || (pp->flags & pp_c_flag_gnu_v3) != 0)
       && (TREE_CODE (DECL_TI_TEMPLATE (t)) != TEMPLATE_DECL
 	  || PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (t))))
-    dump_template_parms (pp, DECL_TEMPLATE_INFO (t), !DECL_USE_TEMPLATE (t),
-                         flags);
+    dump_template_parms (pp, DECL_TEMPLATE_INFO (t), false, flags);
 }
 
 /* Dump the template parameters from the template info INFO under control of
@@ -1989,6 +2029,8 @@  dump_template_parms (cxx_pretty_printer *pp, tree info,
 {
   tree args = info ? TI_ARGS (info) : NULL_TREE;
 
+  if (flags & TFF_AS_PRIMARY)
+    primary = true;
   if (primary && flags & TFF_TEMPLATE_NAME)
     return;
   flags &= ~(TFF_CLASS_KEY_OR_ENUM | TFF_TEMPLATE_NAME);
@@ -1998,10 +2040,11 @@  dump_template_parms (cxx_pretty_printer *pp, tree info,
      to crash producing error messages.  */
   if (args && !primary)
     {
-      int len, ix;
-      len = get_non_default_template_args_count (args, flags);
+      int ix;
 
       args = INNERMOST_TEMPLATE_ARGS (args);
+      const int len = MIN (NUM_TMPL_ARGS (args),
+			   get_non_default_template_args_count (args, flags));
       for (ix = 0; ix != len; ix++)
 	{
 	  tree arg = TREE_VEC_ELT (args, ix);
@@ -2028,6 +2071,8 @@  dump_template_parms (cxx_pretty_printer *pp, tree info,
 
       parms = TREE_CODE (parms) == TREE_LIST ? TREE_VALUE (parms) : NULL_TREE;
       len = parms ? TREE_VEC_LENGTH (parms) : 0;
+      if (args)
+	len = MIN (len, get_non_default_template_args_count (args, flags));
 
       for (ix = 0; ix != len; ix++)
 	{
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 66040035b2f..800249f0933 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -2504,7 +2504,7 @@  determine_specialization (tree template_id,
   if (candidates)
     {
       tree fn = TREE_VALUE (candidates);
-      *targs_out = copy_node (DECL_TI_ARGS (fn));
+      *targs_out = copy_template_args (DECL_TI_ARGS (fn));
 
       /* Propagate the candidate's constraints to the declaration.  */
       if (tsk != tsk_template)
@@ -4828,11 +4828,16 @@  template_parms_level_to_args (tree parms)
 {
   tree a = copy_node (parms);
   TREE_TYPE (a) = NULL_TREE;
+  int nondefault = 0;
   for (int i = TREE_VEC_LENGTH (a) - 1; i >= 0; --i)
-    TREE_VEC_ELT (a, i) = template_parm_to_arg (TREE_VEC_ELT (a, i));
+    {
+      tree elt = TREE_VEC_ELT (a, i);
+      TREE_VEC_ELT (a, i) = template_parm_to_arg (elt);
+      if (!elt || elt == error_mark_node || !TREE_PURPOSE (elt))
+	++nondefault;
+    }
 
-  if (CHECKING_P)
-    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, TREE_VEC_LENGTH (a));
+  SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (a, nondefault);
 
   return a;
 }
@@ -13332,8 +13337,9 @@  copy_template_args (tree t)
       TREE_VEC_ELT (new_vec, i) = elt;
     }
 
-  NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_vec)
-    = NON_DEFAULT_TEMPLATE_ARGS_COUNT (t);
+  if (!TMPL_ARGS_HAVE_MULTIPLE_LEVELS (t))
+    NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_vec)
+      = NON_DEFAULT_TEMPLATE_ARGS_COUNT (t);
 
   return new_vec;
 }
@@ -13433,7 +13439,13 @@  tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl)
      if it doesn't contain any nested TREE_VEC.  */
   if (NON_DEFAULT_TEMPLATE_ARGS_COUNT (orig_t))
     {
-      int count = GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (orig_t);
+      /* If ARGS defines a number for the defaulted argument count then that's
+	 the correct number to propagate. Otherwise, assume the number of
+	 defaulted arguments after substitution equals the number of default
+	 arguments before substitution (i.e. ORIG_T).  */
+      int count = args && NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)
+		    ? GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (args)
+		    : GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (orig_t);
       count += expanded_len_adjust;
       SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (t, count);
     }
@@ -21874,6 +21886,12 @@  fn_type_unification (tree fn,
 	excessive_deduction_depth = false;
     }
 
+  /* If all template parameters were explicitly given, treat them like default
+     template arguments for diagnostics. NON_DEFAULT_TEMPLATE_ARGS_COUNT must be
+     present for SET_EXPLICIT_TEMPLATE_ARGS_P.  */
+  if (explicit_targs && NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs))
+    SET_EXPLICIT_TEMPLATE_ARGS_P (targs);
+
   return r;
 }
 
@@ -22516,9 +22534,17 @@  type_unification_real (tree tparms,
 	     be NULL_TREE or ERROR_MARK_NODE, so we do not need
 	     to explicitly check cxx_dialect here.  */
 	  if (TREE_PURPOSE (TREE_VEC_ELT (tparms, i)))
-	    /* OK, there is a default argument.  Wait until after the
-	       conversion check to do substitution.  */
-	    continue;
+	    {
+	      /* The position of the first default template argument,
+		 is also the number of non-defaulted arguments in TARGS.
+		 Record that.  */
+	      if (!NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs))
+		SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs, i);
+
+	      /* OK, there is a default argument.  Wait until after the
+		 conversion check to do substitution.  */
+	      continue;
+	    }
 
 	  /* If the type parameter is a parameter pack, then it will
 	     be deduced to an empty parameter pack.  */
@@ -22621,14 +22647,7 @@  type_unification_real (tree tparms,
 	  if (arg == error_mark_node)
 	    return 1;
 	  else if (arg)
-	    {
-	      TREE_VEC_ELT (targs, i) = arg;
-	      /* The position of the first default template argument,
-		 is also the number of non-defaulted arguments in TARGS.
-		 Record that.  */
-	      if (!NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs))
-		SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (targs, i);
-	    }
+	    TREE_VEC_ELT (targs, i) = arg;
 	}
 
       if (saw_undeduced++ == 1)
@@ -24998,6 +25017,9 @@  get_partial_spec_bindings (tree tmpl, tree spec_tmpl, tree args)
   if (!template_template_parm_bindings_ok_p (tparms, deduced_args))
     return NULL_TREE;
 
+  if (CHECKING_P)
+    SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (deduced_args, ntparms);
+
   return deduced_args;
 }
 
diff --git a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
index d3c1f589f87..b88bf7d4b34 100644
--- a/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
+++ b/gcc/testsuite/g++.dg/debug/dwarf2/template-params-12n.C
@@ -1,6 +1,6 @@ 
 // { dg-options "-gdwarf-2 -dA" }
 // { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param" 1 } }
-// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n\[^\n\]* DW_AT_const_value" 1 } }
+// { dg-final { scan-assembler-times "DIE \\(\[^\n\]*\\) DW_TAG_template_value_param\[^\n\]*\n\[^\n\]* DW_AT_name\n\[^\n\]* DW_AT_type\n(?:\[^\n\]* DW_AT_default_value\n)?\[^\n\]* DW_AT_const_value" 1 } }
 #include "template-params-12.H"
 /* We get const_value for NULL pointers to member functions.  */
 #if __cplusplus > 199711L // Ugh, C++98 barfs at both the cast and the overload.
diff --git a/gcc/testsuite/g++.dg/diagnostic/default-template-args-1.C b/gcc/testsuite/g++.dg/diagnostic/default-template-args-1.C
new file mode 100644
index 00000000000..7a535515740
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/default-template-args-1.C
@@ -0,0 +1,73 @@ 
+// { dg-options "-fpretty-templates" }
+// { dg-do compile { target c++11 } }
+
+template <int a = 1>
+  [[deprecated]] void f0(); // { dg-message "'template<int a> void f0\\(\\)'" }
+
+template <int a>
+  [[deprecated]] void f1(); // { dg-message "'template<int a> void f1\\(\\)'" }
+
+template <class a, int b = 1>
+  [[deprecated]] void f2(); // { dg-message "'template<class a, int b> void f2\\(\\)'" }
+
+template <class a, int b = 1>
+  [[deprecated]] void f3(a); // { dg-message "'template<class a, int b> void f3\\(a\\)'" }
+
+template <class a>
+  [[deprecated]] void f4(a); // { dg-message "'template<class a> void f4\\(a\\)'" }
+
+template <>
+  [[deprecated]] void f4<int>(int);
+
+template <>
+  [[deprecated]] void f4(float);
+
+template <class a, class b = int>
+  [[deprecated]] void f5(a);
+
+template void f5<float>(float); // { dg-error "'void f5<a>\\(a\\) .with a = float.'" }
+
+template <class a, class b = int>
+  struct c
+  {
+    template <class d, class e = int>
+      [[deprecated]] static void f(d);
+  };
+
+template <class T>
+struct B { typedef T X; };
+
+template <class U>
+struct D
+{
+  template <class V = typename B<U>::X>
+    [[deprecated]] static void foo (typename B<V>::X);
+};
+
+int main()
+{
+  f0();            // { dg-warning "'void f0\\(\\)'" }
+  f1<1>();         // { dg-warning "'void f1<a>\\(\\) .with int a = 1.'" }
+  f2<int>();       // { dg-warning "'void f2<a>\\(\\) .with a = int.'" }
+  f3(1);           // { dg-warning "'void f3\\(a\\) .with a = int.'" }
+  f3<float>(1);    // { dg-warning "'void f3<a>\\(a\\) .with a = float.'" }
+  f3<float, 2>(1); // { dg-warning "'void f3<a, b>\\(a\\) .with a = float; int b = 2.'" }
+  f4(1.);          // { dg-warning "'void f4\\(a\\) .with a = double.'" }
+  f4(1);           // { dg-warning "'void f4<int>\\(int\\)'" }
+  f4(1.f);         // { dg-warning "'void f4\\(float\\)'" }
+
+  f0(0); // { dg-error "" }
+  f1(0); // { dg-error "" }
+  f2(0); // { dg-error "" }
+  f3();  // { dg-error "" }
+  f4();  // { dg-error "" }
+
+  c<int>::f(1.);    // { dg-warning "'static void c<a>::f\\(d\\) .with d = double; a = int.'" }
+  c<int>::f<int>(1);    // { dg-warning "'static void c<a>::f<d>\\(d\\) .with d = int; a = int.'" }
+  c<int>::f<float, int>(1.f);    // { dg-warning "'static void c<a>::f<d, e>\\(d\\) .with d = float; e = int; a = int.'" }
+  c<float, int>::f(1.);    // { dg-warning "'static void c<a, b>::f\\(d\\) .with d = double; a = float; b = int.'" }
+  c<float, int>::f<int>(1);    // { dg-warning "'static void c<a, b>::f<d>\\(d\\) .with d = int; a = float; b = int.'" }
+  c<float, int>::f<float, int>(1.f);    // { dg-warning "'static void c<a, b>::f<d, e>\\(d\\) .with d = float; e = int; a = float; b = int.'" }
+
+  D<int>::foo(1); // { dg-warning "'static void D<U>::foo\\(typename B<V>::X\\) .with U = int; typename B<V>::X = int.'" }
+}
diff --git a/gcc/testsuite/g++.dg/diagnostic/default-template-args-2.C b/gcc/testsuite/g++.dg/diagnostic/default-template-args-2.C
new file mode 100644
index 00000000000..a82709f5785
--- /dev/null
+++ b/gcc/testsuite/g++.dg/diagnostic/default-template-args-2.C
@@ -0,0 +1,37 @@ 
+// { dg-options "-fno-pretty-templates" }
+// { dg-do compile { target c++11 } }
+
+template <int a = 1>
+  [[deprecated]] void f0();
+
+template <int a>
+  [[deprecated]] void f1();
+
+template <class a, int b = 1>
+  [[deprecated]] void f2();
+
+template <class a, int b = 1>
+  [[deprecated]] void f3(a);
+
+template <class a>
+  [[deprecated]] void f4(a);
+
+template <>
+  [[deprecated]] void f4<int>(int);
+
+template <class a, class b = int>
+  [[deprecated]] void f5(a);
+
+template void f5<float>(float); // { dg-error "'void f5<float>\\(float\\)'" }
+
+int main()
+{
+  f0();            // { dg-warning "'void f0\\(\\)'" }
+  f1<1>();         // { dg-warning "'void f1<1>\\(\\)'" }
+  f2<int>();       // { dg-warning "'void f2<int>\\(\\)'" }
+  f3(1);           // { dg-warning "'void f3\\(int\\)'" }
+  f3<float>(1);    // { dg-warning "'void f3<float>\\(float\\)'" }
+  f3<float, 2>(1); // { dg-warning "'void f3<float, 2>\\(float\\)'" }
+  f4(1.);          // { dg-warning "'void f4\\(double\\)'" }
+  f4(1);           // { dg-warning "'void f4<int>\\(int\\)'" }
+}
diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
index de7570a6efa..0aa45404283 100644
--- a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
+++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
@@ -141,7 +141,7 @@  int test_7 (int first, const char *second, float third)
                                          |
                                          const char*
      { dg-end-multiline-output "" } */
-  // { dg-message "initializing argument 2 of 'int test_7\\(int, T, float\\) .with T = const char\\*\\*.'" "" { target *-*-* } test_7_decl }
+  // { dg-message "initializing argument 2 of 'int test_7<T>\\(int, T, float\\) .with T = const char\\*\\*.'" "" { target *-*-* } test_7_decl }
   /* { dg-begin-multiline-output "" }
  int test_7 (int one, T two, float three);
                       ~~^~~
diff --git a/gcc/testsuite/g++.dg/ext/pretty1.C b/gcc/testsuite/g++.dg/ext/pretty1.C
index 06608ae30eb..c5bfd6082a7 100644
--- a/gcc/testsuite/g++.dg/ext/pretty1.C
+++ b/gcc/testsuite/g++.dg/ext/pretty1.C
@@ -60,7 +60,7 @@  __assert_fail (const char *cond, const char *file, unsigned int line,
   abort ();
 }
 
-// { dg-final { scan-assembler "int bar\\(T\\).*with T = int" } }
+// { dg-final { scan-assembler "int bar<int>\\(int\\)" } }
 // { dg-final { scan-assembler "top level" } }
 // { dg-final { scan-assembler "int main\\(\\)" } }
 // { dg-final { scan-assembler "int bar\\(T\\).*with T = double" } }
diff --git a/gcc/testsuite/g++.dg/goacc/template.C b/gcc/testsuite/g++.dg/goacc/template.C
index 10d3f446da7..4fcd88bfc56 100644
--- a/gcc/testsuite/g++.dg/goacc/template.C
+++ b/gcc/testsuite/g++.dg/goacc/template.C
@@ -157,12 +157,12 @@  main ()
 }
 
 /* { dg-final { scan-tree-dump-times {(?n)^OpenACC routine '[^']+' has 'nohost' clause\.$} 4 oaccloops } }
-   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble\(int\) \[with T = char\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
+   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble<T>\(int\) \[with T = char\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
    { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'accDouble<char>\(int\)char' has 'nohost' clause\.$} 1 oaccloops { target offloading_enabled } } }
-   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble\(int\) \[with T = int\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
+   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble<T>\(int\) \[with T = int\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
    { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'accDouble<int>\(int\)int' has 'nohost' clause\.$} 1 oaccloops { target offloading_enabled } } }
-   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble\(int\) \[with T = float\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
+   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble<T>\(int\) \[with T = float\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
    { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'accDouble<float>\(int\)float' has 'nohost' clause\.$} 1 oaccloops { target offloading_enabled } } }
-   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble\(int\) \[with T = double\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
+   { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'T accDouble<T>\(int\) \[with T = double\]' has 'nohost' clause\.$} 1 oaccloops { target { ! offloading_enabled } } } }
    { dg-final { scan-tree-dump-times {(?n)^OpenACC routine 'accDouble<double>\(int\)double' has 'nohost' clause\.$} 1 oaccloops { target offloading_enabled } } }
    TODO See PR101551 for 'offloading_enabled' differences.  */
diff --git a/gcc/testsuite/g++.dg/gomp/declare-variant-7.C b/gcc/testsuite/g++.dg/gomp/declare-variant-7.C
index 7dda899578a..9cb654cb5e6 100644
--- a/gcc/testsuite/g++.dg/gomp/declare-variant-7.C
+++ b/gcc/testsuite/g++.dg/gomp/declare-variant-7.C
@@ -70,6 +70,6 @@  test ()
   s.f12 (0.0);		// { dg-final { scan-tree-dump-times "S<1>::f11<double> \\\(&s, 0.0\\\);" 1 "gimple" } }
   s.f14 (0LL);		// { dg-final { scan-tree-dump-times "S<1>::f13<long long int> \\\(&s, 0\\\);" 1 "gimple" } }
   T<0> t;
-  t.f16 (s);		// { dg-final { scan-tree-dump-times "T<0>::f16<S<1> > \\\(&t, s\\\);" 1 "gimple" } }
-  t.f18 (s);		// { dg-final { scan-tree-dump-times "T<0>::f18<S<1> > \\\(&t, s\\\);" 1 "gimple" } }
+  t.f16 (s);		// { dg-final { scan-tree-dump-times "T<0>::f16 \\\(&t, s\\\);" 1 "gimple" } }
+  t.f18 (s);		// { dg-final { scan-tree-dump-times "T<0>::f18 \\\(&t, s\\\);" 1 "gimple" } }
 }
diff --git a/gcc/testsuite/g++.dg/template/error40.C b/gcc/testsuite/g++.dg/template/error40.C
index c5df56fc1be..16a44d1819f 100644
--- a/gcc/testsuite/g++.dg/template/error40.C
+++ b/gcc/testsuite/g++.dg/template/error40.C
@@ -8,11 +8,11 @@  struct A
 
 void foo(void)
 {
-  A<void> a = 0;		// { dg-error "A<void, 0, 1>" }
+  A<void> a = 0;		// { dg-error "A<void>" }
 }
 
-template <class T> T f(T);	    // { dg-message "int f<int>.int." }
-template <class T> T f(T, int = 0); // { dg-message "" }
+template <class T> T f(T);	    // { dg-message "int f.int." }
+template <class T> T f(T, int = 0); // { dg-message "int f.int, int." }
 
 template <class T>
 struct B
diff --git a/gcc/testsuite/g++.old-deja/g++.ext/pretty3.C b/gcc/testsuite/g++.old-deja/g++.ext/pretty3.C
index 6348ae1ee67..30c7ecd5065 100644
--- a/gcc/testsuite/g++.old-deja/g++.ext/pretty3.C
+++ b/gcc/testsuite/g++.old-deja/g++.ext/pretty3.C
@@ -35,7 +35,7 @@  template<> void f1<int> (int)
   
   if (strcmp (function, "f1<int>"))
     bad = true;
-  if (strcmp (pretty, "void f1(T) [with T = int]"))
+  if (strcmp (pretty, "void f1<int>(int)"))
     bad = true;
 }
 
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C b/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C
index 6dd4b546c0c..93dbf4b489f 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/memtemp77.C
@@ -19,7 +19,7 @@  const char* S3<char>::h(int) { return __PRETTY_FUNCTION__; }
 int main()
 {
   if (strcmp (S3<double>::h(7), 
-	      "static const char* S3<T>::h(U) [with U = int; T = double]") == 0)
+	      "static const char* S3<double>::h(int)") == 0)
     return 0;
   else 
     return 1;