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" } > > > >