On Mon, 14 Apr 2025, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
> look OK for trunk and perhaps 14?
Ping; this patch slipped through the cracks. Would it be OK
for trunk and perhaps 15?
>
> -- >8 --
>
> 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 levels of caching
> linked in some way anyway.
>
> gcc/cp/ChangeLog:
>
> * constraint.cc (struct sat_entry): New data member inst_entry.
> (satisfaction_cache::get): Use it to prefer printing the
> instantiated atom in case of constraint recursion.
> (satisfy_atom): Set inst_entry on the first 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.
> ---
> gcc/cp/constraint.cc | 12 ++++++++++--
> gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat2.C | 3 ++-
> gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C | 3 ++-
> 3 files changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index 44fb086c6306..1bb237f5bb4f 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -1895,6 +1895,10 @@ struct GTY((for_user)) sat_entry
> the first time. */
> tree result;
>
> + /* For a !ATOMIC_CONSTR_MAP_INSTANTIATED_P atom, this is 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;
> @@ -2093,10 +2097,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;
> }
>
> @@ -2416,6 +2422,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)
> + 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 9bc96f58979d..a264f26bc55d 100644
> --- 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 ba564873a204..63e032494e3d 100644
> --- 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);
>
> --
> 2.49.0.221.g485f5f8636
>
>
