libstdc++: Implement LWG 4366 for std::expected comparisons

This modifies the equality comparisons for std::expected so that they do
not use explicit conversions to bool, to match the constraints which are
specified in terms of "convertible to" which implies implicitly
convertible. As a result of those changes, we cannot use logical
expressions with && or || that involve comparisons of the contained
values, because x && (*y == *z) might do the wrong thing if *y == *z
does not return bool.

Also add [[nodiscard]] attributes which were missing.

The new lwg4366.cc testcase is a dg-do run test not dg-do compile,
because the original example won't compile with libstdc++ even after
these fixes. We constrain the std::expected comparison operators with
std::convertible_to<bool> and the pathological Bool type in the issue
doesn't satisfy that concept. So the new test replaces the deleted
explicit conversion oeprator in the issue with one that isn't deleted
but terminates if called. This ensures we don't call it, thus ensuring
that std::expected's comparisons do implicit conversions only.

It's unclear to me whether using the convertible_to concept in
std::expected comparisons is conforming, or if we should switch to an
__implicitly_convertible_to<bool> concept which only uses
std::is_convertible_v<T, bool> and doesn't check for explicit
conversions. That can be addressed separately from this change.

libstdc++-v3/ChangeLog:

	* include/std/expected (operator==): Use implicit conversion to
	bool and do not use logical && and || with operands of unknown
	types. Add nodiscard attributes.
	* testsuite/20_util/expected/equality.cc: Test some missing
	cases which were not covered previously.
	* testsuite/20_util/expected/lwg4366.cc: New test.

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

diff --git a/libstdc++-v3/include/std/expected b/libstdc++-v3/include/std/expected
index 591fc72a43887dc269b7db65d9eb08c1015b3ee1..a03a7d3f722f460252f485d2c938128fc8d1d449 100644 (file)
--- a/libstdc++-v3/include/std/expected
+++ b/libstdc++-v3/include/std/expected
@@ -1163,15 +1163,17 @@ namespace __expected
            { __t == __u } -> convertible_to<bool>;
            { __e == __e2 } -> convertible_to<bool>;
          }
+       [[nodiscard]]
        friend constexpr bool
        operator==(const expected& __x, const expected<_Up, _Er2>& __y)
        noexcept(noexcept(bool(*__x == *__y))
                  && noexcept(bool(__x.error() == __y.error())))
        {
+         if (__x.has_value() != __y.has_value())
+           return false;
          if (__x.has_value())
-           return __y.has_value() && bool(*__x == *__y);
-         else
-           return !__y.has_value() && bool(__x.error() == __y.error());
+           return *__x == *__y;
+         return __x.error() == __y.error();
        }
 
       template<typename _Up, same_as<_Tp> _Vp>
@@ -1179,19 +1181,29 @@ namespace __expected
          && requires (const _Tp& __t, const _Up& __u) {
            { __t == __u } -> convertible_to<bool>;
          }
+       [[nodiscard]]
        friend constexpr bool
        operator==(const expected<_Vp, _Er>& __x, const _Up& __v)
        noexcept(noexcept(bool(*__x == __v)))
-       { return __x.has_value() && bool(*__x == __v); }
+       {
+         if (__x.has_value())
+           return *__x == __v;
+         return false;
+       }
 
       template<typename _Er2>
        requires requires (const _Er& __e, const _Er2& __e2) {
          { __e == __e2 } -> convertible_to<bool>;
        }
+       [[nodiscard]]
        friend constexpr bool
        operator==(const expected& __x, const unexpected<_Er2>& __e)
        noexcept(noexcept(bool(__x.error() == __e.error())))
-       { return !__x.has_value() && bool(__x.error() == __e.error()); }
+       {
+         if (!__x.has_value())
+           return __x.error() == __e.error();
+         return false;
+       }
 
       friend constexpr void
       swap(expected& __x, expected& __y)
@@ -1841,24 +1853,31 @@ namespace __expected
          && requires (const _Er& __e, const _Er2& __e2) {
            { __e == __e2 } -> convertible_to<bool>;
          }
