Re: [PATCH] libstdc++: Fix backward compatibility of P2325R3 backport [PR106320]

[Patrick Palka <ppalka@redhat.com>]

On Fri, Jul 22, 2022 at 11:52 AM Patrick Palka <ppalka@redhat.com> wrote:
>
> The 11 and 10 partial backports of P2325R3, r11-9555-g92d612cccc1eec and
> r10-10808-g22b86cdc4d7fdd, unnecessarily preserved some changes from the
> paper that made certain views no longer default constructible, changes
> which aren't required to reap the overall benefits of the paper and
> which conflicted with the goal to maximize backwards compatibility with
> pre-P2325R3 code.
>
> This patch reverts the problematic changes, specifically it relaxes
> the constraints on various views' default constructors so that they
> reflect only the requirements that were already implicitly imposed by
> the NSDMIs of the view.  Thus for example this patch retains the
> default_initializable<_Vp> constraint on transform_view's default
> constructor since its '_Vp _M_base = _Vp()' NSDMI already reflects this
> constraint, and it removes the default_initializable<_Fp> constraint
> since the corresponding member '__detail::__box<_Fp> _M_fun' doesn't
> require default initializability (specializations of __box are always
> default constructible).
>
> After reverting these changes, all static_asserts from p2325.cc that
> verify lack of default constructibility now fail as expected, matching
> the pre-P2325R3 behavior.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for the 11 and 10
> branches?
>
>         PR libstdc++/106320
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/ranges (single_view): Remove

Whoops I forgot to finish the changelog entry, will fix.

