For title say "LWG 4406 and LWG 3424", and update description paragraph, to clarify which issue paragraph applies to which.
There is also suggestions to remove "else" in expected, otherwise the patch LGTM. On Thu, Nov 20, 2025 at 6:02 PM Jonathan Wakely <[email protected]> wrote: > This adjusts the return statements of optional::value_or and > expected::value_or to not perform explicit conversions, so that the > actual conversion performed matches the requirements expressed in the > Mandates: elements. > > Also adjust the return types to remove cv-qualifiers (LWG 3424). > > libstdc++-v3/ChangeLog: > > * include/std/expected (expected::value_or): Use remove_cv_t for > the return type. Do not use static_cast for return statement. > Adjust static_assert conditions to match return statements. > * include/std/optional (optional::value_or): Likewise. > (optional<T&>::value_or): Likewise. > --- > > Tested x86_64-linux. > > libstdc++-v3/include/std/expected | 18 +++++++++++------- > libstdc++-v3/include/std/optional | 27 +++++++++++++++++---------- > 2 files changed, 28 insertions(+), 17 deletions(-) > > diff --git a/libstdc++-v3/include/std/expected > b/libstdc++-v3/include/std/expected > index 4eaaab693e1e..591fc72a4388 100644 > --- a/libstdc++-v3/include/std/expected > +++ b/libstdc++-v3/include/std/expected > @@ -822,32 +822,36 @@ namespace __expected > return std::move(_M_unex); > } > > + // _GLIBCXX_RESOLVE_LIB_DEFECTS > + // 4406. value_or return statement is inconsistent with Mandates > template<typename _Up = remove_cv_t<_Tp>> > - constexpr _Tp > + constexpr remove_cv_t<_Tp> > value_or(_Up&& __v) const & > noexcept(__and_v<is_nothrow_copy_constructible<_Tp>, > is_nothrow_convertible<_Up, _Tp>>) > { > - static_assert( is_copy_constructible_v<_Tp> ); > + using _Xp = remove_cv_t<_Tp>; > + static_assert( is_convertible_v<const _Tp&, _Xp> ); > static_assert( is_convertible_v<_Up, _Tp> ); > > if (_M_has_value) > return _M_val; > - return static_cast<_Tp>(std::forward<_Up>(__v)); > + return std::forward<_Up>(__v); > } > > template<typename _Up = remove_cv_t<_Tp>> > - constexpr _Tp > + constexpr remove_cv_t<_Tp> > value_or(_Up&& __v) && > noexcept(__and_v<is_nothrow_move_constructible<_Tp>, > is_nothrow_convertible<_Up, _Tp>>) > { > - static_assert( is_move_constructible_v<_Tp> ); > - static_assert( is_convertible_v<_Up, _Tp> ); > + using _Xp = remove_cv_t<_Tp>; > + static_assert( is_convertible_v<_Tp, _Xp> ); > + static_assert( is_convertible_v<_Up, _Xp> ); > > if (_M_has_value) > return std::move(_M_val); > - return static_cast<_Tp>(std::forward<_Up>(__v)); > + return std::forward<_Up>(__v); > } > > template<typename _Gr = _Er> > diff --git a/libstdc++-v3/include/std/optional > b/libstdc++-v3/include/std/optional > index 41c04b10720e..7d5af1936617 100644 > --- a/libstdc++-v3/include/std/optional > +++ b/libstdc++-v3/include/std/optional > @@ -1285,30 +1285,34 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > __throw_bad_optional_access(); > } > > + // _GLIBCXX_RESOLVE_LIB_DEFECTS > + // 4406. value_or return statement is inconsistent with Mandates > template<typename _Up = remove_cv_t<_Tp>> > - constexpr _Tp > + constexpr remove_cv_t<_Tp> > value_or(_Up&& __u) const& > { > - static_assert(is_copy_constructible_v<_Tp>); > - static_assert(is_convertible_v<_Up&&, _Tp>); > + using _Xp = remove_cv_t<_Tp>; > + static_assert(is_convertible_v<const _Tp&, _Xp>); > + static_assert(is_convertible_v<_Up, _Xp>); > > if (this->_M_is_engaged()) > return this->_M_get(); > else > Optional and standard does not use this else (it is not needed), so I would also remove it here for consistency. > - return static_cast<_Tp>(std::forward<_Up>(__u)); > + return std::forward<_Up>(__u); > } > > template<typename _Up = remove_cv_t<_Tp>> > - constexpr _Tp > + constexpr remove_cv_t<_Tp> > value_or(_Up&& __u) && > { > - static_assert(is_move_constructible_v<_Tp>); > - static_assert(is_convertible_v<_Up&&, _Tp>); > + using _Xp = remove_cv_t<_Tp>; > + static_assert(is_convertible_v<_Tp, _Xp>); > + static_assert(is_convertible_v<_Up, _Xp>); > > if (this->_M_is_engaged()) > return std::move(this->_M_get()); > else > Same here. > - return static_cast<_Tp>(std::forward<_Up>(__u)); > + return std::forward<_Up>(__u); > } > > #if __cpp_lib_optional >= 202110L // C++23 > @@ -1726,9 +1730,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION > value_or(_Up&& __u) const > { > using _Xp = remove_cv_t<_Tp>; > - static_assert(is_constructible_v<_Xp, _Tp&>); > + static_assert(is_convertible_v<_Tp&, _Xp>); > static_assert(is_convertible_v<_Up, _Xp>); > - return _M_val ? *_M_val : > static_cast<_Xp>(std::forward<_Up>(__u)); > + if (_M_val) > + return *_M_val; + else > Sane here. > + return std::forward<_Up>(__u); > } > > // Monadic operations. > -- > 2.51.1 > >