+       [[nodiscard]]
        friend constexpr bool
        operator==(const expected& __x, const expected<_Up, _Er2>& __y)
        noexcept(noexcept(bool(__x.error() == __y.error())))
        {
+         if (__x.has_value() != __y.has_value())
+           return false;
          if (__x.has_value())
-           return __y.has_value();
-         else
-           return !__y.has_value() && bool(__x.error() == __y.error());
+           return true;
+         return __x.error() == __y.error();
        }
 
       template<typename _Er2>
        requires requires (const _Er& __e, const _Er2& __e2) {
          { __e == __e2 } -> convertible_to<bool>;
        }
+       [[nodiscard]]
        friend constexpr bool
        operator==(const expected& __x, const unexpected<_Er2>& __e)
        noexcept(noexcept(bool(__x.error() == __e.error())))
-       { return !__x.has_value() && bool(__x.error() == __e.error()); }
+       {
+         if (!__x.has_value())
+           return __x.error() == __e.error();
+         return false;
+       }
 
       friend constexpr void
       swap(expected& __x, expected& __y)

diff --git a/libstdc++-v3/testsuite/20_util/expected/equality.cc b/libstdc++-v3/testsuite/20_util/expected/equality.cc
index db19b1510a73e43ba44f10f4b17ed691b4e6849b..cc122f4690820cce7ad894c465326daaf8c15460 100644 (file)
--- a/libstdc++-v3/testsuite/20_util/expected/equality.cc
+++ b/libstdc++-v3/testsuite/20_util/expected/equality.cc
@@ -28,19 +28,45 @@ test_eq()
   std::expected<int, int> e2;
   VERIFY(e2 == e2);
   VERIFY(e1 == e2);
+  VERIFY(e2 == e1);
   VERIFY(e1 != std::unexpected<int>(1));
+
   e1 = std::unexpected<int>(1);
   VERIFY(e1 == std::unexpected<int>(1));
   VERIFY(e1 != std::unexpected<int>(2));
   VERIFY(e1 != e2);
+  VERIFY(e2 != e1);
+  VERIFY(e1 != 1);
+
+  e2 = std::unexpected<int>(1);
+  VERIFY(e1 == e2);
+  VERIFY(e2 == e1);
+
+  e2 = std::unexpected<int>(2);
+  VERIFY(e1 != e2);
+  VERIFY(e2 != e1);
 
   std::expected<void, int> e3;
   VERIFY(e3 == e3);
   VERIFY(e3 != std::unexpected<int>(1));
+  std::expected<const void, long> e4;
+  VERIFY(e3 == e4);
+  VERIFY(e4 == e3);
+
   e3 = std::unexpected<int>(1);
   VERIFY(e3 == e3);
   VERIFY(e3 == std::unexpected<int>(1));
   VERIFY(e3 != std::unexpected<int>(2));
+  VERIFY(e3 != e4);
+  VERIFY(e4 != e3);
+
+  e4 = e3;
+  VERIFY(e3 == e4);
+  VERIFY(e4 == e3);
+
+  e4 = std::unexpected<int>(4);
+  VERIFY(e3 != e4);
+  VERIFY(e4 != e3);
 
   return true;
 }

diff --git a/libstdc++-v3/testsuite/20_util/expected/lwg4366.cc b/libstdc++-v3/testsuite/20_util/expected/lwg4366.cc
new file mode 100644 (file)
index 0000000..35d5371
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/expected/lwg4366.cc
@@ -0,0 +1,33 @@
+// { dg-do run { target c++23 } }
+
+// LWG 4366. Heterogeneous comparison of expected may be ill-formed
+
+#include <expected>
+#include <testsuite_hooks.h>
+
+struct Bool
+{
+  operator bool() const { return true; }
+  explicit operator bool() { throw; }
+};
+
+struct E1 {
+  friend Bool operator==(E1, E1) { return {}; }
+} e1;
+
+struct E2 {
+  friend Bool operator==(E1, E2) { return {}; }
+} e2;
+
+int main()
+{
+  std::expected<int, E1> u1(std::unexpect, e1);
+  VERIFY(u1 == u1);
+
+  std::unexpected<E2> u2(e2);
+  VERIFY(u1 == u2);
+
+  std::expected<void, E1> u3(std::unexpect, e1);
+  VERIFY(u3 == u3);
+  VERIFY(u3 == u2);
+}