libstdc++: Don't initialize from *this inside some views [PR97600]
authorPatrick Palka <ppalka@redhat.com>
Sat, 31 Oct 2020 00:33:19 +0000 (20:33 -0400)
committerPatrick Palka <ppalka@redhat.com>
Sat, 31 Oct 2020 00:33:19 +0000 (20:33 -0400)
commitafb8da7faa9dfe5a0d94ed45a373d74c076784ab
tree1838a684be02275242b26ab1ebcd41c37d4b90a9
parent39bf4f14fc75e14aafc4ba8a53a34775f29b743a
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.
libstdc++-v3/include/std/ranges
libstdc++-v3/testsuite/std/ranges/97600.cc [new file with mode: 0644]