diff options
author | Patrick Palka <ppalka@redhat.com> | 2020-10-30 20:33:19 -0400 |
---|---|---|
committer | Patrick Palka <ppalka@redhat.com> | 2020-10-30 20:33:19 -0400 |
commit | afb8da7faa9dfe5a0d94ed45a373d74c076784ab (patch) | |
tree | 1838a684be02275242b26ab1ebcd41c37d4b90a9 | |
parent | 39bf4f14fc75e14aafc4ba8a53a34775f29b743a (diff) | |
download | gcc-afb8da7faa9dfe5a0d94ed45a373d74c076784ab.zip gcc-afb8da7faa9dfe5a0d94ed45a373d74c076784ab.tar.gz gcc-afb8da7faa9dfe5a0d94ed45a373d74c076784ab.tar.bz2 |
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.
-rw-r--r-- | libstdc++-v3/include/std/ranges | 78 | ||||
-rw-r--r-- | libstdc++-v3/testsuite/std/ranges/97600.cc | 32 |
2 files changed, 71 insertions, 39 deletions
diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges index bc7bb05..14d2a11 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> @@ -2193,9 +2193,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 @@ -2315,8 +2315,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 @@ -2363,7 +2363,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 @@ -2371,7 +2371,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 @@ -2380,10 +2380,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 @@ -2396,9 +2396,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}; } }; @@ -2517,14 +2517,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)) { } @@ -2749,25 +2749,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 @@ -2776,7 +2776,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 0000000..d992318 --- /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)>); +} |