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

Message ID 20220722155200.1533671-1-ppalka@redhat.com
State New, archived
Headers
Series libstdc++: Fix backward compatibility of P2325R3 backport [PR106320] |

Commit Message

Li, Pan2 via Gcc-patches July 22, 2022, 3:52 p.m. UTC
  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
	* 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(-)
  

Comments

Li, Pan2 via Gcc-patches July 22, 2022, 3:58 p.m. UTC | #1
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
>
  
Li, Pan2 via Gcc-patches July 22, 2022, 4:20 p.m. UTC | #2
On Fri, 22 Jul 2022 at 16:52, Patrick Palka via Libstdc++
<libstdc++@gcc.gnu.org> 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?

OK for gcc-10 and gcc-11 (with the completed ChangeLog), thanks.
  

Patch

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 }