libstdc++: Implement LWG 4370 for std::optional comparisons

This modifies the relational comparisons for std::optional so that they
do not use logical expressions with && or || that involve the
comparisons on the contained values, because x && (*y == *z) might do
the wrong thing if *y == *z does not return bool.

libstdc++-v3/ChangeLog:

	* include/std/optional (operator==, operator!=, operator>)
	(operator>, operator<=, operator>=): Do not use logical
	&& and || with operands of unknown types.
	* testsuite/20_util/optional/relops/lwg4370.cc: New test.

Reviewed-by: Tomasz Kamiński <tkaminsk@redhat.com>

diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index f54dda6cd2378cb31e1b3ac4e29383d34ff28399..8b9fa9244213e735cd27166803f4789e3db25c18 100644 (file)
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -1855,8 +1855,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator==(const optional<_Tp>& __lhs, const optional<_Up>& __rhs)
     -> __optional_eq_t<_Tp, _Up>
     {
-      return static_cast<bool>(__lhs) == static_cast<bool>(__rhs)
-            && (!__lhs || *__lhs == *__rhs);
+      if (__lhs.has_value() != __rhs.has_value())
+       return false;
+      if (!__lhs.has_value())
+       return true;
+      return *__lhs == *__rhs;
     }
 
   template<typename _Tp, typename _Up>
@@ -1864,8 +1867,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator!=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs)
     -> __optional_ne_t<_Tp, _Up>
     {
-      return static_cast<bool>(__lhs) != static_cast<bool>(__rhs)
-       || (static_cast<bool>(__lhs) && *__lhs != *__rhs);
+      if (__lhs.has_value() != __rhs.has_value())
+       return true;
+      if (!__lhs.has_value())
+       return false;
+      return *__lhs != *__rhs;
     }
 
   template<typename _Tp, typename _Up>
@@ -1873,7 +1879,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator<(const optional<_Tp>& __lhs, const optional<_Up>& __rhs)
     -> __optional_lt_t<_Tp, _Up>
     {
-      return static_cast<bool>(__rhs) && (!__lhs || *__lhs < *__rhs);
+      if (!__rhs.has_value())
+       return false;
+      if (!__lhs.has_value())
+       return true;
+      return *__lhs < *__rhs;
     }
 
   template<typename _Tp, typename _Up>
@@ -1881,7 +1891,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator>(const optional<_Tp>& __lhs, const optional<_Up>& __rhs)
     -> __optional_gt_t<_Tp, _Up>
     {
-      return static_cast<bool>(__lhs) && (!__rhs || *__lhs > *__rhs);
+      if (!__lhs.has_value())
+       return false;
+      if (!__rhs.has_value())
+       return true;
+      return *__lhs > *__rhs;
     }
 
   template<typename _Tp, typename _Up>
@@ -1889,7 +1903,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator<=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs)
     -> __optional_le_t<_Tp, _Up>
     {
-      return !__lhs || (static_cast<bool>(__rhs) && *__lhs <= *__rhs);
+      if (!__lhs.has_value())
+       return true;
+      if (!__rhs.has_value())
+       return false;
+      return *__lhs <= *__rhs;
     }
 
   template<typename _Tp, typename _Up>
@@ -1897,7 +1915,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     operator>=(const optional<_Tp>& __lhs, const optional<_Up>& __rhs)
     -> __optional_ge_t<_Tp, _Up>
     {
-      return !__rhs || (static_cast<bool>(__lhs) && *__lhs >= *__rhs);
+      if (!__rhs.has_value())
+       return true;
+      if (!__lhs.has_value())
+       return false;
+      return *__lhs >= *__rhs;
     }
 
 #ifdef __cpp_lib_three_way_comparison
@@ -1994,84 +2016,132 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     constexpr auto
     operator== [[nodiscard]] (const optional<_Tp>& __lhs, const _Up& __rhs)
     -> __optional_eq_t<_Tp, _Up>
-    { return __lhs && *__lhs == __rhs; }
+    {
+      if (__lhs.has_value())
+       return *__lhs == __rhs;
+      return false;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Tp)
     constexpr auto
     operator== [[nodiscard]] (const _Tp& __lhs, const optional<_Up>& __rhs)
     -> __optional_eq_t<_Tp, _Up>
-    { return __rhs && __lhs == *__rhs; }
+    {
+      if (__rhs.has_value())
+       return __lhs == *__rhs;
+      return false;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Up)
     constexpr auto
     operator!= [[nodiscard]] (const optional<_Tp>& __lhs, const _Up& __rhs)
     -> __optional_ne_t<_Tp, _Up>
-    { return !__lhs || *__lhs != __rhs; }
+    {
+      if (__lhs.has_value())
+       return *__lhs != __rhs;
+      return true;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Tp)
     constexpr auto
     operator!= [[nodiscard]] (const _Tp& __lhs, const optional<_Up>& __rhs)
     -> __optional_ne_t<_Tp, _Up>
