c++: Include the constraint parameter mapping in diagnostic constraint contexts

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 test case below:

  .../diagnostic6.C:14:15: error: static assertion failed
     14 | static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
        |               ^~~~~~
  .../diagnostic6.C:14:15: note: constraints not satisfied
  .../diagnostic6.C:4:11:   required for the satisfaction of ‘C<T>’
  .../diagnostic6.C:8:11:   required for the satisfaction of ‘D<typename T::type>’
  .../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 understand
the underlying failure.  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:

  .../diagnostic6.C:14:15: error: static assertion failed
     14 | static_assert(E<int>); // { dg-error "static assertion failed|not a class" }
        |               ^~~~~~
  .../diagnostic6.C:14:15: note: constraints not satisfied
  .../diagnostic6.C:4:11:   required for the satisfaction of ‘C<T>’ [with T = typename T::type]
  .../diagnostic6.C:8:11:   required for the satisfaction of ‘D<typename T::type>’ [with T = int]
  .../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 creating an invalid type rather than
failing due to the constraint evaluating to false.

gcc/cp/ChangeLog:

	* cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern.  Move
	the "[with ]" bits to here from ...
	(pp_cxx_atomic_constraint): ... here.
	* 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.

diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 929e709fadfad573269ddf2a4ceed8beee497fcd..0e01056aaeefd81a39951e81accd051dcbcef70d 100644 (file)
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,13 @@
+2020-03-20  Patrick Palka  <ppalka@redhat.com>
+
+       * cxx-pretty-print.c (pp_cxx_parameter_mapping): Make extern.  Move
+       the "[with ]" bits to here from ...
+       (pp_cxx_atomic_constraint): ... here.
+       * 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.
+
 2020-03-19  Jason Merrill  <jason@redhat.com>
 
        PR c++/94175

diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
index 100154e400f61e2ac59de182b26c1e079ae3dfc8..840b5a8db8ba6672334ec883fc61474d312a5c1d 100644 (file)
--- a/gcc/cp/cxx-pretty-print.c
+++ b/gcc/cp/cxx-pretty-print.c
@@ -2878,9 +2878,14 @@ 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)
 {
+  pp_cxx_whitespace (pp);
+  pp_cxx_left_bracket (pp);
+  pp->translate_string ("with");
+  pp_cxx_whitespace (pp);
+
   for (tree p = map; p; p = TREE_CHAIN (p))
     {
       tree parm = TREE_VALUE (p);
@@ -2903,6 +2908,8 @@ pp_cxx_parameter_mapping (cxx_pretty_printer *pp, tree map)
       if (TREE_CHAIN (p) != NULL_TREE)
        pp_cxx_separate_with (pp, ';');
     }
+
+  pp_cxx_right_bracket (pp);
 }
 
 void
@@ -2914,14 +2921,7 @@ pp_cxx_atomic_constraint (cxx_pretty_printer *pp, tree t)
   /* Emit the parameter mapping.  */
   tree map = ATOMIC_CONSTR_MAP (t);
   if (map && map != error_mark_node)
-    {
-      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_cxx_parameter_mapping (pp, map);
 }
 
 void

diff --git a/gcc/cp/cxx-pretty-print.h b/gcc/cp/cxx-pretty-print.h
index 7c7347f57bae5b1fdc70c2f84dd28f3029e43d08..494f3fdde597477c553d9b0e8c786ce86b7c00ac 100644 (file)
--- 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 cc51b6ffe1339c11eafdbd8e9b87561c36d13bb1..61d1218dc9006270f815dc46a763bbe165095e24 100644 (file)
--- 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,18 @@ 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);
+  if (map && map != error_mark_node)
+    {
+      tree subst_map = tsubst_parameter_mapping (map, args, tf_none, NULL_TREE);
+      pp_cxx_parameter_mapping (pp, (subst_map != error_mark_node
+                                    ? subst_map : map));
+    }
+  pp_newline (pp);
 }
 
 /* Diagnose the entry point into the satisfaction error. Returns the next

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index adf53b3e245449c34cc19a14f8fd83048b302751..12076d56e14ce5d226b11e709621be1610470e9a 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2020-03-20  Patrick Palka  <ppalka@redhat.com>
+
+       * g++.dg/concepts/diagnostic6.C: New test.
+
 2020-03-20  Srinath Parvathaneni  <srinath.parvathaneni@arm.com>
 
        * gcc.target/arm/mve/intrinsics/vabdq_x_f16.c: New test.

diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic6.C b/gcc/testsuite/g++.dg/concepts/diagnostic6.C
new file mode 100644 (file)
index 0000000..06b17ca
--- /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" }