public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
@ 2016-07-11 20:41 Ville Voutilainen
  2016-07-12 22:31 ` Jonathan Wakely
  0 siblings, 1 reply; 8+ messages in thread
From: Ville Voutilainen @ 2016-07-11 20:41 UTC (permalink / raw)
  To: gcc-patches, libstdc++

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

Tested on Linux-X64.

The test adjustments are so that the tests are kept valid, which
required adding a bunch of now-required relops to the test types. The new
transparent-but-non-synthesizing aspects of the relops are tested
separately. The constraints are a valid implementation of the
current Requires-clauses on these operators; I will file an LWG
issue suggesting that such Requires-clauses are instead made
SFINAE-constraints like in this implementation. The rationale
for that being that if such "wrapper types" are supposed to be
"transparent", it would be rather good if they are transparent
for SFINAE-querying for the existence of such relops as well, rather
than always report true and then fail to instantiate.

2016-07-11  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Implement P0307R2, Making Optional Greater Equal Again.
    * include/std/optional (__optional_relop_t): New.
    (operator==(const optional<_Tp>&, const optional<_Tp>&)): Constrain.
    (operator!=(const optional<_Tp>&, const optional<_Tp>&)):
    Constrain and make transparent.
    (operator<(const optional<_Tp>&, const optional<_Tp>&)): Constrain.
    (operator>(const optional<_Tp>&, const optional<_Tp>&)):
    Constrain and make transparent.
    (operator<=(const optional<_Tp>&, const optional<_Tp>&)): Likewise.
    (operator>=(const optional<_Tp>&, const optional<_Tp>&)): Likewise.
    (operator==(const optional<_Tp>&, const _Tp&): Constrain.
    (operator==(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator!=(const optional<_Tp>&, _Tp const&)):
    Constrain and make transparent.
    (operator!=(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator<(const optional<_Tp>&, const _Tp&)): Constrain.
    (operator<(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator>(const optional<_Tp>&, const _Tp&)):
    Constrain and make transparent.
    (operator>(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator<=(const optional<_Tp>&, const _Tp&)): Likewise.
    (operator<=(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator>=(const optional<_Tp>&, const _Tp&)): Likewise.
    (operator>=(const _Tp&, const optional<_Tp>&)): Likewise.
    * testsuite/20_util/optional/constexpr/relops/2.cc: Adjust.
    * testsuite/20_util/optional/constexpr/relops/4.cc: Likewise.
    * testsuite/20_util/optional/relops/1.cc: Likewise.
    * testsuite/20_util/optional/relops/2.cc: Likewise.
    * testsuite/20_util/optional/relops/3.cc: Likewise.
    * testsuite/20_util/optional/relops/4.cc: Likewise.
    * testsuite/20_util/optional/requirements.cc: Add tests to verify
    that optional's relops are transparent and don't synthesize
    operators.

[-- Attachment #2: P0307R2.diff --]
[-- Type: text/plain, Size: 14734 bytes --]

diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index e9a86a4..1786c2c 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	}
     };
 
+  template<typename _Tp>
+    using __optional_relop_t =
+    enable_if_t<is_constructible<bool, _Tp>::value, bool>;
+
   // [X.Y.8] Comparisons between optional values.
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator==(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Tp>())>
     {
       return static_cast<bool>(__lhs) == static_cast<bool>(__rhs)
 	     && (!__lhs || *__lhs == *__rhs);
     }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator!=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return !(__lhs == __rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Tp>())>
+    {
+      return static_cast<bool>(__lhs) != static_cast<bool>(__rhs)
+	|| (static_cast<bool>(__lhs) && *__lhs != *__rhs);
+    }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Tp>())>
     {
       return static_cast<bool>(__rhs) && (!__lhs || *__lhs < *__rhs);
     }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return __rhs < __lhs; }
+    -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Tp>())>
+    {
+      return static_cast<bool>(__lhs) && (!__rhs || *__lhs > *__rhs);
+    }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return !(__rhs < __lhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Tp>())>
+    {
+      return !__lhs || (static_cast<bool>(__rhs) && *__lhs <= *__rhs);
+    }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return !(__lhs < __rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Tp>())>
+    {
+      return !__rhs || (static_cast<bool>(__lhs) && *__lhs >= *__rhs);
+    }
 
   // [X.Y.9] Comparisons with nullopt.
   template<typename _Tp>
@@ -884,64 +903,76 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // [X.Y.10] Comparisons with value type.
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator==(const optional<_Tp>& __lhs, const _Tp& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Tp>())>
     { return __lhs && *__lhs == __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator==(const _Tp& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Tp>())>
     { return __rhs && __lhs == *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator!=(const optional<_Tp>& __lhs, _Tp const& __rhs)
-    { return !__lhs || !(*__lhs == __rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Tp>())>
+    { return !__lhs || *__lhs != __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator!=(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return !__rhs || !(__lhs == *__rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Tp>())>
+    { return !__rhs || __lhs != *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<(const optional<_Tp>& __lhs, const _Tp& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Tp>())>
     { return !__lhs || *__lhs < __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<(const _Tp& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Tp>())>
     { return __rhs && __lhs < *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>(const optional<_Tp>& __lhs, const _Tp& __rhs)
-    { return __lhs && __rhs < *__lhs; }
+    -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Tp>())>
+    { return __lhs && *__lhs > __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return !__rhs || *__rhs < __lhs; }
+    -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Tp>())>
+    { return !__rhs || __lhs > *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<=(const optional<_Tp>& __lhs, const _Tp& __rhs)
-    { return !__lhs || !(__rhs < *__lhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Tp>())>
+    { return !__lhs || *__lhs <= __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<=(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return __rhs && !(*__rhs < __lhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Tp>())>
+    { return __rhs && __lhs <= *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>=(const optional<_Tp>& __lhs, const _Tp& __rhs)
-    { return __lhs && !(*__lhs < __rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Tp>())>
+    { return __lhs && *__lhs >= __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>=(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return !__rhs || !(__lhs < *__rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Tp>())>
+    { return !__rhs || __lhs >= *__rhs; }
 
   // [X.Y.11]
   template<typename _Tp>
diff --git a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc
index 9aa9273..0ce00c1 100644
--- a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc
@@ -54,6 +54,18 @@ namespace ns
   operator<(value_type const& lhs, value_type const& rhs)
   { return (lhs.i < rhs.i) || (!(rhs.i < lhs.i) && strrel(lhs.s, rhs.s)); }
 
+  constexpr bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return rhs < lhs; }
+
+  constexpr bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return lhs < rhs || lhs == rhs; }
+
+  constexpr bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return lhs > rhs || lhs == rhs; }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc
index 15130d4..d6294ad 100644
--- a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc
@@ -54,6 +54,18 @@ namespace ns
   operator<(value_type const& lhs, value_type const& rhs)
   { return (lhs.i < rhs.i) || (!(rhs.i < lhs.i) && strrel(lhs.s, rhs.s)); }
 
+  constexpr bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return rhs < lhs; }
+
+  constexpr bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return lhs < rhs || lhs == rhs; }
+
+  constexpr bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return lhs > rhs || lhs == rhs; }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/1.cc b/libstdc++-v3/testsuite/20_util/optional/relops/1.cc
index 6277032..1315902 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/1.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/1.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/2.cc b/libstdc++-v3/testsuite/20_util/optional/relops/2.cc
index 65071c0..1351265 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/2.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/2.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/3.cc b/libstdc++-v3/testsuite/20_util/optional/relops/3.cc
index 2fd9e8b..95fde3b 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/3.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/3.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/4.cc b/libstdc++-v3/testsuite/20_util/optional/relops/4.cc
index 363e633..78d0eb8 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/4.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/4.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/requirements.cc b/libstdc++-v3/testsuite/20_util/optional/requirements.cc
index aab572f..d416e59 100644
--- a/libstdc++-v3/testsuite/20_util/optional/requirements.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/requirements.cc
@@ -257,3 +257,73 @@ int main()
     static_assert( *o == 33, "" );
   }
 }
+
+using std::void_t;
+using std::declval;
+using std::true_type;
+using std::false_type;
+
+template <class T, class = void>
+struct is_eq_comparable : false_type {};
+template <class T>
+struct is_eq_comparable<T, void_t<decltype(declval<T>() == declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_neq_comparable : false_type {};
+template <class T>
+struct is_neq_comparable<T, void_t<decltype(declval<T>() != declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_lt_comparable : false_type {};
+template <class T>
+struct is_lt_comparable<T, void_t<decltype(declval<T>() < declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_gt_comparable : false_type {};
+template <class T>
+struct is_gt_comparable<T, void_t<decltype(declval<T>() > declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_le_comparable : false_type {};
+template <class T>
+struct is_le_comparable<T, void_t<decltype(declval<T>() <= declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_ge_comparable : false_type {};
+template <class T>
+struct is_ge_comparable<T, void_t<decltype(declval<T>() >= declval<T>())>>
+: true_type {};
+
+using std::optional;
+
+static_assert(is_eq_comparable<optional<int>>::value, "");
+static_assert(is_neq_comparable<optional<int>>::value, "");
+static_assert(is_lt_comparable<optional<int>>::value, "");
+static_assert(is_gt_comparable<optional<int>>::value, "");
+static_assert(is_le_comparable<optional<int>>::value, "");
+static_assert(is_ge_comparable<optional<int>>::value, "");
+
+struct JustEq {};
+bool operator==(const JustEq&, const JustEq&);
+
+static_assert(is_eq_comparable<optional<JustEq>>::value, "");
+static_assert(!is_neq_comparable<optional<JustEq>>::value, "");
+static_assert(!is_lt_comparable<optional<JustEq>>::value, "");
+static_assert(!is_gt_comparable<optional<JustEq>>::value, "");
+static_assert(!is_le_comparable<optional<JustEq>>::value, "");
+static_assert(!is_ge_comparable<optional<JustEq>>::value, "");
+
+struct JustLt {};
+bool operator<(const JustLt&, const JustLt&);
+
+static_assert(!is_eq_comparable<optional<JustLt>>::value, "");
+static_assert(!is_neq_comparable<optional<JustLt>>::value, "");
+static_assert(is_lt_comparable<optional<JustLt>>::value, "");
+static_assert(!is_gt_comparable<optional<JustLt>>::value, "");
+static_assert(!is_le_comparable<optional<JustLt>>::value, "");
+static_assert(!is_ge_comparable<optional<JustLt>>::value, "");

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
  2016-07-11 20:41 [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again Ville Voutilainen
@ 2016-07-12 22:31 ` Jonathan Wakely
  2016-07-13 10:06   ` Ville Voutilainen
  0 siblings, 1 reply; 8+ messages in thread
From: Jonathan Wakely @ 2016-07-12 22:31 UTC (permalink / raw)
  To: Ville Voutilainen; +Cc: gcc-patches, libstdc++

On 11/07/16 23:41 +0300, Ville Voutilainen wrote:
>@@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 	}
>     };
>
>+  template<typename _Tp>
>+    using __optional_relop_t =
>+    enable_if_t<is_constructible<bool, _Tp>::value, bool>;

Should this be is_convertible<_Tp, bool> instead?

>   template<typename _Tp>
>-    constexpr bool
>+    constexpr auto
>     operator!=(const optional<_Tp>& __lhs, _Tp const& __rhs)

Dunno why this has _Tp const& rather than const _Tp&, could you fix it
while you're in the file anyway? It's a bit confusing to have one
place using a different style.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
  2016-07-12 22:31 ` Jonathan Wakely
