On Fri, 21 Nov 2025 at 10:02, Tomasz Kaminski <[email protected]> wrote:
>
>
>
> 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.
Yeah, maybe we should just open a library issue and then close it as
"NAD - using convertible_to is allowed, but not required. Only silly
programs care about the difference between the trait and the concept".
>>
>>
>> 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
>>
