| Message ID | 20230316164816.2493686-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: ICE with diagnosed constraint recursion [PR100288] | expand |
On 3/16/23 12:48, Patrick Palka wrote: > When satisfaction_cache::get detects constraint recursion, it asserts > that entry->result is empty. This makes sense when we're initially > detecting/diagnosing recursion from the inner recursive call, but > aftewards from the outer recursive call the recursion error is treated > like any other SFINAE error encountered during satisfaction, and we set > entry->result to whatever the satisfaction value ended up being. > > Perhaps we should keep entry->result cleared in this case, but that'd > require the inner recursive call to communicate to the outer recursive > call that constraint recursion occurred, likely via setting entry->result > to some sentinel value, which doesn't seem to be worth the complexity. > So this patch just relaxes the problematic assert to accept non-empty > entry->result as long as we've already issued an error. > > Tested on x86_64-pc-linux-gnu, does this look OK for trunk? Backports > seems unnecessary as the problematic assert is a checking assert. OK. > PR c++/100288 > > gcc/cp/ChangeLog: > > * constraint.cc (satisfaction_cache::get): Relax overly strict > checking assert in the constraint recursion case. > > gcc/testsuite/ChangeLog: > > * g++.dg/cpp2a/concepts-recursive-sat5.C: New test. > --- > gcc/cp/constraint.cc | 2 +- > .../g++.dg/cpp2a/concepts-recursive-sat5.C | 13 +++++++++++++ > 2 files changed, 14 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index a28c85178fe..273d15ab097 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2705,7 +2705,7 @@ satisfaction_cache::get () > if (entry->evaluating) > { > /* If we get here, it means satisfaction is self-recursive. */ > - gcc_checking_assert (!entry->result); > + gcc_checking_assert (!entry->result || seen_error ()); > if (info.noisy ()) > error_at (EXPR_LOCATION (ATOMIC_CONSTR_EXPR (entry->atom)), > "satisfaction of atomic constraint %qE depends on itself", > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C > new file mode 100644 > index 00000000000..b7a02815db9 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C > @@ -0,0 +1,13 @@ > +// PR c++/100288 > +// { dg-do compile { target c++20 } } > + > +class A { }; > + > +template <class T> concept pipeable = requires(A a, T t) { a | t; }; // { dg-error "depends on itself" } > + > +template <pipeable T> void operator|(A, T); > + > +void f(A tab) { > + tab | 1; // { dg-error "no match" } > + tab | 1; // { dg-error "no match" } > +}
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index a28c85178fe..273d15ab097 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2705,7 +2705,7 @@ satisfaction_cache::get () if (entry->evaluating) { /* If we get here, it means satisfaction is self-recursive. */ - gcc_checking_assert (!entry->result); + gcc_checking_assert (!entry->result || seen_error ()); if (info.noisy ()) error_at (EXPR_LOCATION (ATOMIC_CONSTR_EXPR (entry->atom)), "satisfaction of atomic constraint %qE depends on itself", diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C new file mode 100644 index 00000000000..b7a02815db9 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat5.C @@ -0,0 +1,13 @@ +// PR c++/100288 +// { dg-do compile { target c++20 } } + +class A { }; + +template <class T> concept pipeable = requires(A a, T t) { a | t; }; // { dg-error "depends on itself" } + +template <pipeable T> void operator|(A, T); + +void f(A tab) { + tab | 1; // { dg-error "no match" } + tab | 1; // { dg-error "no match" } +}