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 at any point 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 satisaction 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. Tested on x86_64-pc-linux-gnu, and also verified that this fixes the testsuite failures when combined with the mentioned frontend patch (https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557237.html). Does this look OK for trunk? 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 | 78 +++++++++++----------- libstdc++-v3/testsuite/std/ranges/97600.cc | 32 +++++++++ 2 files changed, 71 insertions(+), 39 deletions(-) create mode 100644 libstdc++-v3/testsuite/std/ranges/97600.cc diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index a3e5354848a..28787cc5cc3 100644 --- a/libstdc++-v3/include/std/ranges +++ b/libstdc++-v3/include/std/ranges @@ -636,7 +636,7 @@ namespace views { if (_M_stream != nullptr) *_M_stream >> _M_object; - return _Iterator{*this}; + return _Iterator{this}; } constexpr default_sentinel_t @@ -657,8 +657,8 @@ namespace views _Iterator() = default; constexpr explicit - _Iterator(basic_istream_view& __parent) noexcept - : _M_parent(std::__addressof(__parent)) + _Iterator(basic_istream_view* __parent) noexcept + : _M_parent(__parent) { } _Iterator(const _Iterator&) = delete; @@ -1147,9 +1147,9 @@ namespace views _Iterator() = default; constexpr - _Iterator(filter_view& __parent, _Vp_iter __current) + _Iterator(filter_view* __parent, _Vp_iter __current) : _M_current(std::move(__current)), - _M_parent(std::__addressof(__parent)) + _M_parent(__parent) { } constexpr _Vp_iter @@ -1239,8 +1239,8 @@ namespace views _Sentinel() = default; constexpr explicit - _Sentinel(filter_view& __parent) - : _M_end(ranges::end(__parent._M_base)) + _Sentinel(filter_view* __parent) + : _M_end(ranges::end(__parent->_M_base)) { } constexpr sentinel_t<_Vp> @@ -1280,23 +1280,23 @@ namespace views begin() { if (_M_cached_begin._M_has_value()) - return {*this, _M_cached_begin._M_get(_M_base)}; + return {this, _M_cached_begin._M_get(_M_base)}; __glibcxx_assert(_M_pred.has_value()); auto __it = __detail::find_if(ranges::begin(_M_base), ranges::end(_M_base), std::ref(*_M_pred)); _M_cached_begin._M_set(_M_base, __it); - return {*this, std::move(__it)}; + return {this, std::move(__it)}; } constexpr auto end() { if constexpr (common_range<_Vp>) - return _Iterator{*this, ranges::end(_M_base)}; + return _Iterator{this, ranges::end(_M_base)}; else - return _Sentinel{*this}; + return _Sentinel{this}; } }; @@ -1375,9 +1375,9 @@ namespace views _Iterator() = default; constexpr - _Iterator(_Parent& __parent, _Base_iter __current) + _Iterator(_Parent* __parent, _Base_iter __current) : _M_current(std::move(__current)), - _M_parent(std::__addressof(__parent)) + _M_parent(__parent) { } constexpr @@ -1490,17 +1490,17 @@ namespace views friend constexpr _Iterator operator+(_Iterator __i, difference_type __n) requires random_access_range<_Base> - { return {*__i._M_parent, __i._M_current + __n}; } + { return {__i._M_parent, __i._M_current + __n}; } friend constexpr _Iterator operator+(difference_type __n, _Iterator __i) requires random_access_range<_Base> - { return {*__i._M_parent, __i._M_current + __n}; } + { return {__i._M_parent, __i._M_current + __n}; } friend constexpr _Iterator operator-(_Iterator __i, difference_type __n) requires random_access_range<_Base> - { return {*__i._M_parent, __i._M_current - __n}; } + { return {__i._M_parent, __i._M_current - __n}; } // _GLIBCXX_RESOLVE_LIB_DEFECTS // 3483. transform_view::iterator's difference is overconstrained @@ -1611,13 +1611,13 @@ namespace views constexpr _Iterator<false> begin() - { return _Iterator<false>{*this, ranges::begin(_M_base)}; } + { return _Iterator<false>{this, ranges::begin(_M_base)}; } constexpr _Iterator<true> begin() const requires range<const _Vp> && regular_invocable<const _Fp&, range_reference_t<const _Vp>> - { return _Iterator<true>{*this, ranges::begin(_M_base)}; } + { return _Iterator<true>{this, ranges::begin(_M_base)}; } constexpr _Sentinel<false> end() @@ -1625,7 +1625,7 @@ namespace views constexpr _Iterator<false> end() requires common_range<_Vp> - { return _Iterator<false>{*this, ranges::end(_M_base)}; } + { return _Iterator<false>{this, ranges::end(_M_base)}; } constexpr _Sentinel<true> end() const @@ -1637,7 +1637,7 @@ namespace views end() const requires common_range<const _Vp> && regular_invocable<const _Fp&, range_reference_t<const _Vp>> - { return _Iterator<true>{*this, ranges::end(_M_base)}; } + { return _Iterator<true>{this, ranges::end(_M_base)}; } constexpr auto size() requires sized_range<_Vp> @@ -2181,9 +2181,9 @@ namespace views _Iterator() = default; constexpr - _Iterator(_Parent& __parent, _Outer_iter __outer) + _Iterator(_Parent* __parent, _Outer_iter __outer) : _M_outer(std::move(__outer)), - _M_parent(std::__addressof(__parent)) + _M_parent(__parent) { _M_satisfy(); } constexpr @@ -2303,8 +2303,8 @@ namespace views _Sentinel() = default; constexpr explicit - _Sentinel(_Parent& __parent) - : _M_end(ranges::end(__parent._M_base)) + _Sentinel(_Parent* __parent) + : _M_end(ranges::end(__parent->_M_base)) { } constexpr @@ -2351,7 +2351,7 @@ namespace views constexpr bool __use_const = (__detail::__simple_view<_Vp> && is_reference_v<range_reference_t<_Vp>>); - return _Iterator<__use_const>{*this, ranges::begin(_M_base)}; + return _Iterator<__use_const>{this, ranges::begin(_M_base)}; } constexpr auto @@ -2359,7 +2359,7 @@ namespace views requires input_range<const _Vp> && is_reference_v<range_reference_t<const _Vp>> { - return _Iterator<true>{*this, ranges::begin(_M_base)}; + return _Iterator<true>{this, ranges::begin(_M_base)}; } constexpr auto @@ -2368,10 +2368,10 @@ namespace views if constexpr (forward_range<_Vp> && is_reference_v<_InnerRange> && forward_range<_InnerRange> && common_range<_Vp> && common_range<_InnerRange>) - return _Iterator<__detail::__simple_view<_Vp>>{*this, + return _Iterator<__detail::__simple_view<_Vp>>{this, ranges::end(_M_base)}; else - return _Sentinel<__detail::__simple_view<_Vp>>{*this}; + return _Sentinel<__detail::__simple_view<_Vp>>{this}; } constexpr auto @@ -2384,9 +2384,9 @@ namespace views && forward_range<range_reference_t<const _Vp>> && common_range<const _Vp> && common_range<range_reference_t<const _Vp>>) - return _Iterator<true>{*this, ranges::end(_M_base)}; + return _Iterator<true>{this, ranges::end(_M_base)}; else - return _Sentinel<true>{*this}; + return _Sentinel<true>{this}; } }; @@ -2505,14 +2505,14 @@ namespace views _OuterIter() = default; constexpr explicit - _OuterIter(_Parent& __parent) requires (!forward_range<_Base>) - : _M_parent(std::__addressof(__parent)) + _OuterIter(_Parent* __parent) requires (!forward_range<_Base>) + : _M_parent(__parent) { } constexpr - _OuterIter(_Parent& __parent, iterator_t<_Base> __current) + _OuterIter(_Parent* __parent, iterator_t<_Base> __current) requires forward_range<_Base> - : _M_parent(std::__addressof(__parent)), + : _M_parent(__parent), _M_current(std::move(__current)) { } @@ -2737,25 +2737,25 @@ namespace views { if constexpr (forward_range<_Vp>) return _OuterIter<__detail::__simple_view<_Vp>>{ - *this, ranges::begin(_M_base)}; + this, ranges::begin(_M_base)}; else { _M_current = ranges::begin(_M_base); - return _OuterIter<false>{*this}; + return _OuterIter<false>{this}; } } constexpr auto begin() const requires forward_range<_Vp> && forward_range<const _Vp> { - return _OuterIter<true>{*this, ranges::begin(_M_base)}; + return _OuterIter<true>{this, ranges::begin(_M_base)}; } constexpr auto end() requires forward_range<_Vp> && common_range<_Vp> { return _OuterIter<__detail::__simple_view<_Vp>>{ - *this, ranges::end(_M_base)}; + this, ranges::end(_M_base)}; } constexpr auto @@ -2764,7 +2764,7 @@ namespace views if constexpr (forward_range<_Vp> && forward_range<const _Vp> && common_range<const _Vp>) - return _OuterIter<true>{*this, ranges::end(_M_base)}; + return _OuterIter<true>{this, ranges::end(_M_base)}; else return default_sentinel; } diff --git a/libstdc++-v3/testsuite/std/ranges/97600.cc b/libstdc++-v3/testsuite/std/ranges/97600.cc new file mode 100644 index 00000000000..d992318259d --- /dev/null +++ b/libstdc++-v3/testsuite/std/ranges/97600.cc @@ -0,0 +1,32 @@ +// Copyright (C) 2020 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++2a" } +// { dg-do compile { target c++2a } } + +// PR libstdc++/97600 + +#include <sstream> +#include <ranges> + +void +test01() +{ + std::ranges::basic_istream_view<int, char, std::char_traits<char>> v; + v.begin(); + static_assert(std::ranges::range<decltype(v)>); +} -- 2.29.0.rc0
