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" }
--
2.26.2.266.ge870325ee8
