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
>> > >
>> > >
>> >
>>
>>

Reply via email to