@ 2016-07-13 10:06   ` Ville Voutilainen
  2016-07-13 10:25     ` Ville Voutilainen
                       ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ville Voutilainen @ 2016-07-13 10:06 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches, libstdc++

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

On 13 July 2016 at 01:31, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 11/07/16 23:41 +0300, Ville Voutilainen wrote:
>>
>> @@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>         }
>>     };
>>
>> +  template<typename _Tp>
>> +    using __optional_relop_t =
>> +    enable_if_t<is_constructible<bool, _Tp>::value, bool>;
>
>
> Should this be is_convertible<_Tp, bool> instead?

Yeah.. it would be more reasonable to return a type explicitly
convertible to bool from a relop
if a non-bool is returned, but since built-in operators are not
contextually-convertible-to-bool,
that "protection" wouldn't buy much. And since the implementation
doesn't really do bool-wrappings
everywhere, let's go with is_convertible and change that if someone complains.

>
>>   template<typename _Tp>
>> -    constexpr bool
>> +    constexpr auto
>>     operator!=(const optional<_Tp>& __lhs, _Tp const& __rhs)
>
>
> Dunno why this has _Tp const& rather than const _Tp&, could you fix it
> while you're in the file anyway? It's a bit confusing to have one
> place using a different style.

Ha, that was indeed in just one place.

