Re: [committed] libstdc++: Fix constraints on std::optional comparisons [PR 96269]

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 896 bytes --]

On 05/11/20 19:09 +0000, Jonathan Wakely wrote:
>The relational operators for std::optional were using the wrong types
>in the declval expressions used to constrain them. Instead of using
>const lvalues they were using non-const rvalues, which meant that a type
>might satisfy the constraints but then give an error when the function
>body was instantiated.
>
>libstdc++-v3/ChangeLog:
>
>	PR libstdc++/96269
>	* include/std/optional (operator==, operator!=, operator<)
>	(operator>, operator<=, operator>=): Fix types used in
>	SFINAE constraints.
>	* testsuite/20_util/optional/relops/96269.cc: New test.
>
>Tested powerpc64le-linux. Committed to trunk.

When concepts are supported we can make the alias templates
__optional_eq_t et al use a requires-expression instead of SFINAE.
This is potentially faster to compile, given expected improvements
to C++20 compilers.

I'm testing this patch.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2717 bytes --]

commit c5d8e2ba0ad20425cc7778152824d9e5267b0ec5
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 5 19:45:52 2020

    libstdc++: Use concepts to constrain std::optional relops
    
    When concepts are supported we can make the alias templates
    __optional_eq_t et al use a requires-expression instead of SFINAE.
    This is potentially faster to compile, given expected improvements
    to C++20 compilers.
    
    libstdc++-v3/ChangeLog:
    
            * include/std/optional [__cpp_concepts] (__optional_eq_t)
            (__optional_ne_t, __optional_lt_t, __optional_gt_t)
            (__optional_le_t, __optional_ge_t): Use requires-clause on
            alias template.

diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 5ea5b39d0e69..4e9618648250 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -998,9 +998,48 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       void reset() noexcept { this->_M_reset(); }
     };
 
+#if __cpp_lib_concepts
+  template<typename _Tp, typename _Up>
+    requires requires (const _Tp __t, const _Up __u) {
+	  { __t == __u } -> convertible_to<bool>;
+    }
+    using __optional_eq_t = bool;
+
+  template<typename _Tp, typename _Up>
+    requires requires (const _Tp __t, const _Up __u) {
+	  { __t != __u } -> convertible_to<bool>;
+    }
+    using __optional_ne_t = bool;
+
+  template<typename _Tp, typename _Up>
+    requires requires (const _Tp __t, const _Up __u) {
+	  { __t < __u } -> convertible_to<bool>;
+    }
+    using __optional_lt_t = bool;
+
+  template<typename _Tp, typename _Up>
+    requires requires (const _Tp __t, const _Up __u) {
+	  { __t > __u } -> convertible_to<bool>;
+    }
+    using __optional_gt_t = bool;
+
+  template<typename _Tp, typename _Up>
+    requires requires (const _Tp __t, const _Up __u) {
+	  { __t <= __u } -> convertible_to<bool>;
+    }
+    using __optional_le_t = bool;
+
+  template<typename _Tp, typename _Up>
+    requires requires (const _Tp __t, const _Up __u) {
+	  { __t >= __u } -> convertible_to<bool>;
+    }
+    using __optional_ge_t = bool;
+
+#else // concepts
+
   template<typename _Tp>
     using __optional_relop_t =
-      enable_if_t<is_convertible<_Tp, bool>::value, bool>;
+      enable_if_t<is_convertible_v<_Tp, bool>, bool>;
 
   template<typename _Tp, typename _Up>
     using __optional_eq_t = __optional_relop_t<
@@ -1031,6 +1070,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     using __optional_ge_t = __optional_relop_t<
       decltype(std::declval<const _Tp&>() >= std::declval<const _Up&>())
       >;
+#endif // concepts
 
   // Comparisons between optional values.
   template<typename _Tp, typename _Up>