c++: improve constraint recursion diagnostic

Our constraint recursion diagnostics are not ideal because they
usually show the atom with an uninstantiated parameter mapping, e.g

concepts-recursive-sat5.C:6:41: error: satisfaction of atomic constraint 'requires(A a, T t) {a | t;} [with T = T]' depends on itself

This is a consequence of our two-level caching of atomic constraints,
where we first cache the uninstantiated atom+args and then the
instantiated atom+no args, and most likely the first level of caching
detects the recursion, at which point we have no way to get a hold of
the instantiated atom.

This patch fixes this by linking the the first level of caching to the
second level, so that we can conveniently print the instantiated atom in
case of constraint recursion detected from the first level of caching.

Alternatively we could make only the second level of caching diagnose
constraint recursion but then we'd no longer catch constraint recursion
that occurs during parameter mapping instantiation.  This current approach
seems simpler, and it also seems natural to have the two cache entries
somehow linked anyway.

gcc/cp/ChangeLog:

	* constraint.cc (struct sat_entry): New data member inst_entry.
	(satisfaction_cache::satisfaction_cache): Initialize inst_entry.
	(satisfaction_cache::get): Use it to prefer printing the
	instantiated atom in case of constraint recursion.
	(satisfy_atom): Set inst_entry of the first cache entry to point
	to the second entry.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-recursive-sat2.C: Verify that the
	instantiated parameter mapping is printed.
	* g++.dg/cpp2a/concepts-recursive-sat5.C: Likewise.

Reviewed-by: Jason Merrill <jason@redhat.com>

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 718982715990e1185d584e15328597dfbb7ce779..4d667ced342944535e4bf7d8451b74cd4230ed64 100644 (file)
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -1972,6 +1972,10 @@ struct GTY((for_user)) sat_entry
      the first time.  */
   tree result;
 
+  /* For a !ATOMIC_CONSTR_MAP_INSTANTIATED_P atom, this conveniently points to
+     the entry for the corresponding atom after instantiating its mapping.  */
+  sat_entry *inst_entry;
+
   /* The value of input_location when satisfaction of ATOM+ARGS was first
      performed.  */
   location_t location;
@@ -2131,6 +2135,7 @@ satisfaction_cache
       entry->atom = atom;
       entry->args = args;
       entry->result = NULL_TREE;
+      entry->inst_entry = nullptr;
       entry->location = input_location;
       entry->ftc_begin = entry->ftc_end = -1;
       entry->diagnose_instability = false;
@@ -2170,10 +2175,12 @@ satisfaction_cache::get ()
     {
       /* If we get here, it means satisfaction is self-recursive.  */
       gcc_checking_assert (!entry->result || seen_error ());
+      /* Prefer printing the instantiated mapping.  */
+      tree atom = entry->inst_entry ? entry->inst_entry->atom : entry->atom;
       if (info.noisy ())
-       error_at (EXPR_LOCATION (ATOMIC_CONSTR_EXPR (entry->atom)),
+       error_at (EXPR_LOCATION (ATOMIC_CONSTR_EXPR (atom)),
                  "satisfaction of atomic constraint %qE depends on itself",
-                 entry->atom);
+                 atom);
       return error_mark_node;
     }
 
@@ -2493,6 +2500,8 @@ satisfy_atom (tree t, tree args, sat_info info)
   gcc_assert (!ATOMIC_CONSTR_MAP_INSTANTIATED_P (t));
   ATOMIC_CONSTR_MAP_INSTANTIATED_P (t) = true;
   satisfaction_cache inst_cache (t, /*args=*/NULL_TREE, info);
+  if (cache.entry && inst_cache.entry)
+    cache.entry->inst_entry = inst_cache.entry;
   if (tree r = inst_cache.get ())
     {
       cache.entry->location = inst_cache.entry->location;

diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat2.C b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat2.C
index 9bc96f58979dc4c3c065ef3425d6751e0f67c41c..a264f26bc55df9139950d66b6a8f4333d3596c7a 100644 (file)
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat2.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat2.C
@@ -1,7 +1,8 @@
 // { dg-do compile { target c++20 } }
 
 template<typename T>
-concept Fooable = requires(T t) { foo(t); }; // { dg-error "depends on itself" }
+concept Fooable = requires(T t) { foo(t); };
+// { dg-error "T = test::S]' depends on itself" "" { target *-*-* } .-1 }
 
 template<Fooable T>
 void foo(T t) { }

diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C
index ba564873a2042b70aaffc28f4eb7c04efc008ecd..63e032494e3d80ae66816d4b447c38c871b01b5c 100644 (file)
--- a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C
@@ -3,7 +3,8 @@
 
 struct A { };
 
-template<typename T> concept pipeable = requires(A a, T t) { a | t; }; // { dg-error "depends on itself" }
+template<typename T> concept pipeable = requires(A a, T t) { a | t; };
+// { dg-error "with T = int]' depends on itself" "" { target *-*-* } .-1 }
 
 template<pipeable T> void operator|(A, T);