I made the above changes and also made converting assignment operators
SFINAE. That SFINAE
seems consistent with how constructors and relops work. And yes, there
are still some members like
emplace that static_assert rather than SFINAE, but I think that's ok for now.

2016-07-13  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Implement P0307R2, Making Optional Greater Equal Again.
    * include/std/optional (operator=(_Up&&)): Constrain.
    (operator=(const optional<_Up>&)): Likewise.
    (operator=(optional<_Up>&&)): Likewise.
    (__optional_relop_t): New.
    (operator==(const optional<_Tp>&, const optional<_Tp>&)): Constrain.
    (operator!=(const optional<_Tp>&, const optional<_Tp>&)):
    Constrain and make transparent.
    (operator<(const optional<_Tp>&, const optional<_Tp>&)): Constrain.
    (operator>(const optional<_Tp>&, const optional<_Tp>&)):
    Constrain and make transparent.
    (operator<=(const optional<_Tp>&, const optional<_Tp>&)): Likewise.
    (operator>=(const optional<_Tp>&, const optional<_Tp>&)): Likewise.
    (operator==(const optional<_Tp>&, const _Tp&): Constrain.
    (operator==(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator!=(const optional<_Tp>&, _Tp const&)):
    Constrain and make transparent.
    (operator!=(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator<(const optional<_Tp>&, const _Tp&)): Constrain.
    (operator<(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator>(const optional<_Tp>&, const _Tp&)):
    Constrain and make transparent.
    (operator>(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator<=(const optional<_Tp>&, const _Tp&)): Likewise.
    (operator<=(const _Tp&, const optional<_Tp>&)): Likewise.
    (operator>=(const optional<_Tp>&, const _Tp&)): Likewise.
    (operator>=(const _Tp&, const optional<_Tp>&)): Likewise.
    * testsuite/20_util/optional/constexpr/relops/2.cc: Adjust.
    * testsuite/20_util/optional/constexpr/relops/4.cc: Likewise.
    * testsuite/20_util/optional/relops/1.cc: Likewise.
    * testsuite/20_util/optional/relops/2.cc: Likewise.
    * testsuite/20_util/optional/relops/3.cc: Likewise.
    * testsuite/20_util/optional/relops/4.cc: Likewise.
    * testsuite/20_util/optional/requirements.cc: Add tests to verify
    that optional's relops are transparent and don't synthesize
    operators. Also test that assignment sfinaes.

[-- Attachment #2: P0307R2_2.diff --]
[-- Type: text/plain, Size: 17452 bytes --]

diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index e9a86a4..45929c7 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -132,7 +132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     *
     * Practically speaking this detects the presence of such an operator when
     * called on a const-qualified lvalue (i.e.
-    * declval<_Tp * const&>().operator&()).
+    * declval<const _Tp *&>().operator&()).
     */
   template<typename _Tp>
     struct _Has_addressof
@@ -577,16 +577,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       template<typename _Up,
                enable_if_t<__and_<
+			   is_constructible<_Tp, _Up>,
+			   is_assignable<_Tp&, _Up>,
 			   __not_<is_same<_Up, nullopt_t>>,
 			   __not_<__is_optional<_Up>>>::value,
 			 bool> = true>
         optional&
         operator=(_Up&& __u)
         {
-          static_assert(__and_<is_constructible<_Tp, _Up>,
-			       is_assignable<_Tp&, _Up>>(),
-                        "Cannot assign to value type from argument");
-
           if (this->_M_is_engaged())
             this->_M_get() = std::forward<_Up>(__u);
           else
@@ -597,15 +595,13 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       template<typename _Up,
                enable_if_t<__and_<
-		 __not_<is_same<_Tp, _Up>>>::value,
-			   bool> = true>
+		           is_constructible<_Tp, _Up>,
+		           is_assignable<_Tp&, _Up>,
+		           __not_<is_same<_Tp, _Up>>>::value,
+			 bool> = true>
         optional&
         operator=(const optional<_Up>& __u)
         {
-          static_assert(__and_<is_constructible<_Tp, _Up>,
-			       is_assignable<_Tp&, _Up>>(),
-                        "Cannot assign to value type from argument");
-
           if (__u)
             {
               if (this->_M_is_engaged())
@@ -621,16 +617,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         }
 
       template<typename _Up,
-               enable_if_t<__and_<
-		 __not_<is_same<_Tp, _Up>>>::value,
-			   bool> = true>
+	       enable_if_t<__and_<
+			   is_constructible<_Tp, _Up>,
+			   is_assignable<_Tp&, _Up>,
+			   __not_<is_same<_Tp, _Up>>>::value,
+			 bool> = true>
         optional&
         operator=(optional<_Up>&& __u)
         {
-          static_assert(__and_<is_constructible<_Tp, _Up>,
-			       is_assignable<_Tp&, _Up>>(),
-                        "Cannot assign to value type from argument");
-
           if (__u)
             {
               if (this->_M_is_engaged())
@@ -785,41 +779,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 	}
     };
 
+  template<typename _Tp>
+    using __optional_relop_t =
+    enable_if_t<is_convertible<_Tp, bool>::value, bool>;
+
   // [X.Y.8] Comparisons between optional values.
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator==(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Tp>())>
     {
       return static_cast<bool>(__lhs) == static_cast<bool>(__rhs)
 	     && (!__lhs || *__lhs == *__rhs);
     }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator!=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return !(__lhs == __rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Tp>())>
+    {
+      return static_cast<bool>(__lhs) != static_cast<bool>(__rhs)
+	|| (static_cast<bool>(__lhs) && *__lhs != *__rhs);
+    }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Tp>())>
     {
       return static_cast<bool>(__rhs) && (!__lhs || *__lhs < *__rhs);
     }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return __rhs < __lhs; }
+    -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Tp>())>
+    {
+      return static_cast<bool>(__lhs) && (!__rhs || *__lhs > *__rhs);
+    }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return !(__rhs < __lhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Tp>())>
+    {
+      return !__lhs || (static_cast<bool>(__rhs) && *__lhs <= *__rhs);
+    }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>=(const optional<_Tp>& __lhs, const optional<_Tp>& __rhs)
-    { return !(__lhs < __rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Tp>())>
+    {
+      return !__rhs || (static_cast<bool>(__lhs) && *__lhs >= *__rhs);
+    }
 
   // [X.Y.9] Comparisons with nullopt.
   template<typename _Tp>
@@ -884,64 +897,76 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // [X.Y.10] Comparisons with value type.
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator==(const optional<_Tp>& __lhs, const _Tp& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Tp>())>
     { return __lhs && *__lhs == __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator==(const _Tp& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() == declval<_Tp>())>
     { return __rhs && __lhs == *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
-    operator!=(const optional<_Tp>& __lhs, _Tp const& __rhs)
-    { return !__lhs || !(*__lhs == __rhs); }
+    constexpr auto
+    operator!=(const optional<_Tp>& __lhs, const _Tp& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Tp>())>
+    { return !__lhs || *__lhs != __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator!=(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return !__rhs || !(__lhs == *__rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() != declval<_Tp>())>
+    { return !__rhs || __lhs != *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<(const optional<_Tp>& __lhs, const _Tp& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Tp>())>
     { return !__lhs || *__lhs < __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<(const _Tp& __lhs, const optional<_Tp>& __rhs)
+    -> __optional_relop_t<decltype(declval<_Tp>() < declval<_Tp>())>
     { return __rhs && __lhs < *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>(const optional<_Tp>& __lhs, const _Tp& __rhs)
-    { return __lhs && __rhs < *__lhs; }
+    -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Tp>())>
+    { return __lhs && *__lhs > __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return !__rhs || *__rhs < __lhs; }
+    -> __optional_relop_t<decltype(declval<_Tp>() > declval<_Tp>())>
+    { return !__rhs || __lhs > *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<=(const optional<_Tp>& __lhs, const _Tp& __rhs)
-    { return !__lhs || !(__rhs < *__lhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Tp>())>
+    { return !__lhs || *__lhs <= __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator<=(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return __rhs && !(*__rhs < __lhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() <= declval<_Tp>())>
+    { return __rhs && __lhs <= *__rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>=(const optional<_Tp>& __lhs, const _Tp& __rhs)
-    { return __lhs && !(*__lhs < __rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Tp>())>
+    { return __lhs && *__lhs >= __rhs; }
 
   template<typename _Tp>
-    constexpr bool
+    constexpr auto
     operator>=(const _Tp& __lhs, const optional<_Tp>& __rhs)
-    { return !__rhs || !(__lhs < *__rhs); }
+    -> __optional_relop_t<decltype(declval<_Tp>() >= declval<_Tp>())>
+    { return !__rhs || __lhs >= *__rhs; }
 
   // [X.Y.11]
   template<typename _Tp>
diff --git a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc
index 9aa9273..0ce00c1 100644
--- a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/2.cc
@@ -54,6 +54,18 @@ namespace ns
   operator<(value_type const& lhs, value_type const& rhs)
   { return (lhs.i < rhs.i) || (!(rhs.i < lhs.i) && strrel(lhs.s, rhs.s)); }
 
+  constexpr bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return rhs < lhs; }
+
+  constexpr bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return lhs < rhs || lhs == rhs; }
+
+  constexpr bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return lhs > rhs || lhs == rhs; }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc
index 15130d4..d6294ad 100644
--- a/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/constexpr/relops/4.cc
@@ -54,6 +54,18 @@ namespace ns
   operator<(value_type const& lhs, value_type const& rhs)
   { return (lhs.i < rhs.i) || (!(rhs.i < lhs.i) && strrel(lhs.s, rhs.s)); }
 
+  constexpr bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return rhs < lhs; }
+
+  constexpr bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return lhs < rhs || lhs == rhs; }
+
+  constexpr bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return lhs > rhs || lhs == rhs; }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/1.cc b/libstdc++-v3/testsuite/20_util/optional/relops/1.cc
index 6277032..1315902 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/1.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/1.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/2.cc b/libstdc++-v3/testsuite/20_util/optional/relops/2.cc
index 65071c0..1351265 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/2.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/2.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/3.cc b/libstdc++-v3/testsuite/20_util/optional/relops/3.cc
index 2fd9e8b..95fde3b 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/3.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/3.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/4.cc b/libstdc++-v3/testsuite/20_util/optional/relops/4.cc
index 363e633..78d0eb8 100644
--- a/libstdc++-v3/testsuite/20_util/optional/relops/4.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/4.cc
@@ -37,9 +37,25 @@ namespace ns
   { return std::tie(lhs.i, lhs.s) == std::tie(rhs.i, rhs.s); }
 
   bool
+  operator!=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) != std::tie(rhs.i, rhs.s); }
+
+  bool
   operator<(value_type const& lhs, value_type const& rhs)
   { return std::tie(lhs.i, lhs.s) < std::tie(rhs.i, rhs.s); }
 
+  bool
+  operator>(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) > std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator<=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) <= std::tie(rhs.i, rhs.s); }
+
+  bool
+  operator>=(value_type const& lhs, value_type const& rhs)
+  { return std::tie(lhs.i, lhs.s) >= std::tie(rhs.i, rhs.s); }
+
 } // namespace ns
 
 int main()
diff --git a/libstdc++-v3/testsuite/20_util/optional/requirements.cc b/libstdc++-v3/testsuite/20_util/optional/requirements.cc
index aab572f..d0f3ab6 100644
--- a/libstdc++-v3/testsuite/20_util/optional/requirements.cc
+++ b/libstdc++-v3/testsuite/20_util/optional/requirements.cc
@@ -257,3 +257,82 @@ int main()
     static_assert( *o == 33, "" );
   }
 }
+
+using std::void_t;
+using std::declval;
+using std::true_type;
+using std::false_type;
+
+template <class T, class = void>
+struct is_eq_comparable : false_type {};
+template <class T>
+struct is_eq_comparable<T, void_t<decltype(declval<T>() == declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_neq_comparable : false_type {};
+template <class T>
+struct is_neq_comparable<T, void_t<decltype(declval<T>() != declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_lt_comparable : false_type {};
+template <class T>
+struct is_lt_comparable<T, void_t<decltype(declval<T>() < declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_gt_comparable : false_type {};
+template <class T>
+struct is_gt_comparable<T, void_t<decltype(declval<T>() > declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_le_comparable : false_type {};
+template <class T>
+struct is_le_comparable<T, void_t<decltype(declval<T>() <= declval<T>())>>
+: true_type {};
+
+template <class T, class = void>
+struct is_ge_comparable : false_type {};
+template <class T>
+struct is_ge_comparable<T, void_t<decltype(declval<T>() >= declval<T>())>>
+: true_type {};
+
+using std::optional;
+
+static_assert(is_eq_comparable<optional<int>>::value, "");
+static_assert(is_neq_comparable<optional<int>>::value, "");
+static_assert(is_lt_comparable<optional<int>>::value, "");
+static_assert(is_gt_comparable<optional<int>>::value, "");
+static_assert(is_le_comparable<optional<int>>::value, "");
+static_assert(is_ge_comparable<optional<int>>::value, "");
+
+struct JustEq {};
+bool operator==(const JustEq&, const JustEq&);
+
+static_assert(is_eq_comparable<optional<JustEq>>::value, "");
+static_assert(!is_neq_comparable<optional<JustEq>>::value, "");
+static_assert(!is_lt_comparable<optional<JustEq>>::value, "");
+static_assert(!is_gt_comparable<optional<JustEq>>::value, "");
+static_assert(!is_le_comparable<optional<JustEq>>::value, "");
+static_assert(!is_ge_comparable<optional<JustEq>>::value, "");
+
+struct JustLt {};
+bool operator<(const JustLt&, const JustLt&);
+
+static_assert(!is_eq_comparable<optional<JustLt>>::value, "");
+static_assert(!is_neq_comparable<optional<JustLt>>::value, "");
+static_assert(is_lt_comparable<optional<JustLt>>::value, "");
+static_assert(!is_gt_comparable<optional<JustLt>>::value, "");
+static_assert(!is_le_comparable<optional<JustLt>>::value, "");
+static_assert(!is_ge_comparable<optional<JustLt>>::value, "");
+
+static_assert(!std::is_assignable<optional<JustEq>&,
+	      optional<JustLt>>::value, "");
+static_assert(!std::is_assignable<optional<JustEq>&,
+	      JustLt>::value, "");
+static_assert(!std::is_assignable<optional<JustEq>&,
+	      optional<JustLt>&>::value, "");
+static_assert(!std::is_assignable<optional<JustEq>&,
+	      JustLt&>::value, "");

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
  2016-07-13 10:06   ` Ville Voutilainen
@ 2016-07-13 10:25     ` Ville Voutilainen
  2016-07-13 10:43     ` Jonathan Wakely
  2016-07-13 18:25     ` Daniel Krügler
  2 siblings, 0 replies; 8+ messages in thread
From: Ville Voutilainen @ 2016-07-13 10:25 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches, libstdc++

On 13 July 2016 at 13:05, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
>> Dunno why this has _Tp const& rather than const _Tp&, could you fix it
>> while you're in the file anyway? It's a bit confusing to have one
>> place using a different style.
>
> Ha, that was indeed in just one place.


Well, technically two, the other was in a comment. The patch changes that, too.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
  2016-07-13 10:06   ` Ville Voutilainen
  2016-07-13 10:25     ` Ville Voutilainen
@ 2016-07-13 10:43     ` Jonathan Wakely
  2016-07-13 18:25     ` Daniel Krügler
  2 siblings, 0 replies; 8+ messages in thread
From: Jonathan Wakely @ 2016-07-13 10:43 UTC (permalink / raw)
  To: Ville Voutilainen; +Cc: gcc-patches, libstdc++

On 13/07/16 13:05 +0300, Ville Voutilainen wrote:
>Ha, that was indeed in just one place.

See below.

>I made the above changes and also made converting assignment operators
>SFINAE. That SFINAE
>seems consistent with how constructors and relops work. And yes, there
>are still some members like
>emplace that static_assert rather than SFINAE, but I think that's ok for now.
>    operators. Also test that assignment sfinaes.

OK.

>diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
>index e9a86a4..45929c7 100644
>--- a/libstdc++-v3/include/std/optional
>+++ b/libstdc++-v3/include/std/optional
>@@ -132,7 +132,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>     *
>     * Practically speaking this detects the presence of such an operator when
>     * called on a const-qualified lvalue (i.e.
>-    * declval<_Tp * const&>().operator&()).
>+    * declval<const _Tp *&>().operator&()).
>     */
>   template<typename _Tp>
>     struct _Has_addressof

That comment was wrong anyway, it says _Tp* const& when it should have
been _Tp const&.

declval<_Tp * const&>().operator&() doesn't make any sense. Not sure
why I've never spotted that until now.

Please change it to const _Tp& and change "i.e." to "e.g." (because
since my change last year it detects both members and non-members).

OK for trunk with that tweak, thanks.

I'll make the same change to the comment in <experimental/optional>.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
  2016-07-13 10:06   ` Ville Voutilainen
  2016-07-13 10:25     ` Ville Voutilainen
  2016-07-13 10:43     ` Jonathan Wakely
@ 2016-07-13 18:25     ` Daniel Krügler
  2016-07-13 18:32       ` Ville Voutilainen
  2 siblings, 1 reply; 8+ messages in thread
From: Daniel Krügler @ 2016-07-13 18:25 UTC (permalink / raw)
  To: Ville Voutilainen; +Cc: Jonathan Wakely, gcc-patches, libstdc++

2016-07-13 12:05 GMT+02:00 Ville Voutilainen <ville.voutilainen@gmail.com>:
> On 13 July 2016 at 01:31, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 11/07/16 23:41 +0300, Ville Voutilainen wrote:
>>>
>>> @@ -785,41 +785,60 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>         }
>>>     };
>>>
>>> +  template<typename _Tp>
>>> +    using __optional_relop_t =
>>> +    enable_if_t<is_constructible<bool, _Tp>::value, bool>;
>>
>>
>> Should this be is_convertible<_Tp, bool> instead?
>
> Yeah.. it would be more reasonable to return a type explicitly
> convertible to bool from a relop
> if a non-bool is returned, but since built-in operators are not
> contextually-convertible-to-bool,
> that "protection" wouldn't buy much. And since the implementation
> doesn't really do bool-wrappings
> everywhere, let's go with is_convertible and change that if someone complains.

How would you feel about the introduction of an internal trait
__is_boolean_testable, that would test both is_convertible<const T&,
bool> and is_constructible<bool, const T&> for now, so that we could
reuse that at places like these and others pointed out by LWG 2114?

If you like that idea, I would work on a contribution.

Thanks,

- Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
  2016-07-13 18:25     ` Daniel Krügler
@ 2016-07-13 18:32       ` Ville Voutilainen
  2016-07-16 16:55         ` Daniel Krügler
  0 siblings, 1 reply; 8+ messages in thread
From: Ville Voutilainen @ 2016-07-13 18:32 UTC (permalink / raw)
  To: Daniel Krügler; +Cc: Jonathan Wakely, gcc-patches, libstdc++

On 13 July 2016 at 21:25, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
> How would you feel about the introduction of an internal trait
> __is_boolean_testable, that would test both is_convertible<const T&,
> bool> and is_constructible<bool, const T&> for now, so that we could
> reuse that at places like these and others pointed out by LWG 2114?
>
> If you like that idea, I would work on a contribution.


Sounds like an excellent idea to me.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again.
  2016-07-13 18:32       ` Ville Voutilainen
@ 2016-07-16 16:55         ` Daniel Krügler
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Krügler @ 2016-07-16 16:55 UTC (permalink / raw)
  To: Ville Voutilainen; +Cc: Jonathan Wakely, gcc-patches, libstdc++

2016-07-13 20:32 GMT+02:00 Ville Voutilainen <ville.voutilainen@gmail.com>:
> On 13 July 2016 at 21:25, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
>> How would you feel about the introduction of an internal trait
>> __is_boolean_testable, that would test both is_convertible<const T&,
>> bool> and is_constructible<bool, const T&> for now, so that we could
>> reuse that at places like these and others pointed out by LWG 2114?
>>
>> If you like that idea, I would work on a contribution.
>
> Sounds like an excellent idea to me.

I have opened an enhancement request and have now a (actually two)
proposal(s) for this. I would appreciate if you could express your
opinion here:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71899#c1

Thanks,

- Daniel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-07-16 16:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-11 20:41 [v3 PATCH] Implement P0307R2, Making Optional Greater Equal Again Ville Voutilainen
2016-07-12 22:31 ` Jonathan Wakely
2016-07-13 10:06   ` Ville Voutilainen
2016-07-13 10:25     ` Ville Voutilainen
2016-07-13 10:43     ` Jonathan Wakely
2016-07-13 18:25     ` Daniel Krügler
2016-07-13 18:32       ` Ville Voutilainen
2016-07-16 16:55         ` Daniel Krügler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).