[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gcc-bugs
Subject:    [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use
From:       "cvs-commit at gcc dot gnu.org via Gcc-bugs" <gcc-bugs () gcc ! gnu ! org>
Date:       2020-10-31 0:34:18
Message-ID: bug-97600-4-KyiBFKrECM () http ! gcc ! gnu ! org/bugzilla/
[Download RAW message or body]

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

--- Comment #3 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:afb8da7faa9dfe5a0d94ed45a373d74c076784ab

commit r11-4584-gafb8da7faa9dfe5a0d94ed45a373d74c076784ab
Author: Patrick Palka <ppalka@redhat.com>
Date:   Fri Oct 30 20:33:19 2020 -0400

    libstdc++: Don't initialize from *this inside some views [PR97600]

    This works around a subtle issue where instantiating the begin()/end()
    member of some views (as part of return type deduction) inadvertently
    requires computing the satisfaction value of range<foo_view>.

    This is problematic because the constraint range<foo_view> requires the
    begin()/end() member to be callable.  But it's not callable until we've
    deduced its return type, so evaluation of range<foo_view> yields false
    at this point.  And if after both members are instantiated (and their
    return types deduced) we evaluate range<foo_view> again, this time it
    will yield true since the begin()/end() members are now both callable.
    This makes the program ill-formed according to [temp.constr.atomic]/3:

      If, at different points in the program, the satisfaction result is
      different for identical atomic constraints and template arguments, the
      program is ill-formed, no diagnostic required.

    The views affected by this issue are those whose begin()/end() member
    has a placeholder return type and that member initializes an _Iterator
    or _Sentinel object from a reference to *this.  The second condition is
    relevant because it means explicit conversion functions are considered
    during overload resolution (as per [over.match.copy], I think), and
    therefore it causes g++ to check the constraints of the conversion
    function view_interface<foo_view>::operator bool().  And this conversion
    function's constraints indirectly require range<foo_view>.

    This issue is observable on trunk only with basic_istream_view (as in
    the testcase in the PR).  But a pending patch that makes g++ memoize
    constraint satisfaction values indefinitely (it currently invalidates
    the satisfaction cache on various events) causes many existing tests for
    the other affected views to fail, because range<foo_view> then remains
    false for the whole compilation.

    This patch works around this issue by adjusting the constructors of the
    _Iterator and _Sentinel types of the affected views to take their
    foo_view argument by pointer instead of by reference, so that g++ no
    longer considers explicit conversion functions when resolving the
    direct-initialization inside these views' begin()/end() members.

    libstdc++-v3/ChangeLog:

            PR libstdc++/97600
            * include/std/ranges (basic_istream_view::begin): Initialize
            _Iterator from 'this' instead of '*this'.
            (basic_istream_view::_Iterator::_Iterator): Adjust constructor
            accordingly.
            (filter_view::_Iterator::_Iterator): Take a filter_view*
            argument instead of a filter_view& argument.
            (filter_view::_Sentinel::_Sentinel): Likewise.
            (filter_view::begin): Initialize _Iterator from 'this' instead
            of '*this'.
            (filter_view::end): Likewise.
            (transform_view::_Iterator::_Iterator): Take a _Parent* instead
            of a _Parent&.
            (filter_view::_Iterator::operator+): Adjust accordingly.
            (filter_view::_Iterator::operator-): Likewise.
            (filter_view::begin): Initialize _Iterator from 'this' instead
            of '*this'.
            (filter_view::end): Likewise.
            (join_view::_Iterator): Take a _Parent* instead of a _Parent&.
            (join_view::_Sentinel): Likewise.
            (join_view::begin): Initialize _Iterator from 'this' instead of
            '*this'.
            (join_view::end): Initialize _Sentinel from 'this' instead of
            '*this'.
            (split_view::_OuterIter): Take a _Parent& instead of a _Parent*.
            (split_view::begin): Initialize _OuterIter from 'this' instead
            of '*this'.
            (split_view::end): Likewise.
            * testsuite/std/ranges/97600.cc: New test.=
[prev in list] [next in list] [prev in thread] [next in thread]