On Thu, Nov 20, 2025 at 6:01 PM Jonathan Wakely <[email protected]> wrote:
> 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.
>
I think it should be conforming, and if it's not we should change the
standard
and not. So totally agree on a separate issue.
>
> 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.
> ---
>
> Tested x86_64-linux.
>
LGTM
>
> libstdc++-v3/include/std/expected | 37 ++++++++++++++-----
> .../testsuite/20_util/expected/equality.cc | 26 +++++++++++++
> .../testsuite/20_util/expected/lwg4366.cc | 33 +++++++++++++++++
> 3 files changed, 87 insertions(+), 9 deletions(-)
> create mode 100644 libstdc++-v3/testsuite/20_util/expected/lwg4366.cc
>
> diff --git a/libstdc++-v3/include/std/expected
> b/libstdc++-v3/include/std/expected
> index 591fc72a4388..a03a7d3f722f 100644
> --- 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)
>
We
> 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 db19b1510a73..cc122f469082 100644
> --- 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
> index 000000000000..35d53714f036
> --- /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);
> +}
> --
> 2.51.1
>
>
