On Wed, 18 Mar 2020, Patrick Palka wrote:
> When diagnosing a constraint error, we currently try to print the constraint
> inside a diagnostic constraint context with its template arguments substituted
> in. If substitution fails, then we instead just print the dependent
> form, as in the the test case below:
>
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: static assertion
> failed
> 14 | static_assert(E<int>); // { dg-error "static assertion failed|not a
> class" }
> | ^~~~~~
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: note: constraints not
> satisfied
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:4:11: required for the
> satisfaction of ‘C<T>’
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:8:11: required for the
> satisfaction of ‘D<typename T::type>’
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: ‘int’ is not a
> class, struct, or union type
>
> But printing just the dependent form sometimes makes it difficult to decipher
> the diagnostic. In the above example, for instance, there's no indication of
> how the template argument 'int' relates to either of the 'T's.
>
> This patch improves the situation by changing these diagnostics to always
> print
> the dependent form of the constraint, and alongside it the (preferably
> substituted) constraint parameter mapping. So with the same test case below
> we
> now get:
>
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: static assertion
> failed
> 14 | static_assert(E<int>); // { dg-error "static assertion failed|not a
> class" }
> | ^~~~~~
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: note: constraints not
> satisfied
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:4:11: required for the
> satisfaction of ‘C<T>’ [with T = typename T::type]
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:8:11: required for the
> satisfaction of ‘D<typename T::type>’ [with T = int]
> gcc/testsuite/g++.dg/concepts/diagnostic6.C:14:15: error: ‘int’ is not a
> class, struct, or union type
>
> This change arguably makes it easier to figure out what's going on whenever a
> constraint fails due to substitution resulting in an invalid type rather than
> failing due to the constraint evaluating to false.
>
> Tested on x86_64-pc-linux-gnu, does this look reasonable? I'm not sure if
> printing an unsubstituted parameter mapping (like in the line 4 message above)
> is always useful, but perhaps it's better than nothing?
>
Ah sorry, this is an old version of the patch. Please consider this one
instead:
-- >8 --
gcc/cp/ChangeLog:
* cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern.
* cxx-pretty-print.h (pp_cxx_parameter_mapping): Declare.
* error.c (rebuild_concept_check): Delete.
(print_concept_check_info): Print the dependent form of the constraint
and the
preferably substituted parameter mapping alongside it.
gcc/testsuite/ChangeLog:
* g++.dg/concepts/diagnostic6.C: New test.
---
gcc/cp/cxx-pretty-print.c | 2 +-
gcc/cp/cxx-pretty-print.h | 1 +
gcc/cp/error.c | 41 ++++++++-------------
gcc/testsuite/g++.dg/concepts/diagnostic6.C | 14 +++++++
4 files changed, 32 insertions(+), 26 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic6.C
diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f..1e89c3f9033 100644
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2878,7 +2878,7 @@ pp_cxx_check_constraint (cxx_pretty_printer *pp, tree t)
/* Output the "[with ...]" clause for a parameter mapping of an atomic
constraint. */
-static void
+void
pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
{
for (tree p = map; p; p = TREE_CHAIN (p))
diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 7c7347f57ba..494f3fdde59 100644
--- a/gcc/cp/cxx-pretty-print.h
+++ b/gcc/cp/cxx-pretty-print.h
@@ -112,5 +112,6 @@ void pp_cxx_conjunction (cxx_pretty_printer *, tree);
void pp_cxx_disjunction (cxx_pretty_printer *, tree);
void pp_cxx_constraint (cxx_pretty_printer *, tree);
void pp_cxx_constrained_type_spec (cxx_pretty_printer *, tree);
+void pp_cxx_parameter_mapping (cxx_pretty_printer *, tree);
#endif /* GCC_CXX_PRETTY_PRINT_H */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index cc51b6ffe13..fc08558e9b6 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3680,27 +3680,6 @@ print_location (diagnostic_context *context, location_t
loc)
"locus", xloc.file, xloc.line);
}
-/* Instantiate the concept check for the purpose of diagnosing an error. */
-
-static tree
-rebuild_concept_check (tree expr, tree map, tree args)
-{
- /* Instantiate the parameter mapping for the template-id. */
- map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
- if (map == error_mark_node)
- return error_mark_node;
- args = get_mapped_args (map);
-
- /* Rebuild the template id using substituted arguments. Substituting
- directly through the expression will trigger recursive satisfaction,
- so don't do that. */
- tree id = unpack_concept_check (expr);
- args = tsubst_template_args (TREE_OPERAND (id, 1), args, tf_none, NULL_TREE);
- if (args == error_mark_node)
- return error_mark_node;
- return build_nt (TEMPLATE_ID_EXPR, TREE_OPERAND (id, 0), args);
-}
-
static void
print_constrained_decl_info (diagnostic_context *context, tree decl)
{
@@ -3717,12 +3696,24 @@ print_concept_check_info (diagnostic_context *context,
tree expr, tree map, tree
tree tmpl = TREE_OPERAND (id, 0);
if (OVL_P (tmpl))
tmpl = OVL_FIRST (tmpl);
- tree check = rebuild_concept_check (expr, map, args);
- if (check == error_mark_node)
- check = expr;
print_location (context, DECL_SOURCE_LOCATION (tmpl));
- pp_verbatim (context->printer, "required for the satisfaction of %qE\n",
check);
+
+ cxx_pretty_printer *pp = (cxx_pretty_printer *)context->printer;
+ pp_verbatim (pp, "required for the satisfaction of %qE", expr);
+ tree subst_map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
+ if (subst_map != error_mark_node)
+ map = subst_map;
+ if (map)
+ {
+ pp_cxx_whitespace (pp);
+ pp_cxx_left_bracket (pp);
+ pp->translate_string ("with");
+ pp_cxx_whitespace (pp);
+ pp_cxx_parameter_mapping (pp, map);
+ pp_cxx_right_bracket (pp);
+ }
+ pp_newline (pp);
}
/* Diagnose the entry point into the satisfaction error. Returns the next
diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic6.C
b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
new file mode 100644
index 00000000000..06b17caccbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
@@ -0,0 +1,14 @@
+// { dg-do compile { target c++2a } }
+
+template<typename T>
+ concept C = requires (T t) { t + 0; };
+// { dg-message "satisfaction of .C<T>. .with T = typename T::type." "" {
target *-*-* } .-1 }
+
+template<typename T>
+ concept D = C<T>;
+// { dg-message "satisfaction of .D<typename T::type>. .with T = int." "" {
target *-*-* } .-1 }
+
+template<typename T>
+ concept E = D<typename T::type>;
+
+static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
--
2.26.0.rc1.11.g30e9940356