-    { return !__rhs || __lhs != *__rhs; }
+    {
+      if (__rhs.has_value())
+       return __lhs != *__rhs;
+      return true;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Up)
     constexpr auto
     operator< [[nodiscard]] (const optional<_Tp>& __lhs, const _Up& __rhs)
     -> __optional_lt_t<_Tp, _Up>
-    { return !__lhs || *__lhs < __rhs; }
+    {
+      if (__lhs.has_value())
+       return *__lhs < __rhs;
+      return true;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Tp)
     constexpr auto
     operator< [[nodiscard]] (const _Tp& __lhs, const optional<_Up>& __rhs)
     -> __optional_lt_t<_Tp, _Up>
-    { return __rhs && __lhs < *__rhs; }
+    {
+      if (__rhs.has_value())
+       return __lhs < *__rhs;
+      return false;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Up)
     constexpr auto
     operator> [[nodiscard]] (const optional<_Tp>& __lhs, const _Up& __rhs)
     -> __optional_gt_t<_Tp, _Up>
-    { return __lhs && *__lhs > __rhs; }
+    {
+      if (__lhs.has_value())
+       return *__lhs > __rhs;
+      return false;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Tp)
     constexpr auto
     operator> [[nodiscard]] (const _Tp& __lhs, const optional<_Up>& __rhs)
     -> __optional_gt_t<_Tp, _Up>
-    { return !__rhs || __lhs > *__rhs; }
+    {
+      if (__rhs.has_value())
+       return __lhs > *__rhs;
+      return true;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Up)
     constexpr auto
     operator<= [[nodiscard]] (const optional<_Tp>& __lhs, const _Up& __rhs)
     -> __optional_le_t<_Tp, _Up>
-    { return !__lhs || *__lhs <= __rhs; }
+    {
+      if (__lhs.has_value())
+       return *__lhs <= __rhs;
+      return true;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Tp)
     constexpr auto
     operator<= [[nodiscard]] (const _Tp& __lhs, const optional<_Up>& __rhs)
     -> __optional_le_t<_Tp, _Up>
-    { return __rhs && __lhs <= *__rhs; }
+    {
+      if (__rhs.has_value())
+       return __lhs <= *__rhs;
+      return false;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Up)
     constexpr auto
     operator>= [[nodiscard]] (const optional<_Tp>& __lhs, const _Up& __rhs)
     -> __optional_ge_t<_Tp, _Up>
-    { return __lhs && *__lhs >= __rhs; }
+    {
+      if (__lhs.has_value())
+       return *__lhs >= __rhs;
+      return false;
+    }
 
   template<typename _Tp, typename _Up>
     _REQUIRES_NOT_OPTIONAL(_Tp)
     constexpr auto
     operator>= [[nodiscard]] (const _Tp& __lhs, const optional<_Up>& __rhs)
     -> __optional_ge_t<_Tp, _Up>
-    { return !__rhs || __lhs >= *__rhs; }
+    {
+      if (__rhs.has_value())
+       return __lhs >= *__rhs;
+      return true;
+    }
 
 #ifdef __cpp_lib_three_way_comparison
   // _GLIBCXX_RESOLVE_LIB_DEFECTS

diff --git a/libstdc++-v3/testsuite/20_util/optional/relops/lwg4370.cc b/libstdc++-v3/testsuite/20_util/optional/relops/lwg4370.cc
new file mode 100644 (file)
index 0000000..a292cbe
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/relops/lwg4370.cc
@@ -0,0 +1,55 @@
+// { dg-do compile { target c++17 } }
+
+// LWG 4370. Comparison of optional<T> to T may be ill-formed
+
+#include <optional>
+#include <testsuite_hooks.h>
+
+struct Bool
+{
+  Bool(bool);
+  operator bool() const;
+};
+
+template<typename T> void operator&&(Bool, T) = delete;
+template<typename T> void operator&&(T, Bool) = delete;
+template<typename T> void operator||(Bool, T) = delete;
+template<typename T> void operator||(T, Bool) = delete;
+
+struct S
+{
+  Bool operator==(S) const;
+  Bool operator!=(S) const;
+  Bool operator<(S) const;
+  Bool operator>(S) const;
+  Bool operator<=(S) const;
+  Bool operator>=(S) const;
+};
+
+void
+test_lwg4370()
+{
+  std::optional<S> o;
+  (void)(o == o);
+  (void)(o != o);
+  (void)(o < o);
+  (void)(o > o);
+  (void)(o <= o);
+  (void)(o >= o);
+
+  S s;
+  (void)(o == s);
+  (void)(s == o);
+  (void)(o != s);
+  (void)(s != o);
+  (void)(o < s);
+  (void)(s < o);
+  (void)(o > s);
+  (void)(s > o);
+  (void)(o <= s);
+  (void)(s <= o);
+  (void)(o >= s);
+  (void)(s >= o);
+}
+
+