Message ID | 20200529155935.2519697-1-ppalka@redhat.com |
---|---|
State | New |
Headers | show |
Series | c++: satisfaction value of type typedef to bool [PR95386] | expand |
On 5/29/20 11:59 AM, Patrick Palka wrote: > In the testcase below, the satisfaction value of fn1<int>'s constraint > is INTEGER_CST '1' of type BOOLEAN_TYPE value_type, which is a typedef > to the standard boolean_type_node. But satisfaction_value expects to > see exactly boolean_true_node or integer_one_node, which this value is > neither, causing us to trip over the assert therein. > > This patch relaxes satisfaction_value to accept any INTEGER_CST which > satisfies integer_zerop or integer_onep. (It seems we could get away > with accepting only INTEGER_CSTs of type BOOLEAN_TYPE, but that wouldn't > be a proper relaxation of what the subroutine currently accepts and > would therefore be more risky to backport.) I think for GCC 11 I'd prefer to restrict it to BOOLEAN_TYPE. This patch is OK for GCC 10. > Passes 'make check-c++', does this look OK to commit to master and to > the GCC 10 branch after a full bootstrap and regtest? > > gcc/cp/ChangeLog: > > PR c++/95386 > * constraint.cc (satisfaction_value): Relax to accept any > INTEGER_CST that satisfies integer_zerop or integer_onep. > > gcc/testsuite/ChangeLog: > > PR c++/95386 > * g++.dg/concepts/pr95386.C: New test. > --- > gcc/cp/constraint.cc | 7 ++++--- > gcc/testsuite/g++.dg/concepts/pr95386.C | 11 +++++++++++ > 2 files changed, 15 insertions(+), 3 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/pr95386.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index eb72bfe5936..5a247cfb738 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2490,11 +2490,12 @@ satisfy_disjunction (tree t, tree args, subst_info info) > tree > satisfaction_value (tree t) > { > - if (t == error_mark_node) > + if (t == error_mark_node || t == boolean_true_node || t == boolean_false_node) > return t; > - if (t == boolean_true_node || t == integer_one_node) > + gcc_assert (TREE_CODE (t) == INTEGER_CST); > + if (integer_onep (t)) > return boolean_true_node; > - if (t == boolean_false_node || t == integer_zero_node) > + if (integer_zerop (t)) > return boolean_false_node; > > /* Anything else should be invalid. */ > diff --git a/gcc/testsuite/g++.dg/concepts/pr95386.C b/gcc/testsuite/g++.dg/concepts/pr95386.C > new file mode 100644 > index 00000000000..3c683e5693c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/pr95386.C > @@ -0,0 +1,11 @@ > +// PR c++/95386 > +// { dg-do compile { target concepts } } > + > +template <typename> struct blah { > + typedef bool value_type; > + constexpr operator value_type() { return false; } > +}; > + > +template <class T> void fn1(T) requires (!blah<T>()); > + > +void fn2() { fn1(0); } >
On Fri, 29 May 2020, Jason Merrill wrote: > On 5/29/20 11:59 AM, Patrick Palka wrote: > > In the testcase below, the satisfaction value of fn1<int>'s constraint > > is INTEGER_CST '1' of type BOOLEAN_TYPE value_type, which is a typedef > > to the standard boolean_type_node. But satisfaction_value expects to > > see exactly boolean_true_node or integer_one_node, which this value is > > neither, causing us to trip over the assert therein. > > > > This patch relaxes satisfaction_value to accept any INTEGER_CST which > > satisfies integer_zerop or integer_onep. (It seems we could get away > > with accepting only INTEGER_CSTs of type BOOLEAN_TYPE, but that wouldn't > > be a proper relaxation of what the subroutine currently accepts and > > would therefore be more risky to backport.) > > I think for GCC 11 I'd prefer to restrict it to BOOLEAN_TYPE. This patch is > OK for GCC 10. Sounds good. Would the following be OK for GCC 11 after a full bootstrap and regtest? I opted to mirror satisfy_atom and instead check same_type_p (..., boolean_type_node). -- >8 -- Subject: [PATCH] c++: satisfaction value of type typedef to bool [PR95386] In the testcase below, the satisfaction value of fn1<int>'s constraint is INTEGER_CST '1' of type BOOLEAN_TYPE value_type, which is a typedef to the standard boolean_type_node. But satisfaction_value expects to see exactly boolean_true_node or integer_one_node, which this value is neither, causing us to trip over the assert therein. This patch changes satisfaction_value to accept INTEGER_CST of any boolean type. gcc/cp/ChangeLog: PR c++/95386 * constraint.cc (satisfaction_value): Accept INTEGER_CST of any boolean type. gcc/testsuite/ChangeLog: PR c++/95386 * g++.dg/concepts/pr95386.C: New test. --- gcc/cp/constraint.cc | 14 +++++++------- gcc/testsuite/g++.dg/concepts/pr95386.C | 11 +++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) create mode 100644 gcc/testsuite/g++.dg/concepts/pr95386.C diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index eb72bfe5936..92ff283013e 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2490,15 +2490,15 @@ satisfy_disjunction (tree t, tree args, subst_info info) tree satisfaction_value (tree t) { - if (t == error_mark_node) + if (t == error_mark_node || t == boolean_true_node || t == boolean_false_node) return t; - if (t == boolean_true_node || t == integer_one_node) - return boolean_true_node; - if (t == boolean_false_node || t == integer_zero_node) - return boolean_false_node; - /* Anything else should be invalid. */ - gcc_assert (false); + gcc_assert (TREE_CODE (t) == INTEGER_CST + && same_type_p (TREE_TYPE (t), boolean_type_node)); + if (integer_zerop (t)) + return boolean_false_node; + else + return boolean_true_node; } /* Build a new template argument list with template arguments corresponding diff --git a/gcc/testsuite/g++.dg/concepts/pr95386.C b/gcc/testsuite/g++.dg/concepts/pr95386.C new file mode 100644 index 00000000000..3c683e5693c --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/pr95386.C @@ -0,0 +1,11 @@ +// PR c++/95386 +// { dg-do compile { target concepts } } + +template <typename> struct blah { + typedef bool value_type; + constexpr operator value_type() { return false; } +}; + +template <class T> void fn1(T) requires (!blah<T>()); + +void fn2() { fn1(0); }
On 5/29/20 1:40 PM, Patrick Palka wrote: > On Fri, 29 May 2020, Jason Merrill wrote: > >> On 5/29/20 11:59 AM, Patrick Palka wrote: >>> In the testcase below, the satisfaction value of fn1<int>'s constraint >>> is INTEGER_CST '1' of type BOOLEAN_TYPE value_type, which is a typedef >>> to the standard boolean_type_node. But satisfaction_value expects to >>> see exactly boolean_true_node or integer_one_node, which this value is >>> neither, causing us to trip over the assert therein. >>> >>> This patch relaxes satisfaction_value to accept any INTEGER_CST which >>> satisfies integer_zerop or integer_onep. (It seems we could get away >>> with accepting only INTEGER_CSTs of type BOOLEAN_TYPE, but that wouldn't >>> be a proper relaxation of what the subroutine currently accepts and >>> would therefore be more risky to backport.) >> >> I think for GCC 11 I'd prefer to restrict it to BOOLEAN_TYPE. This patch is >> OK for GCC 10. > > Sounds good. Would the following be OK for GCC 11 after a full > bootstrap and regtest? > I opted to mirror satisfy_atom and instead check > same_type_p (..., boolean_type_node). OK. > -- >8 -- > > Subject: [PATCH] c++: satisfaction value of type typedef to bool [PR95386] > > In the testcase below, the satisfaction value of fn1<int>'s constraint > is INTEGER_CST '1' of type BOOLEAN_TYPE value_type, which is a typedef > to the standard boolean_type_node. But satisfaction_value expects to > see exactly boolean_true_node or integer_one_node, which this value is > neither, causing us to trip over the assert therein. > > This patch changes satisfaction_value to accept INTEGER_CST of any > boolean type. > > gcc/cp/ChangeLog: > > PR c++/95386 > * constraint.cc (satisfaction_value): Accept INTEGER_CST of any > boolean type. > > gcc/testsuite/ChangeLog: > > PR c++/95386 > * g++.dg/concepts/pr95386.C: New test. > --- > gcc/cp/constraint.cc | 14 +++++++------- > gcc/testsuite/g++.dg/concepts/pr95386.C | 11 +++++++++++ > 2 files changed, 18 insertions(+), 7 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/concepts/pr95386.C > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc > index eb72bfe5936..92ff283013e 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -2490,15 +2490,15 @@ satisfy_disjunction (tree t, tree args, subst_info info) > tree > satisfaction_value (tree t) > { > - if (t == error_mark_node) > + if (t == error_mark_node || t == boolean_true_node || t == boolean_false_node) > return t; > - if (t == boolean_true_node || t == integer_one_node) > - return boolean_true_node; > - if (t == boolean_false_node || t == integer_zero_node) > - return boolean_false_node; > > - /* Anything else should be invalid. */ > - gcc_assert (false); > + gcc_assert (TREE_CODE (t) == INTEGER_CST > + && same_type_p (TREE_TYPE (t), boolean_type_node)); > + if (integer_zerop (t)) > + return boolean_false_node; > + else > + return boolean_true_node; > } > > /* Build a new template argument list with template arguments corresponding > diff --git a/gcc/testsuite/g++.dg/concepts/pr95386.C b/gcc/testsuite/g++.dg/concepts/pr95386.C > new file mode 100644 > index 00000000000..3c683e5693c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/pr95386.C > @@ -0,0 +1,11 @@ > +// PR c++/95386 > +// { dg-do compile { target concepts } } > + > +template <typename> struct blah { > + typedef bool value_type; > + constexpr operator value_type() { return false; } > +}; > + > +template <class T> void fn1(T) requires (!blah<T>()); > + > +void fn2() { fn1(0); } >
diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc index eb72bfe5936..5a247cfb738 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -2490,11 +2490,12 @@ satisfy_disjunction (tree t, tree args, subst_info info) tree satisfaction_value (tree t) { - if (t == error_mark_node) + if (t == error_mark_node || t == boolean_true_node || t == boolean_false_node) return t; - if (t == boolean_true_node || t == integer_one_node) + gcc_assert (TREE_CODE (t) == INTEGER_CST); + if (integer_onep (t)) return boolean_true_node; - if (t == boolean_false_node || t == integer_zero_node) + if (integer_zerop (t)) return boolean_false_node; /* Anything else should be invalid. */ diff --git a/gcc/testsuite/g++.dg/concepts/pr95386.C b/gcc/testsuite/g++.dg/concepts/pr95386.C new file mode 100644 index 00000000000..3c683e5693c --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/pr95386.C @@ -0,0 +1,11 @@ +// PR c++/95386 +// { dg-do compile { target concepts } } + +template <typename> struct blah { + typedef bool value_type; + constexpr operator value_type() { return false; } +}; + +template <class T> void fn1(T) requires (!blah<T>()); + +void fn2() { fn1(0); }