| Message ID | 20200428134822.3806212-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Parameter pack in requires parameter list [PR94808] | expand |
On 4/28/20 9:48 AM, Patrick Palka wrote: > When printing the substituted parameter list of a requires-expression as > part of the "in requirements with ..." context line during concepts > diagnostics, we weren't considering that substitution into a parameter > pack can yield zero or multiple parameters. > > Since this patch affects only concepts diagnostics, so far I tested with > RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer ICE > with the unreduced testcase in the PR. Is this OK to commit after a > full bootstrap and regtest? OK. Though I wonder about printing the dependent form and template arguments instead. Do you have an opinion? > gcc/cp/ChangeLog: > > PR c++/94808 > * error.c (print_requires_expression_info): Substitute into and > collect all parameters in a vector first before printing them. > Handle zero or multiple parameters of an expanded parameter > pack. > > gcc/testsuite/ChangeLog: > > PR c++/94808 > * g++.dg/concepts/diagnostic12.C: New test. > --- > gcc/cp/error.c | 21 ++++++++++++++------ > gcc/testsuite/g++.dg/concepts/diagnostic12.C | 14 +++++++++++++ > 2 files changed, 29 insertions(+), 6 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C > > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > index 98c163db572..a314412988f 100644 > --- a/gcc/cp/error.c > +++ b/gcc/cp/error.c > @@ -3752,8 +3752,7 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a > pp_verbatim (context->printer, "in requirements "); > > tree parms = TREE_OPERAND (expr, 0); > - if (parms) > - pp_verbatim (context->printer, "with "); > + auto_vec<tree> substed_parms; > while (parms) > { > tree next = TREE_CHAIN (parms); > @@ -3761,15 +3760,25 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a > TREE_CHAIN (parms) = NULL_TREE; > cp_unevaluated u; > tree p = tsubst (parms, args, tf_none, NULL_TREE); > - pp_verbatim (context->printer, "%q#D", p); > + while (p) > + { > + substed_parms.safe_push (p); > + p = TREE_CHAIN (p); > + } > TREE_CHAIN (parms) = next; > > - if (next) > - pp_separate_with_comma ((cxx_pretty_printer *)context->printer); > - > parms = next; > } > > + for (unsigned i = 0; i < substed_parms.length (); i++) > + { > + if (i == 0) > + pp_verbatim (context->printer, "with "); > + else > + pp_separate_with_comma ((cxx_pretty_printer *)context->printer); > + pp_verbatim (context->printer, "%q#D", substed_parms[i]); > + } > + > pp_verbatim (context->printer, "\n"); > } > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > new file mode 100644 > index 00000000000..7dd286291f3 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > @@ -0,0 +1,14 @@ > +// PR c++/94808 > +// { dg-do compile { target concepts } } > + > +template<typename T, typename... Args> > + concept c1 = requires (T t, Args... args) { *t; }; > +// { dg-message "in requirements with .int t." "" { target *-*-* } .-1 } > + > +static_assert(c1<int>); // { dg-error "failed" } > + > +template<typename T, typename... Args> > + concept c2 = requires (T t, Args... args) { *t; }; > +// { dg-message "in requirements with .int t., .char args#0., .bool args#1." "" { target *-*-* } .-1 } > + > +static_assert(c2<int, char, bool>); // { dg-error "failed" } >
On 4/28/20 9:48 AM, Patrick Palka via Gcc-patches wrote: > When printing the substituted parameter list of a requires-expression as > part of the "in requirements with ..." context line during concepts > diagnostics, we weren't considering that substitution into a parameter > pack can yield zero or multiple parameters. > > Since this patch affects only concepts diagnostics, so far I tested with > RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer ICE > with the unreduced testcase in the PR. Is this OK to commit after a > full bootstrap and regtest? > > gcc/cp/ChangeLog: > > PR c++/94808 > * error.c (print_requires_expression_info): Substitute into and > collect all parameters in a vector first before printing them. > Handle zero or multiple parameters of an expanded parameter > pack. ok, thanks
On Tue, 28 Apr 2020, Jason Merrill wrote: > On 4/28/20 9:48 AM, Patrick Palka wrote: > > When printing the substituted parameter list of a requires-expression as > > part of the "in requirements with ..." context line during concepts > > diagnostics, we weren't considering that substitution into a parameter > > pack can yield zero or multiple parameters. > > > > Since this patch affects only concepts diagnostics, so far I tested with > > RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer ICE > > with the unreduced testcase in the PR. Is this OK to commit after a > > full bootstrap and regtest? > > OK. Thanks for the review. > > Though I wonder about printing the dependent form and template arguments > instead. Do you have an opinion? I was going to say that on the one hand, it's convenient to have the substituted form printed since it would let us to see through complicated dependent type aliases, but it seems we don't strip type aliases when pretty printing a parameter and its type with '%q#D' anyway. And I can't think of any other possible advantage of printing the substituted form. So IMHO printing the dependent form and template arguments instead would be better here. I'll try to write a patch for this today. > > > gcc/cp/ChangeLog: > > > > PR c++/94808 > > * error.c (print_requires_expression_info): Substitute into and > > collect all parameters in a vector first before printing them. > > Handle zero or multiple parameters of an expanded parameter > > pack. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/94808 > > * g++.dg/concepts/diagnostic12.C: New test. > > --- > > gcc/cp/error.c | 21 ++++++++++++++------ > > gcc/testsuite/g++.dg/concepts/diagnostic12.C | 14 +++++++++++++ > > 2 files changed, 29 insertions(+), 6 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > > index 98c163db572..a314412988f 100644 > > --- a/gcc/cp/error.c > > +++ b/gcc/cp/error.c > > @@ -3752,8 +3752,7 @@ print_requires_expression_info (diagnostic_context > > *context, tree constr, tree a > > pp_verbatim (context->printer, "in requirements "); > > tree parms = TREE_OPERAND (expr, 0); > > - if (parms) > > - pp_verbatim (context->printer, "with "); > > + auto_vec<tree> substed_parms; > > while (parms) > > { > > tree next = TREE_CHAIN (parms); > > @@ -3761,15 +3760,25 @@ print_requires_expression_info (diagnostic_context > > *context, tree constr, tree a > > TREE_CHAIN (parms) = NULL_TREE; > > cp_unevaluated u; > > tree p = tsubst (parms, args, tf_none, NULL_TREE); > > - pp_verbatim (context->printer, "%q#D", p); > > + while (p) > > + { > > + substed_parms.safe_push (p); > > + p = TREE_CHAIN (p); > > + } > > TREE_CHAIN (parms) = next; > > - if (next) > > - pp_separate_with_comma ((cxx_pretty_printer *)context->printer); > > - > > parms = next; > > } > > + for (unsigned i = 0; i < substed_parms.length (); i++) > > + { > > + if (i == 0) > > + pp_verbatim (context->printer, "with "); > > + else > > + pp_separate_with_comma ((cxx_pretty_printer *)context->printer); > > + pp_verbatim (context->printer, "%q#D", substed_parms[i]); > > + } > > + > > pp_verbatim (context->printer, "\n"); > > } > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > new file mode 100644 > > index 00000000000..7dd286291f3 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > @@ -0,0 +1,14 @@ > > +// PR c++/94808 > > +// { dg-do compile { target concepts } } > > + > > +template<typename T, typename... Args> > > + concept c1 = requires (T t, Args... args) { *t; }; > > +// { dg-message "in requirements with .int t." "" { target *-*-* } .-1 } > > + > > +static_assert(c1<int>); // { dg-error "failed" } > > + > > +template<typename T, typename... Args> > > + concept c2 = requires (T t, Args... args) { *t; }; > > +// { dg-message "in requirements with .int t., .char args#0., .bool > > args#1." "" { target *-*-* } .-1 } > > + > > +static_assert(c2<int, char, bool>); // { dg-error "failed" } > > > >
On Tue, 28 Apr 2020, Patrick Palka wrote: > On Tue, 28 Apr 2020, Jason Merrill wrote: > > On 4/28/20 9:48 AM, Patrick Palka wrote: > > > When printing the substituted parameter list of a requires-expression as > > > part of the "in requirements with ..." context line during concepts > > > diagnostics, we weren't considering that substitution into a parameter > > > pack can yield zero or multiple parameters. > > > > > > Since this patch affects only concepts diagnostics, so far I tested with > > > RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer ICE > > > with the unreduced testcase in the PR. Is this OK to commit after a > > > full bootstrap and regtest? > > > > OK. > > Thanks for the review. > > > > > Though I wonder about printing the dependent form and template arguments > > instead. Do you have an opinion? > > I was going to say that on the one hand, it's convenient to have the > substituted form printed since it would let us to see through > complicated dependent type aliases, but it seems we don't strip type > aliases when pretty printing a parameter and its type with '%q#D' > anyway. And I can't think of any other possible advantage of printing > the substituted form. > > So IMHO printing the dependent form and template arguments instead would > be better here. I'll try to write a patch for this today. Like so, tested so far with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we no longer ICE with the unreduced testcase in the PR. Does the following look OK to commit after a full bootstrap and regtest? -- >8 -- Subject: [PATCH] c++: Parameter pack in requires parameter list [PR94808] When printing the substituted parameter list of a requires-expression as part of the "in requirements with ..." context line during concepts diagnostics, we weren't considering that substitution into a parameter pack can yield zero or multiple parameters. This patch changes the way we print the parameter list of a requires-expression in print_requires_expression_info. We now print the dependent form of the parameter list (along with its template parameter mapping) instead of printing its substituted form. Besides being an improvement in its own, this also sidesteps the above parameter pack expansion issue altogether. Tested with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we longer ICE with the unreduced testcase in the PR. Does this look OK to commit after a bootstrap and regtest? gcc/cp/ChangeLog: PR c++/94808 * error.c (print_requires_expression_info): Print the dependent form of the parameter list with its template parameter mapping, rather than printing the substituted form. gcc/testsuite/ChangeLog: PR c++/94808 * g++.dg/concepts/diagnostic12.C: New test. * g++.dg/concepts/diagnostic5.C: Adjust expected diagnostic. --- gcc/cp/error.c | 16 ++++------------ gcc/testsuite/g++.dg/concepts/diagnostic12.C | 16 ++++++++++++++++ gcc/testsuite/g++.dg/concepts/diagnostic5.C | 4 ++-- 3 files changed, 22 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C diff --git a/gcc/cp/error.c b/gcc/cp/error.c index 98c163db572..46970f9b699 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -3746,7 +3746,6 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE); if (map == error_mark_node) return; - args = get_mapped_args (map); print_location (context, cp_expr_loc_or_input_loc (expr)); pp_verbatim (context->printer, "in requirements "); @@ -3756,19 +3755,12 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a pp_verbatim (context->printer, "with "); while (parms) { - tree next = TREE_CHAIN (parms); - - TREE_CHAIN (parms) = NULL_TREE; - cp_unevaluated u; - tree p = tsubst (parms, args, tf_none, NULL_TREE); - pp_verbatim (context->printer, "%q#D", p); - TREE_CHAIN (parms) = next; - - if (next) + pp_verbatim (context->printer, "%q#D", parms); + if (TREE_CHAIN (parms)) pp_separate_with_comma ((cxx_pretty_printer *)context->printer); - - parms = next; + parms = TREE_CHAIN (parms); } + pp_cxx_parameter_mapping ((cxx_pretty_printer *)context->printer, map); pp_verbatim (context->printer, "\n"); } diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C b/gcc/testsuite/g++.dg/concepts/diagnostic12.C new file mode 100644 index 00000000000..a757342f754 --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C @@ -0,0 +1,16 @@ +// PR c++/94808 +// { dg-do compile { target concepts } } + +template<typename T, typename... Args> + concept c1 = requires (T t, Args... args) { *t; }; +// { dg-message "in requirements with .T t., .Args ... args. .with.* Args = \{\}" "" { target *-*-* } .-1 } + +static_assert(c1<int>); // { dg-error "failed" } + +void f(...); + +template<typename... Args> + concept c2 = requires (Args... args) { f(*args...); }; +// { dg-message "in requirements with .Args ... args. .with Args = \{int, char\}" "" { target *-*-* } .-1 } + +static_assert(c2<int, char>); // { dg-error "failed" } diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C b/gcc/testsuite/g++.dg/concepts/diagnostic5.C index 0d890d6f548..81705f6a0c6 100644 --- a/gcc/testsuite/g++.dg/concepts/diagnostic5.C +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C @@ -9,7 +9,7 @@ template<typename T> template<typename T> concept c2 = requires (T x) { *x; }; // { dg-message "satisfaction of .c2<T>. .with T = char." "" { target *-*-* } .-1 } -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "in requirements with .T x. .with T = char." "" { target *-*-* } .-2 } // { dg-message "required expression .* is invalid" "" { target *-*-* } .-3 } template<typename T> @@ -25,7 +25,7 @@ template<typename T> template<typename T> concept c5 = requires (T x) { { &x } -> c1; }; // { dg-message "satisfaction of .c5<T>. .with T = char." "" { target *-*-* } .-1 } -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } +// { dg-message "in requirements with .T x. .with T = char." "" { target *-*-* } .-2 } template<typename T> requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message "49: no operand" }
On 4/28/20 1:41 PM, Patrick Palka wrote: > On Tue, 28 Apr 2020, Patrick Palka wrote: > >> On Tue, 28 Apr 2020, Jason Merrill wrote: >>> On 4/28/20 9:48 AM, Patrick Palka wrote: >>>> When printing the substituted parameter list of a requires-expression as >>>> part of the "in requirements with ..." context line during concepts >>>> diagnostics, we weren't considering that substitution into a parameter >>>> pack can yield zero or multiple parameters. >>>> >>>> Since this patch affects only concepts diagnostics, so far I tested with >>>> RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer ICE >>>> with the unreduced testcase in the PR. Is this OK to commit after a >>>> full bootstrap and regtest? >>> >>> OK. >> >> Thanks for the review. >> >>> >>> Though I wonder about printing the dependent form and template arguments >>> instead. Do you have an opinion? >> >> I was going to say that on the one hand, it's convenient to have the >> substituted form printed since it would let us to see through >> complicated dependent type aliases, but it seems we don't strip type >> aliases when pretty printing a parameter and its type with '%q#D' >> anyway. And I can't think of any other possible advantage of printing >> the substituted form. >> >> So IMHO printing the dependent form and template arguments instead would >> be better here. I'll try to write a patch for this today. > > Like so, tested so far with RUNTESTFLAGS="dg.exp=*concepts*" and also > verified we no longer ICE with the unreduced testcase in the PR. Does > the following look OK to commit after a full bootstrap and regtest? > > -- >8 -- > > Subject: [PATCH] c++: Parameter pack in requires parameter list [PR94808] > > When printing the substituted parameter list of a requires-expression as > part of the "in requirements with ..." context line during concepts > diagnostics, we weren't considering that substitution into a parameter > pack can yield zero or multiple parameters. > > This patch changes the way we print the parameter list of a > requires-expression in print_requires_expression_info. We now print the > dependent form of the parameter list (along with its template parameter > mapping) instead of printing its substituted form. Besides being an > improvement in its own, this also sidesteps the above parameter pack > expansion issue altogether. > > Tested with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we longer > ICE with the unreduced testcase in the PR. Does this look OK to commit > after a bootstrap and regtest? > > gcc/cp/ChangeLog: > > PR c++/94808 > * error.c (print_requires_expression_info): Print the dependent > form of the parameter list with its template parameter mapping, > rather than printing the substituted form. > > gcc/testsuite/ChangeLog: > > PR c++/94808 > * g++.dg/concepts/diagnostic12.C: New test. > * g++.dg/concepts/diagnostic5.C: Adjust expected diagnostic. > --- > gcc/cp/error.c | 16 ++++------------ > gcc/testsuite/g++.dg/concepts/diagnostic12.C | 16 ++++++++++++++++ > gcc/testsuite/g++.dg/concepts/diagnostic5.C | 4 ++-- > 3 files changed, 22 insertions(+), 14 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C > > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > index 98c163db572..46970f9b699 100644 > --- a/gcc/cp/error.c > +++ b/gcc/cp/error.c > @@ -3746,7 +3746,6 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a > map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE); > if (map == error_mark_node) > return; > - args = get_mapped_args (map); > > print_location (context, cp_expr_loc_or_input_loc (expr)); > pp_verbatim (context->printer, "in requirements "); > @@ -3756,19 +3755,12 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a > pp_verbatim (context->printer, "with "); > while (parms) > { > - tree next = TREE_CHAIN (parms); > - > - TREE_CHAIN (parms) = NULL_TREE; > - cp_unevaluated u; > - tree p = tsubst (parms, args, tf_none, NULL_TREE); > - pp_verbatim (context->printer, "%q#D", p); > - TREE_CHAIN (parms) = next; > - > - if (next) > + pp_verbatim (context->printer, "%q#D", parms); > + if (TREE_CHAIN (parms)) > pp_separate_with_comma ((cxx_pretty_printer *)context->printer); > - > - parms = next; > + parms = TREE_CHAIN (parms); > } > + pp_cxx_parameter_mapping ((cxx_pretty_printer *)context->printer, map); > > pp_verbatim (context->printer, "\n"); > } > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > new file mode 100644 > index 00000000000..a757342f754 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > @@ -0,0 +1,16 @@ > +// PR c++/94808 > +// { dg-do compile { target concepts } } > + > +template<typename T, typename... Args> > + concept c1 = requires (T t, Args... args) { *t; }; > +// { dg-message "in requirements with .T t., .Args ... args. .with.* Args = \{\}" "" { target *-*-* } .-1 } Doesn't this print a binding for T? > + > +static_assert(c1<int>); // { dg-error "failed" } > + > +void f(...); > + > +template<typename... Args> > + concept c2 = requires (Args... args) { f(*args...); }; > +// { dg-message "in requirements with .Args ... args. .with Args = \{int, char\}" "" { target *-*-* } .-1 } > + > +static_assert(c2<int, char>); // { dg-error "failed" } > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > index 0d890d6f548..81705f6a0c6 100644 > --- a/gcc/testsuite/g++.dg/concepts/diagnostic5.C > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > @@ -9,7 +9,7 @@ template<typename T> > template<typename T> > concept c2 = requires (T x) { *x; }; > // { dg-message "satisfaction of .c2<T>. .with T = char." "" { target *-*-* } .-1 } > -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > +// { dg-message "in requirements with .T x. .with T = char." "" { target *-*-* } .-2 } > // { dg-message "required expression .* is invalid" "" { target *-*-* } .-3 } > > template<typename T> > @@ -25,7 +25,7 @@ template<typename T> > template<typename T> > concept c5 = requires (T x) { { &x } -> c1; }; > // { dg-message "satisfaction of .c5<T>. .with T = char." "" { target *-*-* } .-1 } > -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > +// { dg-message "in requirements with .T x. .with T = char." "" { target *-*-* } .-2 } > > template<typename T> > requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message "49: no operand" } >
On Tue, 28 Apr 2020, Jason Merrill wrote: > On 4/28/20 1:41 PM, Patrick Palka wrote: > > On Tue, 28 Apr 2020, Patrick Palka wrote: > > > > > On Tue, 28 Apr 2020, Jason Merrill wrote: > > > > On 4/28/20 9:48 AM, Patrick Palka wrote: > > > > > When printing the substituted parameter list of a requires-expression > > > > > as > > > > > part of the "in requirements with ..." context line during concepts > > > > > diagnostics, we weren't considering that substitution into a parameter > > > > > pack can yield zero or multiple parameters. > > > > > > > > > > Since this patch affects only concepts diagnostics, so far I tested > > > > > with > > > > > RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer > > > > > ICE > > > > > with the unreduced testcase in the PR. Is this OK to commit after a > > > > > full bootstrap and regtest? > > > > > > > > OK. > > > > > > Thanks for the review. > > > > > > > > > > > Though I wonder about printing the dependent form and template arguments > > > > instead. Do you have an opinion? > > > > > > I was going to say that on the one hand, it's convenient to have the > > > substituted form printed since it would let us to see through > > > complicated dependent type aliases, but it seems we don't strip type > > > aliases when pretty printing a parameter and its type with '%q#D' > > > anyway. And I can't think of any other possible advantage of printing > > > the substituted form. > > > > > > So IMHO printing the dependent form and template arguments instead would > > > be better here. I'll try to write a patch for this today. > > > > Like so, tested so far with RUNTESTFLAGS="dg.exp=*concepts*" and also > > verified we no longer ICE with the unreduced testcase in the PR. Does > > the following look OK to commit after a full bootstrap and regtest? > > > > -- >8 -- > > > > Subject: [PATCH] c++: Parameter pack in requires parameter list [PR94808] > > > > When printing the substituted parameter list of a requires-expression as > > part of the "in requirements with ..." context line during concepts > > diagnostics, we weren't considering that substitution into a parameter > > pack can yield zero or multiple parameters. > > > > This patch changes the way we print the parameter list of a > > requires-expression in print_requires_expression_info. We now print the > > dependent form of the parameter list (along with its template parameter > > mapping) instead of printing its substituted form. Besides being an > > improvement in its own, this also sidesteps the above parameter pack > > expansion issue altogether. > > > > Tested with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we longer > > ICE with the unreduced testcase in the PR. Does this look OK to commit > > after a bootstrap and regtest? > > > > gcc/cp/ChangeLog: > > > > PR c++/94808 > > * error.c (print_requires_expression_info): Print the dependent > > form of the parameter list with its template parameter mapping, > > rather than printing the substituted form. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/94808 > > * g++.dg/concepts/diagnostic12.C: New test. > > * g++.dg/concepts/diagnostic5.C: Adjust expected diagnostic. > > --- > > gcc/cp/error.c | 16 ++++------------ > > gcc/testsuite/g++.dg/concepts/diagnostic12.C | 16 ++++++++++++++++ > > gcc/testsuite/g++.dg/concepts/diagnostic5.C | 4 ++-- > > 3 files changed, 22 insertions(+), 14 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > > index 98c163db572..46970f9b699 100644 > > --- a/gcc/cp/error.c > > +++ b/gcc/cp/error.c > > @@ -3746,7 +3746,6 @@ print_requires_expression_info (diagnostic_context > > *context, tree constr, tree a > > map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE); > > if (map == error_mark_node) > > return; > > - args = get_mapped_args (map); > > print_location (context, cp_expr_loc_or_input_loc (expr)); > > pp_verbatim (context->printer, "in requirements "); > > @@ -3756,19 +3755,12 @@ print_requires_expression_info (diagnostic_context > > *context, tree constr, tree a > > pp_verbatim (context->printer, "with "); > > while (parms) > > { > > - tree next = TREE_CHAIN (parms); > > - > > - TREE_CHAIN (parms) = NULL_TREE; > > - cp_unevaluated u; > > - tree p = tsubst (parms, args, tf_none, NULL_TREE); > > - pp_verbatim (context->printer, "%q#D", p); > > - TREE_CHAIN (parms) = next; > > - > > - if (next) > > + pp_verbatim (context->printer, "%q#D", parms); > > + if (TREE_CHAIN (parms)) > > pp_separate_with_comma ((cxx_pretty_printer *)context->printer); > > - > > - parms = next; > > + parms = TREE_CHAIN (parms); > > } > > + pp_cxx_parameter_mapping ((cxx_pretty_printer *)context->printer, map); > > pp_verbatim (context->printer, "\n"); > > } > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > new file mode 100644 > > index 00000000000..a757342f754 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > @@ -0,0 +1,16 @@ > > +// PR c++/94808 > > +// { dg-do compile { target concepts } } > > + > > +template<typename T, typename... Args> > > + concept c1 = requires (T t, Args... args) { *t; }; > > +// { dg-message "in requirements with .T t., .Args ... args. .with.* Args = > > \{\}" "" { target *-*-* } .-1 } > > Doesn't this print a binding for T? Yes, but the order in which the binding for T and the binding for Args is printed appears to be nondeterministic, so I lazily opted to check for just one of the bindings. This nondeterminism stems from how find_template_parameters is implemented, which uses a hash_set<tree> to keep track of seen template parameters and then builds a TREE_LIST of mappings according to the (nondeterministic) order of entries in the table. I think it is harmless semantically but it does affect diagnostics such as these. > > > + > > +static_assert(c1<int>); // { dg-error "failed" } > > + > > +void f(...); > > + > > +template<typename... Args> > > + concept c2 = requires (Args... args) { f(*args...); }; > > +// { dg-message "in requirements with .Args ... args. .with Args = \{int, > > char\}" "" { target *-*-* } .-1 } > > + > > +static_assert(c2<int, char>); // { dg-error "failed" } > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > index 0d890d6f548..81705f6a0c6 100644 > > --- a/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > @@ -9,7 +9,7 @@ template<typename T> > > template<typename T> > > concept c2 = requires (T x) { *x; }; > > // { dg-message "satisfaction of .c2<T>. .with T = char." "" { target > > *-*-* } .-1 } > > -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > > +// { dg-message "in requirements with .T x. .with T = char." "" { target > > *-*-* } .-2 } > > // { dg-message "required expression .* is invalid" "" { target *-*-* } > > .-3 } > > template<typename T> > > @@ -25,7 +25,7 @@ template<typename T> > > template<typename T> > > concept c5 = requires (T x) { { &x } -> c1; }; > > // { dg-message "satisfaction of .c5<T>. .with T = char." "" { target > > *-*-* } .-1 } > > -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > > +// { dg-message "in requirements with .T x. .with T = char." "" { target > > *-*-* } .-2 } > > template<typename T> > > requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message > > "49: no operand" } > > > >
On Tue, 28 Apr 2020, Patrick Palka wrote: > On Tue, 28 Apr 2020, Jason Merrill wrote: > > > On 4/28/20 1:41 PM, Patrick Palka wrote: > > > On Tue, 28 Apr 2020, Patrick Palka wrote: > > > > > > > On Tue, 28 Apr 2020, Jason Merrill wrote: > > > > > On 4/28/20 9:48 AM, Patrick Palka wrote: > > > > > > When printing the substituted parameter list of a requires-expression > > > > > > as > > > > > > part of the "in requirements with ..." context line during concepts > > > > > > diagnostics, we weren't considering that substitution into a parameter > > > > > > pack can yield zero or multiple parameters. > > > > > > > > > > > > Since this patch affects only concepts diagnostics, so far I tested > > > > > > with > > > > > > RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer > > > > > > ICE > > > > > > with the unreduced testcase in the PR. Is this OK to commit after a > > > > > > full bootstrap and regtest? > > > > > > > > > > OK. > > > > > > > > Thanks for the review. > > > > > > > > > > > > > > Though I wonder about printing the dependent form and template arguments > > > > > instead. Do you have an opinion? > > > > > > > > I was going to say that on the one hand, it's convenient to have the > > > > substituted form printed since it would let us to see through > > > > complicated dependent type aliases, but it seems we don't strip type > > > > aliases when pretty printing a parameter and its type with '%q#D' > > > > anyway. And I can't think of any other possible advantage of printing > > > > the substituted form. > > > > > > > > So IMHO printing the dependent form and template arguments instead would > > > > be better here. I'll try to write a patch for this today. > > > > > > Like so, tested so far with RUNTESTFLAGS="dg.exp=*concepts*" and also > > > verified we no longer ICE with the unreduced testcase in the PR. Does > > > the following look OK to commit after a full bootstrap and regtest? > > > > > > -- >8 -- > > > > > > Subject: [PATCH] c++: Parameter pack in requires parameter list [PR94808] > > > > > > When printing the substituted parameter list of a requires-expression as > > > part of the "in requirements with ..." context line during concepts > > > diagnostics, we weren't considering that substitution into a parameter > > > pack can yield zero or multiple parameters. > > > > > > This patch changes the way we print the parameter list of a > > > requires-expression in print_requires_expression_info. We now print the > > > dependent form of the parameter list (along with its template parameter > > > mapping) instead of printing its substituted form. Besides being an > > > improvement in its own, this also sidesteps the above parameter pack > > > expansion issue altogether. > > > > > > Tested with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we longer > > > ICE with the unreduced testcase in the PR. Does this look OK to commit > > > after a bootstrap and regtest? > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/94808 > > > * error.c (print_requires_expression_info): Print the dependent > > > form of the parameter list with its template parameter mapping, > > > rather than printing the substituted form. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/94808 > > > * g++.dg/concepts/diagnostic12.C: New test. > > > * g++.dg/concepts/diagnostic5.C: Adjust expected diagnostic. > > > --- > > > gcc/cp/error.c | 16 ++++------------ > > > gcc/testsuite/g++.dg/concepts/diagnostic12.C | 16 ++++++++++++++++ > > > gcc/testsuite/g++.dg/concepts/diagnostic5.C | 4 ++-- > > > 3 files changed, 22 insertions(+), 14 deletions(-) > > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > > > > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > > > index 98c163db572..46970f9b699 100644 > > > --- a/gcc/cp/error.c > > > +++ b/gcc/cp/error.c > > > @@ -3746,7 +3746,6 @@ print_requires_expression_info (diagnostic_context > > > *context, tree constr, tree a > > > map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE); > > > if (map == error_mark_node) > > > return; > > > - args = get_mapped_args (map); > > > print_location (context, cp_expr_loc_or_input_loc (expr)); > > > pp_verbatim (context->printer, "in requirements "); > > > @@ -3756,19 +3755,12 @@ print_requires_expression_info (diagnostic_context > > > *context, tree constr, tree a > > > pp_verbatim (context->printer, "with "); > > > while (parms) > > > { > > > - tree next = TREE_CHAIN (parms); > > > - > > > - TREE_CHAIN (parms) = NULL_TREE; > > > - cp_unevaluated u; > > > - tree p = tsubst (parms, args, tf_none, NULL_TREE); > > > - pp_verbatim (context->printer, "%q#D", p); > > > - TREE_CHAIN (parms) = next; > > > - > > > - if (next) > > > + pp_verbatim (context->printer, "%q#D", parms); > > > + if (TREE_CHAIN (parms)) > > > pp_separate_with_comma ((cxx_pretty_printer *)context->printer); > > > - > > > - parms = next; > > > + parms = TREE_CHAIN (parms); > > > } > > > + pp_cxx_parameter_mapping ((cxx_pretty_printer *)context->printer, map); > > > pp_verbatim (context->printer, "\n"); > > > } > > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > new file mode 100644 > > > index 00000000000..a757342f754 > > > --- /dev/null > > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > @@ -0,0 +1,16 @@ > > > +// PR c++/94808 > > > +// { dg-do compile { target concepts } } > > > + > > > +template<typename T, typename... Args> > > > + concept c1 = requires (T t, Args... args) { *t; }; > > > +// { dg-message "in requirements with .T t., .Args ... args. .with.* Args = > > > \{\}" "" { target *-*-* } .-1 } > > > > Doesn't this print a binding for T? > > Yes, but the order in which the binding for T and the binding for Args > is printed appears to be nondeterministic, so I lazily opted to check > for just one of the bindings. > > This nondeterminism stems from how find_template_parameters is > implemented, which uses a hash_set<tree> to keep track of seen template > parameters and then builds a TREE_LIST of mappings according to the > (nondeterministic) order of entries in the table. I think it is > harmless semantically but it does affect diagnostics such as these. I've just created PR94830 for this issue. > > > > > > + > > > +static_assert(c1<int>); // { dg-error "failed" } > > > + > > > +void f(...); > > > + > > > +template<typename... Args> > > > + concept c2 = requires (Args... args) { f(*args...); }; > > > +// { dg-message "in requirements with .Args ... args. .with Args = \{int, > > > char\}" "" { target *-*-* } .-1 } > > > + > > > +static_assert(c2<int, char>); // { dg-error "failed" } > > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > > b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > > index 0d890d6f548..81705f6a0c6 100644 > > > --- a/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C > > > @@ -9,7 +9,7 @@ template<typename T> > > > template<typename T> > > > concept c2 = requires (T x) { *x; }; > > > // { dg-message "satisfaction of .c2<T>. .with T = char." "" { target > > > *-*-* } .-1 } > > > -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > > > +// { dg-message "in requirements with .T x. .with T = char." "" { target > > > *-*-* } .-2 } > > > // { dg-message "required expression .* is invalid" "" { target *-*-* } > > > .-3 } > > > template<typename T> > > > @@ -25,7 +25,7 @@ template<typename T> > > > template<typename T> > > > concept c5 = requires (T x) { { &x } -> c1; }; > > > // { dg-message "satisfaction of .c5<T>. .with T = char." "" { target > > > *-*-* } .-1 } > > > -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } > > > +// { dg-message "in requirements with .T x. .with T = char." "" { target > > > *-*-* } .-2 } > > > template<typename T> > > > requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message > > > "49: no operand" } > > > > > > > >
On 4/28/20 3:19 PM, Patrick Palka wrote: > On Tue, 28 Apr 2020, Patrick Palka wrote: > >> On Tue, 28 Apr 2020, Jason Merrill wrote: >> >>> On 4/28/20 1:41 PM, Patrick Palka wrote: >>>> On Tue, 28 Apr 2020, Patrick Palka wrote: >>>> >>>>> On Tue, 28 Apr 2020, Jason Merrill wrote: >>>>>> On 4/28/20 9:48 AM, Patrick Palka wrote: >>>>>>> When printing the substituted parameter list of a requires-expression >>>>>>> as >>>>>>> part of the "in requirements with ..." context line during concepts >>>>>>> diagnostics, we weren't considering that substitution into a parameter >>>>>>> pack can yield zero or multiple parameters. >>>>>>> >>>>>>> Since this patch affects only concepts diagnostics, so far I tested >>>>>>> with >>>>>>> RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no longer >>>>>>> ICE >>>>>>> with the unreduced testcase in the PR. Is this OK to commit after a >>>>>>> full bootstrap and regtest? >>>>>> >>>>>> OK. >>>>> >>>>> Thanks for the review. >>>>> >>>>>> >>>>>> Though I wonder about printing the dependent form and template arguments >>>>>> instead. Do you have an opinion? >>>>> >>>>> I was going to say that on the one hand, it's convenient to have the >>>>> substituted form printed since it would let us to see through >>>>> complicated dependent type aliases, but it seems we don't strip type >>>>> aliases when pretty printing a parameter and its type with '%q#D' >>>>> anyway. And I can't think of any other possible advantage of printing >>>>> the substituted form. >>>>> >>>>> So IMHO printing the dependent form and template arguments instead would >>>>> be better here. I'll try to write a patch for this today. >>>> >>>> Like so, tested so far with RUNTESTFLAGS="dg.exp=*concepts*" and also >>>> verified we no longer ICE with the unreduced testcase in the PR. Does >>>> the following look OK to commit after a full bootstrap and regtest? >>>> >>>> -- >8 -- >>>> >>>> Subject: [PATCH] c++: Parameter pack in requires parameter list [PR94808] >>>> >>>> When printing the substituted parameter list of a requires-expression as >>>> part of the "in requirements with ..." context line during concepts >>>> diagnostics, we weren't considering that substitution into a parameter >>>> pack can yield zero or multiple parameters. >>>> >>>> This patch changes the way we print the parameter list of a >>>> requires-expression in print_requires_expression_info. We now print the >>>> dependent form of the parameter list (along with its template parameter >>>> mapping) instead of printing its substituted form. Besides being an >>>> improvement in its own, this also sidesteps the above parameter pack >>>> expansion issue altogether. >>>> >>>> Tested with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we longer >>>> ICE with the unreduced testcase in the PR. Does this look OK to commit >>>> after a bootstrap and regtest? >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/94808 >>>> * error.c (print_requires_expression_info): Print the dependent >>>> form of the parameter list with its template parameter mapping, >>>> rather than printing the substituted form. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/94808 >>>> * g++.dg/concepts/diagnostic12.C: New test. >>>> * g++.dg/concepts/diagnostic5.C: Adjust expected diagnostic. >>>> --- >>>> gcc/cp/error.c | 16 ++++------------ >>>> gcc/testsuite/g++.dg/concepts/diagnostic12.C | 16 ++++++++++++++++ >>>> gcc/testsuite/g++.dg/concepts/diagnostic5.C | 4 ++-- >>>> 3 files changed, 22 insertions(+), 14 deletions(-) >>>> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>> >>>> diff --git a/gcc/cp/error.c b/gcc/cp/error.c >>>> index 98c163db572..46970f9b699 100644 >>>> --- a/gcc/cp/error.c >>>> +++ b/gcc/cp/error.c >>>> @@ -3746,7 +3746,6 @@ print_requires_expression_info (diagnostic_context >>>> *context, tree constr, tree a >>>> map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE); >>>> if (map == error_mark_node) >>>> return; >>>> - args = get_mapped_args (map); >>>> print_location (context, cp_expr_loc_or_input_loc (expr)); >>>> pp_verbatim (context->printer, "in requirements "); >>>> @@ -3756,19 +3755,12 @@ print_requires_expression_info (diagnostic_context >>>> *context, tree constr, tree a >>>> pp_verbatim (context->printer, "with "); >>>> while (parms) >>>> { >>>> - tree next = TREE_CHAIN (parms); >>>> - >>>> - TREE_CHAIN (parms) = NULL_TREE; >>>> - cp_unevaluated u; >>>> - tree p = tsubst (parms, args, tf_none, NULL_TREE); >>>> - pp_verbatim (context->printer, "%q#D", p); >>>> - TREE_CHAIN (parms) = next; >>>> - >>>> - if (next) >>>> + pp_verbatim (context->printer, "%q#D", parms); >>>> + if (TREE_CHAIN (parms)) >>>> pp_separate_with_comma ((cxx_pretty_printer *)context->printer); >>>> - >>>> - parms = next; >>>> + parms = TREE_CHAIN (parms); >>>> } >>>> + pp_cxx_parameter_mapping ((cxx_pretty_printer *)context->printer, map); >>>> pp_verbatim (context->printer, "\n"); >>>> } >>>> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>> b/gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>> new file mode 100644 >>>> index 00000000000..a757342f754 >>>> --- /dev/null >>>> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>> @@ -0,0 +1,16 @@ >>>> +// PR c++/94808 >>>> +// { dg-do compile { target concepts } } >>>> + >>>> +template<typename T, typename... Args> >>>> + concept c1 = requires (T t, Args... args) { *t; }; >>>> +// { dg-message "in requirements with .T t., .Args ... args. .with.* Args = >>>> \{\}" "" { target *-*-* } .-1 } >>> >>> Doesn't this print a binding for T? >> >> Yes, but the order in which the binding for T and the binding for Args >> is printed appears to be nondeterministic, so I lazily opted to check >> for just one of the bindings. >> >> This nondeterminism stems from how find_template_parameters is >> implemented, which uses a hash_set<tree> to keep track of seen template >> parameters and then builds a TREE_LIST of mappings according to the >> (nondeterministic) order of entries in the table. I think it is >> harmless semantically but it does affect diagnostics such as these. > > I've just created PR94830 for this issue. Thanks. I'd think we should build up the list in find_template_parameter_info and only use the hash_set to avoid adding the same parameter more than once. This patch is OK. >> >>> >>>> + >>>> +static_assert(c1<int>); // { dg-error "failed" } >>>> + >>>> +void f(...); >>>> + >>>> +template<typename... Args> >>>> + concept c2 = requires (Args... args) { f(*args...); }; >>>> +// { dg-message "in requirements with .Args ... args. .with Args = \{int, >>>> char\}" "" { target *-*-* } .-1 } >>>> + >>>> +static_assert(c2<int, char>); // { dg-error "failed" } >>>> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic5.C >>>> b/gcc/testsuite/g++.dg/concepts/diagnostic5.C >>>> index 0d890d6f548..81705f6a0c6 100644 >>>> --- a/gcc/testsuite/g++.dg/concepts/diagnostic5.C >>>> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic5.C >>>> @@ -9,7 +9,7 @@ template<typename T> >>>> template<typename T> >>>> concept c2 = requires (T x) { *x; }; >>>> // { dg-message "satisfaction of .c2<T>. .with T = char." "" { target >>>> *-*-* } .-1 } >>>> -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } >>>> +// { dg-message "in requirements with .T x. .with T = char." "" { target >>>> *-*-* } .-2 } >>>> // { dg-message "required expression .* is invalid" "" { target *-*-* } >>>> .-3 } >>>> template<typename T> >>>> @@ -25,7 +25,7 @@ template<typename T> >>>> template<typename T> >>>> concept c5 = requires (T x) { { &x } -> c1; }; >>>> // { dg-message "satisfaction of .c5<T>. .with T = char." "" { target >>>> *-*-* } .-1 } >>>> -// { dg-message "in requirements with .char x." "" { target *-*-* } .-2 } >>>> +// { dg-message "in requirements with .T x. .with T = char." "" { target >>>> *-*-* } .-2 } >>>> template<typename T> >>>> requires (c1<T> || c2<T>) || (c3<T> || c4<T>) || c5<T> // { dg-message >>>> "49: no operand" } >>>> >>> >>> >> >
On Tue, 28 Apr 2020, Jason Merrill wrote: > On 4/28/20 3:19 PM, Patrick Palka wrote: > > On Tue, 28 Apr 2020, Patrick Palka wrote: > > > > > On Tue, 28 Apr 2020, Jason Merrill wrote: > > > > > > > On 4/28/20 1:41 PM, Patrick Palka wrote: > > > > > On Tue, 28 Apr 2020, Patrick Palka wrote: > > > > > > > > > > > On Tue, 28 Apr 2020, Jason Merrill wrote: > > > > > > > On 4/28/20 9:48 AM, Patrick Palka wrote: > > > > > > > > When printing the substituted parameter list of a > > > > > > > > requires-expression > > > > > > > > as > > > > > > > > part of the "in requirements with ..." context line during > > > > > > > > concepts > > > > > > > > diagnostics, we weren't considering that substitution into a > > > > > > > > parameter > > > > > > > > pack can yield zero or multiple parameters. > > > > > > > > > > > > > > > > Since this patch affects only concepts diagnostics, so far I > > > > > > > > tested > > > > > > > > with > > > > > > > > RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no > > > > > > > > longer > > > > > > > > ICE > > > > > > > > with the unreduced testcase in the PR. Is this OK to commit > > > > > > > > after a > > > > > > > > full bootstrap and regtest? > > > > > > > > > > > > > > OK. > > > > > > > > > > > > Thanks for the review. > > > > > > > > > > > > > > > > > > > > Though I wonder about printing the dependent form and template > > > > > > > arguments > > > > > > > instead. Do you have an opinion? > > > > > > > > > > > > I was going to say that on the one hand, it's convenient to have the > > > > > > substituted form printed since it would let us to see through > > > > > > complicated dependent type aliases, but it seems we don't strip type > > > > > > aliases when pretty printing a parameter and its type with '%q#D' > > > > > > anyway. And I can't think of any other possible advantage of > > > > > > printing > > > > > > the substituted form. > > > > > > > > > > > > So IMHO printing the dependent form and template arguments instead > > > > > > would > > > > > > be better here. I'll try to write a patch for this today. > > > > > > > > > > Like so, tested so far with RUNTESTFLAGS="dg.exp=*concepts*" and also > > > > > verified we no longer ICE with the unreduced testcase in the PR. Does > > > > > the following look OK to commit after a full bootstrap and regtest? > > > > > > > > > > -- >8 -- > > > > > > > > > > Subject: [PATCH] c++: Parameter pack in requires parameter list > > > > > [PR94808] > > > > > > > > > > When printing the substituted parameter list of a requires-expression > > > > > as > > > > > part of the "in requirements with ..." context line during concepts > > > > > diagnostics, we weren't considering that substitution into a parameter > > > > > pack can yield zero or multiple parameters. > > > > > > > > > > This patch changes the way we print the parameter list of a > > > > > requires-expression in print_requires_expression_info. We now print > > > > > the > > > > > dependent form of the parameter list (along with its template > > > > > parameter > > > > > mapping) instead of printing its substituted form. Besides being an > > > > > improvement in its own, this also sidesteps the above parameter pack > > > > > expansion issue altogether. > > > > > > > > > > Tested with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we > > > > > longer > > > > > ICE with the unreduced testcase in the PR. Does this look OK to > > > > > commit > > > > > after a bootstrap and regtest? > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > PR c++/94808 > > > > > * error.c (print_requires_expression_info): Print the > > > > > dependent > > > > > form of the parameter list with its template parameter > > > > > mapping, > > > > > rather than printing the substituted form. > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > PR c++/94808 > > > > > * g++.dg/concepts/diagnostic12.C: New test. > > > > > * g++.dg/concepts/diagnostic5.C: Adjust expected diagnostic. > > > > > --- > > > > > gcc/cp/error.c | 16 ++++------------ > > > > > gcc/testsuite/g++.dg/concepts/diagnostic12.C | 16 ++++++++++++++++ > > > > > gcc/testsuite/g++.dg/concepts/diagnostic5.C | 4 ++-- > > > > > 3 files changed, 22 insertions(+), 14 deletions(-) > > > > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > > > > > > > > diff --git a/gcc/cp/error.c b/gcc/cp/error.c > > > > > index 98c163db572..46970f9b699 100644 > > > > > --- a/gcc/cp/error.c > > > > > +++ b/gcc/cp/error.c > > > > > @@ -3746,7 +3746,6 @@ print_requires_expression_info > > > > > (diagnostic_context > > > > > *context, tree constr, tree a > > > > > map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE); > > > > > if (map == error_mark_node) > > > > > return; > > > > > - args = get_mapped_args (map); > > > > > print_location (context, cp_expr_loc_or_input_loc (expr)); > > > > > pp_verbatim (context->printer, "in requirements "); > > > > > @@ -3756,19 +3755,12 @@ print_requires_expression_info > > > > > (diagnostic_context > > > > > *context, tree constr, tree a > > > > > pp_verbatim (context->printer, "with "); > > > > > while (parms) > > > > > { > > > > > - tree next = TREE_CHAIN (parms); > > > > > - > > > > > - TREE_CHAIN (parms) = NULL_TREE; > > > > > - cp_unevaluated u; > > > > > - tree p = tsubst (parms, args, tf_none, NULL_TREE); > > > > > - pp_verbatim (context->printer, "%q#D", p); > > > > > - TREE_CHAIN (parms) = next; > > > > > - > > > > > - if (next) > > > > > + pp_verbatim (context->printer, "%q#D", parms); > > > > > + if (TREE_CHAIN (parms)) > > > > > pp_separate_with_comma ((cxx_pretty_printer > > > > > *)context->printer); > > > > > - > > > > > - parms = next; > > > > > + parms = TREE_CHAIN (parms); > > > > > } > > > > > + pp_cxx_parameter_mapping ((cxx_pretty_printer *)context->printer, > > > > > map); > > > > > pp_verbatim (context->printer, "\n"); > > > > > } > > > > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > > > b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > > > new file mode 100644 > > > > > index 00000000000..a757342f754 > > > > > --- /dev/null > > > > > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > > > > > @@ -0,0 +1,16 @@ > > > > > +// PR c++/94808 > > > > > +// { dg-do compile { target concepts } } > > > > > + > > > > > +template<typename T, typename... Args> > > > > > + concept c1 = requires (T t, Args... args) { *t; }; > > > > > +// { dg-message "in requirements with .T t., .Args ... args. .with.* > > > > > Args = > > > > > \{\}" "" { target *-*-* } .-1 } > > > > > > > > Doesn't this print a binding for T? > > > > > > Yes, but the order in which the binding for T and the binding for Args > > > is printed appears to be nondeterministic, so I lazily opted to check > > > for just one of the bindings. > > > > > > This nondeterminism stems from how find_template_parameters is > > > implemented, which uses a hash_set<tree> to keep track of seen template > > > parameters and then builds a TREE_LIST of mappings according to the > > > (nondeterministic) order of entries in the table. I think it is > > > harmless semantically but it does affect diagnostics such as these. > > > > I've just created PR94830 for this issue. > > Thanks. I'd think we should build up the list in find_template_parameter_info > and only use the hash_set to avoid adding the same parameter more than once. > > This patch is OK. Nice, building up the list in find_template_parameter_info works pretty well. FWIW, here's a patch for this: -- >8 -- Subject: [PATCH] c++: Nondeterministic concepts diagnostics [PR94830] This patch makes the order in which template parameters appear in the TREE_LIST returned by find_template_parameters deterministic between runs. The current nondeterminism is semantically harmless, but it has the undesirable effect of causing some concepts diagnostics which print a constraint's parameter mapping via pp_cxx_parameter_mapping to also be nondeterministic, as in the testcases below. Passes 'make check-c++', does this look OK to commit after a full bootstrap and regtest? (I considered reversing the resulting TREE_LIST in find_template_parameters so that it's ordered with respect to when keep_template_parm first sees each parameter, but doing so doesn't always give the natural ordering anyway, since walk_tree_1 visits the the template arguments of a TEMPLATE_ID_EXPR (i.e. the elements of a TREE_VEC) backwards.) gcc/cp/ChangeLog: PR c++/94830 * pt.c (find_template_parameter_info::parm_list): New field. (keep_template_parm): Use the new field to build up the parameter list here instead of ... (find_template_parameters): ... here. Return ftpi.parm_list. gcc/testsuite/ChangeLog: PR c++/94830 * g++.dg/concepts/diagnostics12.C: Clarify the dg-message now that the corresponding diagnostic is deterministic. * g++.dg/concepts/diagnostics13.C: New test. --- gcc/cp/pt.c | 13 ++++++------- gcc/testsuite/g++.dg/concepts/diagnostic12.C | 2 +- gcc/testsuite/g++.dg/concepts/diagnostic13.C | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic13.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index ba22d9ec538..94557a0439e 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -10440,11 +10440,13 @@ struct find_template_parameter_info { explicit find_template_parameter_info (tree ctx_parms) : ctx_parms (ctx_parms), - max_depth (TMPL_PARMS_DEPTH (ctx_parms)) + max_depth (TMPL_PARMS_DEPTH (ctx_parms)), + parm_list (NULL_TREE) {} hash_set<tree> visited; hash_set<tree> parms; + tree parm_list; tree ctx_parms; int max_depth; }; @@ -10476,7 +10478,8 @@ keep_template_parm (tree t, void* data) T and const T. Adjust types to their unqualified versions. */ if (TYPE_P (t)) t = TYPE_MAIN_VARIANT (t); - ftpi->parms.add (t); + if (!ftpi->parms.add (t)) + ftpi->parm_list = tree_cons (NULL_TREE, t, ftpi->parm_list); return 0; } @@ -10587,11 +10590,7 @@ find_template_parameters (tree t, tree ctx_parms) find_template_parameter_info ftpi (ctx_parms); for_each_template_parm (t, keep_template_parm, &ftpi, &ftpi.visited, /*include_nondeduced*/true, any_template_parm_r); - tree list = NULL_TREE; - for (hash_set<tree>::iterator iter = ftpi.parms.begin(); - iter != ftpi.parms.end(); ++iter) - list = tree_cons (NULL_TREE, *iter, list); - return list; + return ftpi.parm_list; } /* Returns true if T depends on any template parameter. */ diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C b/gcc/testsuite/g++.dg/concepts/diagnostic12.C index a757342f754..548ba9c1b3d 100644 --- a/gcc/testsuite/g++.dg/concepts/diagnostic12.C +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C @@ -3,7 +3,7 @@ template<typename T, typename... Args> concept c1 = requires (T t, Args... args) { *t; }; -// { dg-message "in requirements with .T t., .Args ... args. .with.* Args = \{\}" "" { target *-*-* } .-1 } +// { dg-message "in requirements with .T t., .Args ... args. .with Args = \{\}; T = int" "" { target *-*-* } .-1 } static_assert(c1<int>); // { dg-error "failed" } diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic13.C b/gcc/testsuite/g++.dg/concepts/diagnostic13.C new file mode 100644 index 00000000000..accd8a6d2bd --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic13.C @@ -0,0 +1,14 @@ +// PR c++/94830 +// { dg-do compile { target concepts } } + +template<typename T, typename R> + concept c = __is_same(T, R); // { dg-message "with T = int; R = char" } + +template<typename T, typename R> + requires c<T,R> +void foo() { } + +void bar() +{ + foo<int, char>(); // { dg-error "unsatisfied constraints" } +}
On 4/29/20 12:05 AM, Patrick Palka wrote: > On Tue, 28 Apr 2020, Jason Merrill wrote: > >> On 4/28/20 3:19 PM, Patrick Palka wrote: >>> On Tue, 28 Apr 2020, Patrick Palka wrote: >>> >>>> On Tue, 28 Apr 2020, Jason Merrill wrote: >>>> >>>>> On 4/28/20 1:41 PM, Patrick Palka wrote: >>>>>> On Tue, 28 Apr 2020, Patrick Palka wrote: >>>>>> >>>>>>> On Tue, 28 Apr 2020, Jason Merrill wrote: >>>>>>>> On 4/28/20 9:48 AM, Patrick Palka wrote: >>>>>>>>> When printing the substituted parameter list of a >>>>>>>>> requires-expression >>>>>>>>> as >>>>>>>>> part of the "in requirements with ..." context line during >>>>>>>>> concepts >>>>>>>>> diagnostics, we weren't considering that substitution into a >>>>>>>>> parameter >>>>>>>>> pack can yield zero or multiple parameters. >>>>>>>>> >>>>>>>>> Since this patch affects only concepts diagnostics, so far I >>>>>>>>> tested >>>>>>>>> with >>>>>>>>> RUNTESTFLAGS="dg.exp=*concepts*" and also verified that we no >>>>>>>>> longer >>>>>>>>> ICE >>>>>>>>> with the unreduced testcase in the PR. Is this OK to commit >>>>>>>>> after a >>>>>>>>> full bootstrap and regtest? >>>>>>>> >>>>>>>> OK. >>>>>>> >>>>>>> Thanks for the review. >>>>>>> >>>>>>>> >>>>>>>> Though I wonder about printing the dependent form and template >>>>>>>> arguments >>>>>>>> instead. Do you have an opinion? >>>>>>> >>>>>>> I was going to say that on the one hand, it's convenient to have the >>>>>>> substituted form printed since it would let us to see through >>>>>>> complicated dependent type aliases, but it seems we don't strip type >>>>>>> aliases when pretty printing a parameter and its type with '%q#D' >>>>>>> anyway. And I can't think of any other possible advantage of >>>>>>> printing >>>>>>> the substituted form. >>>>>>> >>>>>>> So IMHO printing the dependent form and template arguments instead >>>>>>> would >>>>>>> be better here. I'll try to write a patch for this today. >>>>>> >>>>>> Like so, tested so far with RUNTESTFLAGS="dg.exp=*concepts*" and also >>>>>> verified we no longer ICE with the unreduced testcase in the PR. Does >>>>>> the following look OK to commit after a full bootstrap and regtest? >>>>>> >>>>>> -- >8 -- >>>>>> >>>>>> Subject: [PATCH] c++: Parameter pack in requires parameter list >>>>>> [PR94808] >>>>>> >>>>>> When printing the substituted parameter list of a requires-expression >>>>>> as >>>>>> part of the "in requirements with ..." context line during concepts >>>>>> diagnostics, we weren't considering that substitution into a parameter >>>>>> pack can yield zero or multiple parameters. >>>>>> >>>>>> This patch changes the way we print the parameter list of a >>>>>> requires-expression in print_requires_expression_info. We now print >>>>>> the >>>>>> dependent form of the parameter list (along with its template >>>>>> parameter >>>>>> mapping) instead of printing its substituted form. Besides being an >>>>>> improvement in its own, this also sidesteps the above parameter pack >>>>>> expansion issue altogether. >>>>>> >>>>>> Tested with RUNTESTFLAGS="dg.exp=*concepts*" and also verified we >>>>>> longer >>>>>> ICE with the unreduced testcase in the PR. Does this look OK to >>>>>> commit >>>>>> after a bootstrap and regtest? >>>>>> >>>>>> gcc/cp/ChangeLog: >>>>>> >>>>>> PR c++/94808 >>>>>> * error.c (print_requires_expression_info): Print the >>>>>> dependent >>>>>> form of the parameter list with its template parameter >>>>>> mapping, >>>>>> rather than printing the substituted form. >>>>>> >>>>>> gcc/testsuite/ChangeLog: >>>>>> >>>>>> PR c++/94808 >>>>>> * g++.dg/concepts/diagnostic12.C: New test. >>>>>> * g++.dg/concepts/diagnostic5.C: Adjust expected diagnostic. >>>>>> --- >>>>>> gcc/cp/error.c | 16 ++++------------ >>>>>> gcc/testsuite/g++.dg/concepts/diagnostic12.C | 16 ++++++++++++++++ >>>>>> gcc/testsuite/g++.dg/concepts/diagnostic5.C | 4 ++-- >>>>>> 3 files changed, 22 insertions(+), 14 deletions(-) >>>>>> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>>>> >>>>>> diff --git a/gcc/cp/error.c b/gcc/cp/error.c >>>>>> index 98c163db572..46970f9b699 100644 >>>>>> --- a/gcc/cp/error.c >>>>>> +++ b/gcc/cp/error.c >>>>>> @@ -3746,7 +3746,6 @@ print_requires_expression_info >>>>>> (diagnostic_context >>>>>> *context, tree constr, tree a >>>>>> map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE); >>>>>> if (map == error_mark_node) >>>>>> return; >>>>>> - args = get_mapped_args (map); >>>>>> print_location (context, cp_expr_loc_or_input_loc (expr)); >>>>>> pp_verbatim (context->printer, "in requirements "); >>>>>> @@ -3756,19 +3755,12 @@ print_requires_expression_info >>>>>> (diagnostic_context >>>>>> *context, tree constr, tree a >>>>>> pp_verbatim (context->printer, "with "); >>>>>> while (parms) >>>>>> { >>>>>> - tree next = TREE_CHAIN (parms); >>>>>> - >>>>>> - TREE_CHAIN (parms) = NULL_TREE; >>>>>> - cp_unevaluated u; >>>>>> - tree p = tsubst (parms, args, tf_none, NULL_TREE); >>>>>> - pp_verbatim (context->printer, "%q#D", p); >>>>>> - TREE_CHAIN (parms) = next; >>>>>> - >>>>>> - if (next) >>>>>> + pp_verbatim (context->printer, "%q#D", parms); >>>>>> + if (TREE_CHAIN (parms)) >>>>>> pp_separate_with_comma ((cxx_pretty_printer >>>>>> *)context->printer); >>>>>> - >>>>>> - parms = next; >>>>>> + parms = TREE_CHAIN (parms); >>>>>> } >>>>>> + pp_cxx_parameter_mapping ((cxx_pretty_printer *)context->printer, >>>>>> map); >>>>>> pp_verbatim (context->printer, "\n"); >>>>>> } >>>>>> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>>>> b/gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>>>> new file mode 100644 >>>>>> index 00000000000..a757342f754 >>>>>> --- /dev/null >>>>>> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C >>>>>> @@ -0,0 +1,16 @@ >>>>>> +// PR c++/94808 >>>>>> +// { dg-do compile { target concepts } } >>>>>> + >>>>>> +template<typename T, typename... Args> >>>>>> + concept c1 = requires (T t, Args... args) { *t; }; >>>>>> +// { dg-message "in requirements with .T t., .Args ... args. .with.* >>>>>> Args = >>>>>> \{\}" "" { target *-*-* } .-1 } >>>>> >>>>> Doesn't this print a binding for T? >>>> >>>> Yes, but the order in which the binding for T and the binding for Args >>>> is printed appears to be nondeterministic, so I lazily opted to check >>>> for just one of the bindings. >>>> >>>> This nondeterminism stems from how find_template_parameters is >>>> implemented, which uses a hash_set<tree> to keep track of seen template >>>> parameters and then builds a TREE_LIST of mappings according to the >>>> (nondeterministic) order of entries in the table. I think it is >>>> harmless semantically but it does affect diagnostics such as these. >>> >>> I've just created PR94830 for this issue. >> >> Thanks. I'd think we should build up the list in find_template_parameter_info >> and only use the hash_set to avoid adding the same parameter more than once. >> >> This patch is OK. > > Nice, building up the list in find_template_parameter_info works pretty > well. FWIW, here's a patch for this: OK, thanks. > -- >8 -- > > Subject: [PATCH] c++: Nondeterministic concepts diagnostics [PR94830] > > This patch makes the order in which template parameters appear in the > TREE_LIST returned by find_template_parameters deterministic between > runs. > > The current nondeterminism is semantically harmless, but it has the > undesirable effect of causing some concepts diagnostics which print a > constraint's parameter mapping via pp_cxx_parameter_mapping to also be > nondeterministic, as in the testcases below. > > Passes 'make check-c++', does this look OK to commit after a full > bootstrap and regtest? > > (I considered reversing the resulting TREE_LIST in > find_template_parameters so that it's ordered with respect to when > keep_template_parm first sees each parameter, but doing so doesn't > always give the natural ordering anyway, since walk_tree_1 visits the > the template arguments of a TEMPLATE_ID_EXPR (i.e. the elements of a > TREE_VEC) backwards.) > > gcc/cp/ChangeLog: > > PR c++/94830 > * pt.c (find_template_parameter_info::parm_list): New field. > (keep_template_parm): Use the new field to build up the > parameter list here instead of ... > (find_template_parameters): ... here. Return ftpi.parm_list. > > gcc/testsuite/ChangeLog: > > PR c++/94830 > * g++.dg/concepts/diagnostics12.C: Clarify the dg-message now > that the corresponding diagnostic is deterministic. > * g++.dg/concepts/diagnostics13.C: New test. > --- > gcc/cp/pt.c | 13 ++++++------- > gcc/testsuite/g++.dg/concepts/diagnostic12.C | 2 +- > gcc/testsuite/g++.dg/concepts/diagnostic13.C | 14 ++++++++++++++ > 3 files changed, 21 insertions(+), 8 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic13.C > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index ba22d9ec538..94557a0439e 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -10440,11 +10440,13 @@ struct find_template_parameter_info > { > explicit find_template_parameter_info (tree ctx_parms) > : ctx_parms (ctx_parms), > - max_depth (TMPL_PARMS_DEPTH (ctx_parms)) > + max_depth (TMPL_PARMS_DEPTH (ctx_parms)), > + parm_list (NULL_TREE) > {} > > hash_set<tree> visited; > hash_set<tree> parms; > + tree parm_list; > tree ctx_parms; > int max_depth; > }; > @@ -10476,7 +10478,8 @@ keep_template_parm (tree t, void* data) > T and const T. Adjust types to their unqualified versions. */ > if (TYPE_P (t)) > t = TYPE_MAIN_VARIANT (t); > - ftpi->parms.add (t); > + if (!ftpi->parms.add (t)) > + ftpi->parm_list = tree_cons (NULL_TREE, t, ftpi->parm_list); > > return 0; > } > @@ -10587,11 +10590,7 @@ find_template_parameters (tree t, tree ctx_parms) > find_template_parameter_info ftpi (ctx_parms); > for_each_template_parm (t, keep_template_parm, &ftpi, &ftpi.visited, > /*include_nondeduced*/true, any_template_parm_r); > - tree list = NULL_TREE; > - for (hash_set<tree>::iterator iter = ftpi.parms.begin(); > - iter != ftpi.parms.end(); ++iter) > - list = tree_cons (NULL_TREE, *iter, list); > - return list; > + return ftpi.parm_list; > } > > /* Returns true if T depends on any template parameter. */ > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > index a757342f754..548ba9c1b3d 100644 > --- a/gcc/testsuite/g++.dg/concepts/diagnostic12.C > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C > @@ -3,7 +3,7 @@ > > template<typename T, typename... Args> > concept c1 = requires (T t, Args... args) { *t; }; > -// { dg-message "in requirements with .T t., .Args ... args. .with.* Args = \{\}" "" { target *-*-* } .-1 } > +// { dg-message "in requirements with .T t., .Args ... args. .with Args = \{\}; T = int" "" { target *-*-* } .-1 } > > static_assert(c1<int>); // { dg-error "failed" } > > diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic13.C b/gcc/testsuite/g++.dg/concepts/diagnostic13.C > new file mode 100644 > index 00000000000..accd8a6d2bd > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic13.C > @@ -0,0 +1,14 @@ > +// PR c++/94830 > +// { dg-do compile { target concepts } } > + > +template<typename T, typename R> > + concept c = __is_same(T, R); // { dg-message "with T = int; R = char" } > + > +template<typename T, typename R> > + requires c<T,R> > +void foo() { } > + > +void bar() > +{ > + foo<int, char>(); // { dg-error "unsatisfied constraints" } > +} >
diff --git a/gcc/cp/error.c b/gcc/cp/error.c index 98c163db572..a314412988f 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -3752,8 +3752,7 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a pp_verbatim (context->printer, "in requirements "); tree parms = TREE_OPERAND (expr, 0); - if (parms) - pp_verbatim (context->printer, "with "); + auto_vec<tree> substed_parms; while (parms) { tree next = TREE_CHAIN (parms); @@ -3761,15 +3760,25 @@ print_requires_expression_info (diagnostic_context *context, tree constr, tree a TREE_CHAIN (parms) = NULL_TREE; cp_unevaluated u; tree p = tsubst (parms, args, tf_none, NULL_TREE); - pp_verbatim (context->printer, "%q#D", p); + while (p) + { + substed_parms.safe_push (p); + p = TREE_CHAIN (p); + } TREE_CHAIN (parms) = next; - if (next) - pp_separate_with_comma ((cxx_pretty_printer *)context->printer); - parms = next; } + for (unsigned i = 0; i < substed_parms.length (); i++) + { + if (i == 0) + pp_verbatim (context->printer, "with "); + else + pp_separate_with_comma ((cxx_pretty_printer *)context->printer); + pp_verbatim (context->printer, "%q#D", substed_parms[i]); + } + pp_verbatim (context->printer, "\n"); } diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic12.C b/gcc/testsuite/g++.dg/concepts/diagnostic12.C new file mode 100644 index 00000000000..7dd286291f3 --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic12.C @@ -0,0 +1,14 @@ +// PR c++/94808 +// { dg-do compile { target concepts } } + +template<typename T, typename... Args> + concept c1 = requires (T t, Args... args) { *t; }; +// { dg-message "in requirements with .int t." "" { target *-*-* } .-1 } + +static_assert(c1<int>); // { dg-error "failed" } + +template<typename T, typename... Args> + concept c2 = requires (T t, Args... args) { *t; }; +// { dg-message "in requirements with .int t., .char args#0., .bool args#1." "" { target *-*-* } .-1 } + +static_assert(c2<int, char, bool>); // { dg-error "failed" }