On Thu, Nov 20, 2025 at 6:02 PM Jonathan Wakely <[email protected]> wrote:
> 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.
> ---
>
> Tested x86_64-linux.
>
LGTM
>
> libstdc++-v3/include/std/optional | 110 ++++++++++++++----
> .../20_util/optional/relops/lwg4370.cc | 55 +++++++++
> 2 files changed, 145 insertions(+), 20 deletions(-)
> create mode 100644
> libstdc++-v3/testsuite/20_util/optional/relops/lwg4370.cc
>
> diff --git a/libstdc++-v3/include/std/optional
> b/libstdc++-v3/include/std/optional
> index 7d5af1936617..39db8d3d4a0f 100644
> --- a/libstdc++-v3/include/std/optional
> +++ b/libstdc++-v3/include/std/optional
> @@ -1858,8 +1858,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>
> @@ -1867,8 +1870,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>
> @@ -1876,7 +1882,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>
> @@ -1884,7 +1894,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>
> @@ -1892,7 +1906,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>
> @@ -1900,7 +1918,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
> @@ -1997,84 +2019,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
> index 000000000000..a292cbe6b404
> --- /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);
> +}
> +
> +
> --
> 2.51.1
>
>
