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 }
