From 170101d1dc835dfb000182ba452b760c5047351f Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Wed, 9 Jun 2021 12:36:16 +0200 Subject: [PATCH 1/7] Adopt LWG-3546 We currently have no real infrastructure to test move only iterators, we should discuss that Addresses #1965 --- stl/inc/iterator | 10 ++++++++-- stl/inc/yvals_core.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/stl/inc/iterator b/stl/inc/iterator index 195a8dd61b0..04eebede3d8 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -841,7 +841,7 @@ private: public: explicit _Proxy(iter_reference_t<_Iter>&& _Right) noexcept( is_nothrow_move_constructible_v>) // strengthened - : _Keep(_STD move(_Right)) {} + : _Keep(_STD forward>(_Right)) {} _NODISCARD const iter_value_t<_Iter>* operator->() const noexcept /* strengthened */ { return _STD addressof(_Keep); @@ -932,8 +932,14 @@ public: common_iterator _Tmp = *this; ++_Val._Iterator; return _Tmp; - } else { + } else if constexpr (_Can_reference<_Iter> // + || !(constructible_from, iter_reference_t<_Iter>> // + && move_constructible>) ) { return _Val._Iterator++; + } else { + auto _Tmp = _Proxy{*_Val._Iterator}; + ++_Val._Iterator; + return _Tmp; } } diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index 3b2c62061be..47a8cd7e428 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -249,6 +249,7 @@ // _HAS_CXX20 indirectly controls: // P0619R4 Removing C++17-Deprecated Features +// LWG-3546 common_iterator postfix-proxy is not quite right // _HAS_CXX20 and _SILENCE_ALL_CXX20_DEPRECATION_WARNINGS control: // P0767R1 Deprecating is_pod From 194a5c43f03658d9e13d1b53f9ba525fd765a86a Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 10 Jun 2021 13:58:32 +0200 Subject: [PATCH 2/7] drop mentions in yvals_core --- stl/inc/yvals_core.h | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/yvals_core.h b/stl/inc/yvals_core.h index 47a8cd7e428..3b2c62061be 100644 --- a/stl/inc/yvals_core.h +++ b/stl/inc/yvals_core.h @@ -249,7 +249,6 @@ // _HAS_CXX20 indirectly controls: // P0619R4 Removing C++17-Deprecated Features -// LWG-3546 common_iterator postfix-proxy is not quite right // _HAS_CXX20 and _SILENCE_ALL_CXX20_DEPRECATION_WARNINGS control: // P0767R1 Deprecating is_pod From 7b8dd14715212f5b36c0eae81293efbb83f7fa75 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Tue, 13 Jul 2021 17:12:25 +0200 Subject: [PATCH 3/7] Address review comments --- stl/inc/iterator | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/stl/inc/iterator b/stl/inc/iterator index ebb49d861c6..a1517643a58 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -827,12 +827,16 @@ private: public: explicit _Proxy(iter_reference_t<_Iter>&& _Right) noexcept( - is_nothrow_move_constructible_v>) // strengthened + is_nothrow_constructible_v, iter_reference_t<_Iter>>) // strengthened : _Keep(_STD forward>(_Right)) {} _NODISCARD const iter_value_t<_Iter>* operator->() const noexcept /* strengthened */ { return _STD addressof(_Keep); } + + _NODISCARD const iter_value_t<_Iter>& operator*() const noexcept /* strengthened */ { + return _Keep; + } }; public: @@ -1032,15 +1036,24 @@ struct _Common_iterator_pointer_type<_Iter> { using pointer = decltype(_STD declval<_Iter&>().operator->()); }; -template -struct iterator_traits> { - using iterator_concept = conditional_t, forward_iterator_tag, input_iterator_tag>; +template +struct _Common_iterator_traits_category_base { + using iterator_category = input_iterator_tag; +}; + +template <_Has_member_iterator_category _Iter> +struct _Common_iterator_traits_category_base<_Iter> { using iterator_category = conditional_t, forward_iterator_tag>, forward_iterator_tag, input_iterator_tag>; - using value_type = iter_value_t<_Iter>; - using difference_type = iter_difference_t<_Iter>; - using pointer = typename _Common_iterator_pointer_type<_Iter>::pointer; - using reference = iter_reference_t<_Iter>; +}; + +template +struct iterator_traits> : _Common_iterator_traits_category_base<_Iter> { + using iterator_concept = conditional_t, forward_iterator_tag, input_iterator_tag>; + using value_type = iter_value_t<_Iter>; + using difference_type = iter_difference_t<_Iter>; + using pointer = typename _Common_iterator_pointer_type<_Iter>::pointer; + using reference = iter_reference_t<_Iter>; }; // CLASS TEMPLATE counted_iterator From cffdefbc1901d41462a7f9aebbbd1756cf912ffb Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Tue, 24 Aug 2021 10:21:56 +0200 Subject: [PATCH 4/7] Rebase upon P2259 and address review comments --- stl/inc/iterator | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/stl/inc/iterator b/stl/inc/iterator index e245c9a6a0c..871c55d0ffb 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -803,6 +803,11 @@ public: _Variantish_state _Contains; }; +template +concept _Postfix_can_reference = input_or_output_iterator<_Iter> && requires(_Iter& __it) { + { *__it++ } -> _Can_reference; +}; + // clang-format off template _Se> requires (!same_as<_Iter, _Se> && copyable<_Iter>) @@ -821,6 +826,16 @@ private: _NODISCARD const iter_value_t<_Iter>* operator->() const noexcept /* strengthened */ { return _STD addressof(_Keep); } + }; + + class _Postfix_proxy { + private: + iter_value_t<_Iter> _Keep; + + public: + explicit _Postfix_proxy(iter_reference_t<_Iter>&& _Right) noexcept( + is_nothrow_constructible_v, iter_reference_t<_Iter>>) // strengthened + : _Keep(_STD forward>(_Right)) {} _NODISCARD const iter_value_t<_Iter>& operator*() const noexcept /* strengthened */ { return _Keep; @@ -913,12 +928,12 @@ public: common_iterator _Tmp = *this; ++_Val._Iterator; return _Tmp; - } else if constexpr (_Can_reference<_Iter> // + } else if constexpr (_Postfix_can_reference<_Iter> // || !(constructible_from, iter_reference_t<_Iter>> // && move_constructible>) ) { return _Val._Iterator++; } else { - auto _Tmp = _Proxy{*_Val._Iterator}; + auto _Tmp = _Postfix_proxy{*_Val._Iterator}; ++_Val._Iterator; return _Tmp; } From d125a26bfd4788c6360dd4f8d94989e67d524bf1 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Thu, 2 Sep 2021 06:34:11 +0200 Subject: [PATCH 5/7] Update stl/inc/iterator Co-authored-by: S. B. Tam --- stl/inc/iterator | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/iterator b/stl/inc/iterator index 871c55d0ffb..c137749ab1c 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -933,7 +933,7 @@ public: && move_constructible>) ) { return _Val._Iterator++; } else { - auto _Tmp = _Postfix_proxy{*_Val._Iterator}; + _Postfix_proxy _Tmp{*_Val._Iterator}; ++_Val._Iterator; return _Tmp; } From 2980ced81f3fbdb3ce7a1bf689ea64a82e113f9d Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 23 Sep 2021 20:42:53 -0700 Subject: [PATCH 6/7] Minor refactoring * Move all the conditions for using the postfix-proxy into the helper concept for maximal short-circuiting benefit. * Notice that the postfix-proxy reads from the iterator without requiring it to be `indirectly_readable`, add constraint and submit LWG issue to fix. * Factor commonolity out of two proxy types into a common base * drive-by fix the constraint on `operator->` by requiring `i.operator->()` to be valid for a `const _Iter` lvalue instead of and `_Iter` rvalue. --- stl/inc/iterator | 69 ++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/stl/inc/iterator b/stl/inc/iterator index c137749ab1c..7064c964c28 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -803,43 +803,24 @@ public: _Variantish_state _Contains; }; +// clang-format off template -concept _Postfix_can_reference = input_or_output_iterator<_Iter> && requires(_Iter& __it) { - { *__it++ } -> _Can_reference; -}; +concept _Use_postfix_proxy = !requires(_Iter& __it) { { *__it++ } -> _Can_reference; } + && indirectly_readable<_Iter> // Per LWG issue submitted but unnumbered as of 2021-09-23 + && constructible_from, iter_reference_t<_Iter>> + && move_constructible>; -// clang-format off template _Se> requires (!same_as<_Iter, _Se> && copyable<_Iter>) class common_iterator { // clang-format on private: - class _Proxy { - private: + struct _Proxy_base { iter_value_t<_Iter> _Keep; - public: - explicit _Proxy(iter_reference_t<_Iter>&& _Right) noexcept( - is_nothrow_constructible_v, iter_reference_t<_Iter>>) // strengthened - : _Keep(_STD forward>(_Right)) {} - - _NODISCARD const iter_value_t<_Iter>* operator->() const noexcept /* strengthened */ { - return _STD addressof(_Keep); - } - }; - - class _Postfix_proxy { - private: - iter_value_t<_Iter> _Keep; - - public: - explicit _Postfix_proxy(iter_reference_t<_Iter>&& _Right) noexcept( + explicit _Proxy_base(iter_reference_t<_Iter>&& _Right) noexcept( is_nothrow_constructible_v, iter_reference_t<_Iter>>) // strengthened : _Keep(_STD forward>(_Right)) {} - - _NODISCARD const iter_value_t<_Iter>& operator*() const noexcept /* strengthened */ { - return _Keep; - } }; public: @@ -893,20 +874,31 @@ public: // clang-format off _NODISCARD decltype(auto) operator->() const requires indirectly_readable - && (_Has_member_arrow<_Iter> || is_reference_v> + && (_Has_member_arrow || is_reference_v> || constructible_from, iter_reference_t<_Iter>>) { // clang-format on #if _ITERATOR_DEBUG_LEVEL != 0 _STL_VERIFY(_Val._Contains == _Variantish_state::_Holds_iter, "common_iterator can only be dereferenced if it holds an iterator"); #endif // _ITERATOR_DEBUG_LEVEL != 0 - if constexpr (is_pointer_v<_Iter> || _Has_member_arrow<_Iter>) { + if constexpr (is_pointer_v<_Iter> || _Has_member_arrow) { return (_Val._Iterator); // NB: () are necessary for decltype(auto) } else if constexpr (is_reference_v>) { auto&& _Tmp = *_Val._Iterator; return _STD addressof(_Tmp); } else { - return _Proxy{*_Val._Iterator}; + class _Arrow_proxy : private _Proxy_base { + public: + friend common_iterator; + + using _Proxy_base::_Proxy_base; + + _NODISCARD const iter_value_t<_Iter>* operator->() const noexcept /* strengthened */ { + return _STD addressof(this->_Keep); + } + }; + + return _Arrow_proxy{*_Val._Iterator}; } } @@ -928,14 +920,23 @@ public: common_iterator _Tmp = *this; ++_Val._Iterator; return _Tmp; - } else if constexpr (_Postfix_can_reference<_Iter> // - || !(constructible_from, iter_reference_t<_Iter>> // - && move_constructible>) ) { - return _Val._Iterator++; - } else { + } else if constexpr (_Use_postfix_proxy<_Iter>) { + class _Postfix_proxy : private _Proxy_base { + public: + friend common_iterator; + + using _Proxy_base::_Proxy_base; + + _NODISCARD const iter_value_t<_Iter>& operator*() const noexcept /* strengthened */ { + return this->_Keep; + } + }; + _Postfix_proxy _Tmp{*_Val._Iterator}; ++_Val._Iterator; return _Tmp; + } else { + return _Val._Iterator++; } } From 556527b00be9c2f1d8bfda88baa366c9c52abb2b Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Mon, 27 Sep 2021 09:55:06 -0700 Subject: [PATCH 7/7] LWG issue is 3601; miscco's comment --- stl/inc/iterator | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stl/inc/iterator b/stl/inc/iterator index 7064c964c28..2aeb23824cc 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -806,7 +806,7 @@ public: // clang-format off template concept _Use_postfix_proxy = !requires(_Iter& __it) { { *__it++ } -> _Can_reference; } - && indirectly_readable<_Iter> // Per LWG issue submitted but unnumbered as of 2021-09-23 + && indirectly_readable<_Iter> // Per LWG-3601 && constructible_from, iter_reference_t<_Iter>> && move_constructible>; @@ -881,7 +881,7 @@ public: _STL_VERIFY(_Val._Contains == _Variantish_state::_Holds_iter, "common_iterator can only be dereferenced if it holds an iterator"); #endif // _ITERATOR_DEBUG_LEVEL != 0 - if constexpr (is_pointer_v<_Iter> || _Has_member_arrow) { + if constexpr (_Has_member_arrow || is_pointer_v<_Iter>) { return (_Val._Iterator); // NB: () are necessary for decltype(auto) } else if constexpr (is_reference_v>) { auto&& _Tmp = *_Val._Iterator;