| Message ID | CAA42iKzcx9dfs8-jfEwicV+SCmpaMuRo--NP0dFQfiUca+cqZA@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR libstdc++/98842: Fixed Constraints on operator<=>(optional, U) | expand |
On Thu, 3 Jun 2021 at 17:27, Seija K. via Libstdc++ <libstdc++@gcc.gnu.org> wrote: > The original operator was underconstrained. _Up needs to fulfill > compare_three_way_result, > as mentioned in this bug report > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98842 > Thanks, I'll get the patch applied next week. > diff --git a/libstdc++-v3/include/std/optional > b/libstdc++-v3/include/std/optional > index 8b9e038e6e510..9e61c1b2cbfbd 100644 > --- a/libstdc++-v3/include/std/optional > +++ b/libstdc++-v3/include/std/optional > @@ -1234,7 +1234,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > { return !__rhs || __lhs >= *__rhs; } > > #ifdef __cpp_lib_three_way_comparison > - template<typename _Tp, typename _Up> > + template<typename _Tp, three_way_comparable_with<_Tp> _Up> > constexpr compare_three_way_result_t<_Tp, _Up> > operator<=>(const optional<_Tp>& __x, const _Up& __v) > { return bool(__x) ? *__x <=> __v : strong_ordering::less; } > >
On Fri, 4 Jun 2021 at 21:41, Jonathan Wakely wrote: > > On Thu, 3 Jun 2021 at 17:27, Seija K. via Libstdc++ <libstdc++@gcc.gnu.org> > wrote: > > > The original operator was underconstrained. _Up needs to fulfill > > compare_three_way_result, > > as mentioned in this bug report > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98842 > > > > Thanks, I'll get the patch applied next week. The patch causes testsuite failures. I fixed it with the attached change instead, including a workaround for an apparent C++20 defect. Pushed to trunk so far. commit adec14811714e22a6c1f7f0199adc05370f0d8b0 Author: Jonathan Wakely <jwakely@redhat.com> Date: Mon Jun 7 13:02:15 2021 libstdc++: Constrain three-way comparison for std::optional [PR 98842] The operator<=>(const optional<T>&, const U&) operator is supposed to be constrained with three_way_comparable_with<U, T> so that it can only be used when T and U are weakly-equality-comparable and also three-way comparable. Adding that constrain completely breaks std::optional comparisons, because it causes constraint recursion. To avoid that, an additional check that U is not a specialization of std::optional is needed. That appears to be a defect in the standard and should be reported to LWG. Signed-off-by: Jonathan Wakely <jwakely@redhat.com> libstdc++-v3/ChangeLog: PR libstdc++/98842 * include/std/optional (operator<=>(const optional<T>& const U&)): Add missing constraint and add workaround for template recursion. * testsuite/20_util/optional/relops/three_way.cc: Check that type without equality comparison cannot be compared when wrapped in std::optional. diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 8b9e038e6e5..415f8c49ef4 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -1234,7 +1234,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return !__rhs || __lhs >= *__rhs; } #ifdef __cpp_lib_three_way_comparison + template<typename _Tp> + inline constexpr bool __is_optional_v = false; + template<typename _Tp> + inline constexpr bool __is_optional_v<optional<_Tp>> = true; + template<typename _Tp, typename _Up> + requires (!__is_optional_v<_Up>) + && three_way_comparable_with<_Tp, _Up> constexpr compare_three_way_result_t<_Tp, _Up> operator<=>(const optional<_Tp>& __x, const _Up& __v) { return bool(__x) ? *__x <=> __v : strong_ordering::less; } diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/three_way.cc b/libstdc++-v3/testsuite/20_util/optional/relops/three_way.cc index 953bef4a1ea..5fc5eec5abf 100644 --- a/libstdc++-v3/testsuite/20_util/optional/relops/three_way.cc +++ b/libstdc++-v3/testsuite/20_util/optional/relops/three_way.cc @@ -15,8 +15,8 @@ // with this library; see the file COPYING3. If not see // <http://www.gnu.org/licenses/>. -// { dg-options "-std=gnu++2a" } -// { dg-do compile { target c++2a } } +// { dg-options "-std=gnu++20" } +// { dg-do compile { target c++20 } } #include <optional> @@ -74,3 +74,21 @@ test02() static_assert( nullopt <= O{} ); static_assert( nullopt <= O{1} ); } + +template<typename T> + concept has_spaceship = requires (const T& t) { t <=> t; }; + +void +test03() +{ + struct E + { + auto operator<=>(const E&) const { return std::strong_ordering::equal; } + }; + static_assert( !std::three_way_comparable<E> ); // not equality comparable + using O = std::optional<E>; + static_assert( !std::three_way_comparable<O> ); + static_assert( ! has_spaceship<O> ); // PR libstdc++/98842 + struct U : O { }; + static_assert( ! has_spaceship<U> ); +}
Awesome, thanks! On Mon, Jun 7, 2021 at 11:30 AM Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > On Fri, 4 Jun 2021 at 21:41, Jonathan Wakely wrote: > > > > On Thu, 3 Jun 2021 at 17:27, Seija K. via Libstdc++ < > libstdc++@gcc.gnu.org> > > wrote: > > > > > The original operator was underconstrained. _Up needs to fulfill > > > compare_three_way_result, > > > as mentioned in this bug report > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98842 > > > > > > > Thanks, I'll get the patch applied next week. > > The patch causes testsuite failures. > > I fixed it with the attached change instead, including a workaround > for an apparent C++20 defect. > > Pushed to trunk so far. >
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional index 8b9e038e6e510..9e61c1b2cbfbd 100644 --- a/libstdc++-v3/include/std/optional +++ b/libstdc++-v3/include/std/optional @@ -1234,7 +1234,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION { return !__rhs || __lhs >= *__rhs; } #ifdef __cpp_lib_three_way_comparison - template<typename _Tp, typename _Up> + template<typename _Tp, three_way_comparable_with<_Tp> _Up> constexpr compare_three_way_result_t<_Tp, _Up> operator<=>(const optional<_Tp>& __x, const _Up& __v) { return bool(__x) ? *__x <=> __v : strong_ordering::less; }