>         * testsuite/std/ranges/adaptors/join.cc (test13): New test.
>         * testsuite/std/ranges/p2325.cc: Fix S to be only non default
>         constructible and not also non copy constructible.  XFAIL the
>         tests that verify a non default constructible functor makes a
>         view non default constructible (lines 94, 97 and 98).  XFAIL
>         the test that effectively verifies a non default constructible
>         element type makes single_view non default constructible (line
>         114).
> ---
>  libstdc++-v3/include/std/ranges                | 18 +++++-------------
>  .../testsuite/std/ranges/adaptors/join.cc      | 15 +++++++++++++++
>  libstdc++-v3/testsuite/std/ranges/p2325.cc     | 12 ++++++++----
>  3 files changed, 28 insertions(+), 17 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index bbdfb7dbe5c..0a67c45f1b8 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -200,7 +200,7 @@ namespace ranges
>      class single_view : public view_interface<single_view<_Tp>>
>      {
>      public:
> -      single_view() requires default_initializable<_Tp> = default;
> +      single_view() = default;
>
>        constexpr explicit
>        single_view(const _Tp& __t)
> @@ -1463,9 +1463,7 @@ namespace views::__adaptor
>        _Vp _M_base = _Vp();
>
>      public:
> -      filter_view() requires (default_initializable<_Vp>
> -                             && default_initializable<_Pred>)
> -       = default;
> +      filter_view() requires default_initializable<_Vp> = default;
>
>        constexpr
>        filter_view(_Vp __base, _Pred __pred)
> @@ -1829,9 +1827,7 @@ namespace views::__adaptor
>        _Vp _M_base = _Vp();
>
>      public:
> -      transform_view() requires (default_initializable<_Vp>
> -                                && default_initializable<_Fp>)
> -       = default;
> +      transform_view() requires default_initializable<_Vp> = default;
>
>        constexpr
>        transform_view(_Vp __base, _Fp __fun)
> @@ -2150,9 +2146,7 @@ namespace views::__adaptor
>        _Vp _M_base = _Vp();
>
>      public:
> -      take_while_view() requires (default_initializable<_Vp>
> -                                 && default_initializable<_Pred>)
> -       = default;
> +      take_while_view() requires default_initializable<_Vp> = default;
>
>        constexpr
>        take_while_view(_Vp base, _Pred __pred)
> @@ -2356,9 +2350,7 @@ namespace views::__adaptor
>        _Vp _M_base = _Vp();
>
>      public:
> -      drop_while_view() requires (default_initializable<_Vp>
> -                                 && default_initializable<_Pred>)
> -       = default;
> +      drop_while_view() requires default_initializable<_Vp> = default;
>
>        constexpr
>        drop_while_view(_Vp __base, _Pred __pred)
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/join.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/join.cc
> index d774e8d9385..14e254bc734 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/join.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/join.cc
> @@ -193,6 +193,20 @@ test11()
>      ;
>  }
>
> +void
> +test13()
> +{
> +  // PR libstdc++/106320
> +  auto l = std::views::transform([](auto x) {
> +    return x | std::views::transform([x=0](auto y) {
> +      return y;
> +    });
> +  });
> +  static_assert(!std::default_initializable<decltype(l)>);
> +  std::vector<std::vector<int>> v{{5, 6, 7}};
> +  v | l | std::views::join;
> +}
> +
>  int
>  main()
>  {
> @@ -207,4 +221,5 @@ main()
>    test09();
>    test10();
>    test11();
> +  test13();
>  }
> diff --git a/libstdc++-v3/testsuite/std/ranges/p2325.cc b/libstdc++-v3/testsuite/std/ranges/p2325.cc
> index 205b3458928..99f1b0f08f1 100644
> --- a/libstdc++-v3/testsuite/std/ranges/p2325.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/p2325.cc
> @@ -5,8 +5,8 @@
>  // Parts of P2325R3 are deliberately omitted in libstdc++ 11, in particular the
>  // removal of default ctors for back_/front_insert_iterator, ostream_iterator,
>  // ref_view and basic_istream_view/::iterator, so as to maximize backward
> -// compatibility with pre-P2325R3 code.  So most static_asserts in this test fail,
> -// see the xfails at the end of this file.
> +// compatibility with pre-P2325R3 code.  Namely all static_asserts that verify
> +// lack of default constructibility fail; see the xfails at the end of this file.
>
>  #include <ranges>
>  #include <iterator>
> @@ -93,7 +93,7 @@ test06()
>    static_assert(default_initializable<decltype(views::single(0) | adaptor(f1))>);
>    static_assert(!default_initializable<decltype(views::single(0) | adaptor(f2))>);
>
> -  struct S { S() = delete; };
> +  struct S { S() = delete; S(const S&) = default; S(S&&) = default; };
>    static_assert(!default_initializable<decltype(views::single(declval<S>()) | adaptor(f1))>);
>    static_assert(!default_initializable<decltype(views::single(declval<S>()) | adaptor(f2))>);
>  }
> @@ -109,7 +109,7 @@ void
>  test07()
>  {
>    // Verify join_view is conditionally default constructible.
> -  struct S { S() = delete; };
> +  struct S { S() = delete; S(const S&) = default; S(S&&) = default; };
>    using type1 = ranges::join_view<ranges::single_view<ranges::single_view<S>>>;
>    static_assert(!default_initializable<type1>);
>    using type2 = ranges::join_view<ranges::single_view<ranges::single_view<int>>>;
> @@ -173,6 +173,10 @@ test11()
>  // { dg-bogus "static assertion failed" "" { xfail *-*-* } 76 }
>  // { dg-bogus "static assertion failed" "" { xfail *-*-* } 77 }
>  // { dg-bogus "static assertion failed" "" { xfail *-*-* } 84 }
> +// { dg-bogus "static assertion failed" "" { xfail *-*-* } 94 }
> +// { dg-bogus "static assertion failed" "" { xfail *-*-* } 97 }
> +// { dg-bogus "static assertion failed" "" { xfail *-*-* } 98 }
> +// { dg-bogus "static assertion failed" "" { xfail *-*-* } 114 }
>  // { dg-bogus "static assertion failed" "" { xfail *-*-* } 124 }
>  // { dg-bogus "static assertion failed" "" { xfail *-*-* } 126 }
>  // { dg-bogus "static assertion failed" "" { xfail *-*-* } 128 }
> --
> 2.37.1.208.ge72d93e88c
>