[Bug c++/106320] New: [10 regression] build failure (due to view requirement changes?)

[Bug c++/106320] New: [10 regression] build failure (due to view requirement changes?)

[h2+bugs at fsfe dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

            Bug ID: 106320
           Summary: [10 regression] build failure (due to view requirement
                    changes?)
           Product: gcc
           Version: 10.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: h2+bugs at fsfe dot org
  Target Milestone: ---

The following snippet builds on GCC10.3, GCC11.3, GCC12.1 and even MSVC 19.32.
It no longer builds on GCC10.4. This might be related to 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103904

#include <ranges>
#include <vector>

int main()
{
    auto l = std::views::transform([](auto v)
    {
        auto x = int{}; //!BROKEN - remove this capture and it will work
        return v | std::views::transform([x](auto v2)
        {
            return v2;
        });
    });

    std::vector<std::vector<int>> v{{5, 6, 7}};
    v | l | std::views::join;
}

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |10.5
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
           Priority|P3                          |P2
      Known to work|                            |10.3.0
           Keywords|                            |needs-bisection,
                   |                            |rejects-valid
   Last reconfirmed|                            |2022-07-18

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I can confirm that the branch head with the standard library from GCC 10.3
still accepts the code.

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|needs-bisection             |

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I was definitely r10-10808-g22b86cdc4d7fdd

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[marxin at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r10-10808-g22b86cdc4d7fdd.

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The problem is that join_view still requires its _InnerView to be
default-constructible, but that's a transform_view holding a
non-default-constructible callable.

For gcc-11 this isn't a problem because of r11-8477-ge1489a3d613b6f
implementing P2328, which makes join_view uses non-propagating-cache for
_M_inner. That isn't on the gcc-10 branch.

I'm tempted to revert the PR 103904 backport on the branch. I was never very
happy about backporting it.

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |ppalka at gcc dot gnu.org

--- Comment #5 from Patrick Palka <ppalka at gcc dot gnu.org> ---
P2328 constrains the default ctors of various views and iterators.  Some of
these constraints just repeat what is already implicitly required by the
NSDMIs, such as default_initializable<_Vp> in transform_view.  Note that GCC
already treats NSDMIs as part of the "immediate context" when synthesizing a
default constructor, such that if an NSDMI is ill-formed then the constructor
will get defined as deleted.  So these constraints are redundant for GCC at
least, but IIUC this behavior is an extension that other compilers e.g. Clang
don't implement: https://godbolt.org/z/s4a7axYv7.  Backporting these
constraints should be safe since for GCC they have no effect and for other
compilers they'd just make some invalid programs valid.

But P2328 also adds default ctor constraints that weren't already implicitly
required by the NSDMIs, such as default_initializable<_Fp> in transform_view. 
(The corresponding data member _M_fun is always default constructible since it
uses the semiregular wrapper __box.)  We shouldn't have backported _these_
constraints since they cause some views to no longer be default xible.

I think we can salvage the backport by removing the problematic constraints
added by r10-10808-g22b86cdc4d7fdd:

 libstdc++-v3/include/std/ranges | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index 17018a5616c..faa55ebceb7 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -518,7 +518,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)
@@ -1592,9 +1592,7 @@ namespace views
       [[no_unique_address]] __detail::_CachedPosition<_Vp> _M_cached_begin;

     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)
@@ -1940,9 +1938,7 @@ namespace views
       __detail::__box<_Fp> _M_fun;

     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)
@@ -2220,9 +2216,7 @@ namespace views
       __detail::__box<_Pred> _M_pred;

     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)
@@ -2389,9 +2383,7 @@ namespace views
       [[no_unique_address]] __detail::_CachedPosition<_Vp> _M_cached_begin;

     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)

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:36dd51bf87e1c533bf33ad3b16fd3fe06c356746

commit r11-10170-g36dd51bf87e1c533bf33ad3b16fd3fe06c356746
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Jul 22 12:52:03 2022 -0400

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

    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 view specializations no longer default
    constructible, changes which aren't required to reap the overall benefits
    of the paper and which are backward incompatible with pre-P2325R3 code in
    practice.

    This patch reverts the problematic changes, specifically it relaxes the
    constraints on various views' default constructors added by the paper
    so that we keep only the constraints 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 requires this
    constraint, and it removes the default_initializable<_Fp> constraint
    since the corresponding member '__detail::__box<_Fp> _M_fun' doesn't
    require default constructibility (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.

            PR libstdc++/106320

    libstdc++-v3/ChangeLog:

            * include/std/ranges (single_view): Relax constraints on
            default constructor so as to preserve pre-P2325R3 behavior.
            (filter_view): Likewise.
            (transform_view): Likewise.
            (take_while_view): Likewise.
            (drop_while_view): Likewise.
            * 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).

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:2cad5c5c2de819b7a5177aa87e5e2fd088357b01

commit r10-10908-g2cad5c5c2de819b7a5177aa87e5e2fd088357b01
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Jul 22 12:52:03 2022 -0400

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

    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 view specializations no longer default
    constructible, changes which aren't required to reap the overall benefits
    of the paper and which are backward incompatible with pre-P2325R3 code in
    practice.

    This patch reverts the problematic changes, specifically it relaxes the
    constraints on various views' default constructors added by the paper
    so that we keep only the constraints 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 requires this
    constraint, and it removes the default_initializable<_Fp> constraint
    since the corresponding member '__detail::__box<_Fp> _M_fun' doesn't
    require default constructibility (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.

            PR libstdc++/106320

    libstdc++-v3/ChangeLog:

            * include/std/ranges (single_view): Relax constraints on
            default constructor so as to preserve pre-P2325R3 behavior.
            (filter_view): Likewise.
            (transform_view): Likewise.
            (take_while_view): Likewise.
            (drop_while_view): Likewise.
            * 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).

    (cherry picked from commit 36dd51bf87e1c533bf33ad3b16fd3fe06c356746)

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[h2+bugs at fsfe dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

--- Comment #8 from Hannes Hauswedell <h2+bugs at fsfe dot org> ---
Thanks a lot for fixing this in a way that preserves the backport <3

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:db19cfdac8ede93172aecc58612171c239c993ad

commit r13-2629-gdb19cfdac8ede93172aecc58612171c239c993ad
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Sep 12 15:05:04 2022 -0400

    libstdc++: Add already-accepted <ranges> testcase [PR106320]

    Although PR106320 affected only the 10 and 11 branches, and the testcase
    from there is already correctly accepted on trunk and the 12 branch, we
    still should add the testcase to trunk/12 too for inter-branch consistency.

            PR libstdc++/106320

    libstdc++-v3/ChangeLog:

            * testsuite/std/ranges/adaptors/join.cc (test13): New test.

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:f323610375f4f87098f98b501ab01d033c930558

commit r12-8757-gf323610375f4f87098f98b501ab01d033c930558
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon Sep 12 15:05:04 2022 -0400

    libstdc++: Add already-accepted <ranges> testcase [PR106320]

    Although PR106320 affected only the 10 and 11 branches, and the testcase
    from there is already correctly accepted on trunk and the 12 branch, we
    still should add the testcase to trunk/12 too for inter-branch consistency.

            PR libstdc++/106320

    libstdc++-v3/ChangeLog:

            * testsuite/std/ranges/adaptors/join.cc (test13): New test.

    (cherry picked from commit db19cfdac8ede93172aecc58612171c239c993ad)

[Bug libstdc++/106320] [10 regression] build failure (due to view requirement changes?)

[ppalka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106320

Patrick Palka <ppalka at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Patrick Palka <ppalka at gcc dot gnu.org> ---
Fixed.