Thanks for the review! All comments addressed in '[PATCH v2] libstdc++: Implement LWG4222 'expected' constructor from a single value missing a constraint' mail.
Tomasz Kaminski <tkami...@redhat.com> 于2025年8月14日周四 17:59写道: > > > On Wed, Aug 13, 2025 at 9:40 PM Patrick Palka <ppa...@redhat.com> wrote: > >> On Wed, 13 Aug 2025, Patrick Palka wrote: >> >> > Thanks for the patch! Looks good to me for the most part. >> > >> > On Fri, 8 Aug 2025, Yihan Wang wrote: >> > >> > > libstdc++-v3/ChangeLog: >> > > >> > > * include/std/expected: >> > >> > This ChangeLog entry should be filled in, e.g. >> > >> > * include/std/expected (expected::expected(_Up&&)): Add >> > missing constraint as per LWG 4222. >> > >> > > * testsuite/20_util/expected/lwg4222.cc: New test. >> > > >> > > Signed-off-by: Yihan Wang <yronglin...@gmail.com> >> > > --- >> > > libstdc++-v3/include/std/expected | 1 + >> > > .../testsuite/20_util/expected/lwg4222.cc | 15 >> +++++++++++++++ >> > > 2 files changed, 16 insertions(+) >> > > create mode 100644 libstdc++-v3/testsuite/20_util/expected/lwg4222.cc >> > > >> > > diff --git a/libstdc++-v3/include/std/expected >> b/libstdc++-v3/include/std/expected >> > > index 60f1565f15b..2b200ea0589 100644 >> > > --- a/libstdc++-v3/include/std/expected >> > > +++ b/libstdc++-v3/include/std/expected >> > > @@ -474,6 +474,7 @@ namespace __expected >> > > template<typename _Up = remove_cv_t<_Tp>> >> > > requires (!is_same_v<remove_cvref_t<_Up>, expected>) >> > > && (!is_same_v<remove_cvref_t<_Up>, in_place_t>) >> > > + && (!is_same_v<remove_cvref_t<_Up>, unexpect_t>) >> > >> > Seems this line is overly indented causing it to not be aligned with >> the rest. >> > >> > > && is_constructible_v<_Tp, _Up> >> > > && (!__expected::__is_unexpected<remove_cvref_t<_Up>>) >> > > && __expected::__not_constructing_bool_from_expected<_Tp, _Up> >> > > diff --git a/libstdc++-v3/testsuite/20_util/expected/lwg4222.cc >> b/libstdc++-v3/testsuite/20_util/expected/lwg4222.cc >> > > new file mode 100644 >> > > index 00000000000..a260cfef3dd >> > > --- /dev/null >> > > +++ b/libstdc++-v3/testsuite/20_util/expected/lwg4222.cc >> > > @@ -0,0 +1,15 @@ >> > > +// { dg-do compile { target c++23 } } >> > > + >> > > +// LWG 4222. 'expected' constructor from a single value missing a >> constraint >> > > + >> > > +#include <expected> >> > > +#include <type_traits> >> > > + >> > > +struct T { >> > > + explicit T(auto) {} >> > > +}; >> > > +struct E { >> > > + E(int) {} >> > > +}; >> > > + >> > > +static_assert(!std::is_constructible_v<std::expected<T, E>, >> std::unexpect_t>); >> >> Maybe we should also test constructing from const unexpect_t and >> unexpect_t& and variations thereof. >> > I agree. I would like to also add a test using inplace constructor to > construct > wrapped type from expected. Something like: > struct V { > explicit V(std:unexpect_t) {} > }; > > std::expected<V, int> e1(std::inplace, std::unexcept); > VERIFY( e1.has_value() ); > std::expected<int, V> e2(std::unexcept, std::unexcept); > VERIFY( !e2.has_value() ); > > >> > > -- >> > > 2.39.5 >> > > >> > > >> > >> >>