From b332ea848105ef650ebdbda1b6b50ec2710c1d9e Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Mon, 31 Aug 2020 23:29:12 -0700 Subject: [PATCH 1/8] Implement views::filter --- stl/inc/algorithm | 46 --- stl/inc/ranges | 319 ++++++++++++++- stl/inc/xutility | 46 +++ tests/std/test.lst | 2 + .../P0896R4_ranges_range_machinery/test.cpp | 1 + tests/std/tests/P0896R4_views_filter/env.lst | 4 + tests/std/tests/P0896R4_views_filter/test.cpp | 384 ++++++++++++++++++ .../tests/P0896R4_views_filter_death/env.lst | 4 + .../tests/P0896R4_views_filter_death/test.cpp | 156 +++++++ 9 files changed, 894 insertions(+), 68 deletions(-) create mode 100644 tests/std/tests/P0896R4_views_filter/env.lst create mode 100644 tests/std/tests/P0896R4_views_filter/test.cpp create mode 100644 tests/std/tests/P0896R4_views_filter_death/env.lst create mode 100644 tests/std/tests/P0896R4_views_filter_death/test.cpp diff --git a/stl/inc/algorithm b/stl/inc/algorithm index d52591085a8..4650fa3db14 100644 --- a/stl/inc/algorithm +++ b/stl/inc/algorithm @@ -459,52 +459,6 @@ template _Se, class _Pj, indirect_unary_predicate> _Pr> - _NODISCARD constexpr _It _Find_if_unchecked(_It _First, const _Se _Last, _Pr _Pred, _Pj _Proj) { - for (; _First != _Last; ++_First) { - if (_STD invoke(_Pred, _STD invoke(_Proj, *_First))) { - break; - } - } - - return _First; - } - - class _Find_if_fn : private _Not_quite_object { - public: - using _Not_quite_object::_Not_quite_object; - - template _Se, class _Pj = identity, - indirect_unary_predicate> _Pr> - _NODISCARD constexpr _It operator()(_It _First, _Se _Last, _Pr _Pred, _Pj _Proj = {}) const { - _Adl_verify_range(_First, _Last); - auto _UResult = _RANGES _Find_if_unchecked( - _Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _Pass_fn(_Pred), _Pass_fn(_Proj)); - - _Seek_wrapped(_First, _STD move(_UResult)); - return _First; - } - - template , _Pj>> _Pr> - _NODISCARD constexpr borrowed_iterator_t<_Rng> operator()(_Rng&& _Range, _Pr _Pred, _Pj _Proj = {}) const { - auto _First = _RANGES begin(_Range); - auto _UResult = _RANGES _Find_if_unchecked( - _Get_unwrapped(_STD move(_First)), _Uend(_Range), _Pass_fn(_Pred), _Pass_fn(_Proj)); - - _Seek_wrapped(_First, _STD move(_UResult)); - return _First; - } - }; - - inline constexpr _Find_if_fn find_if{_Not_quite_object::_Construct_tag{}}; -} // namespace ranges -#endif // __cpp_lib_concepts - // FUNCTION TEMPLATE find_if_not template _NODISCARD _CONSTEXPR20 _InIt find_if_not(_InIt _First, const _InIt _Last, _Pr _Pred) { diff --git a/stl/inc/ranges b/stl/inc/ranges index ead69fbcd17..1cb52a234c8 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -29,9 +29,10 @@ namespace ranges { // clang-format off // CONCEPT ranges::viewable_range - template + template // Per P/R of LWG-3481 concept viewable_range = range<_Rng> - && (borrowed_range<_Rng> || view>); + && ((view> && constructible_from, _Rng>) + || (!view> && borrowed_range<_Rng>)); template concept _Simple_view = view<_Rng> && range @@ -40,6 +41,11 @@ namespace ranges { template concept _Copy_constructible_object = copy_constructible<_Ty> && is_object_v<_Ty>; + + template + concept _Has_arrow = input_iterator<_It> + && (is_pointer_v<_It> + || requires(_It _Iter) { _Iter.operator->(); }); // clang-format on namespace _Pipe { @@ -728,6 +734,275 @@ namespace ranges { using all_t = decltype(all(_STD declval<_Rng>())); } // namespace views + // CLASS TEMPLATE ranges::filter_view + // clang-format off + template > _Pr> + requires view<_Vw> && is_object_v<_Pr> + class filter_view : public _Cached_position_t, _Vw, filter_view<_Vw, _Pr>> { + // clang-format on + private: + /* [[no_unique_address]] */ _Vw _Range{}; + /* [[no_unique_address]] */ _Semiregular_box<_Pr> _Pred{}; + + template // TRANSITION, LWG-3289 + struct _Iterator_base {}; + // clang-format off + template + requires _Has_member_iterator_category + struct _Iterator_base { + // clang-format on + using iterator_category = + conditional_t, + bidirectional_iterator_tag, + conditional_t, + forward_iterator_tag, input_iterator_tag>>; + }; + + class _Iterator : public _Iterator_base>> { + public: + using iterator_concept = conditional_t, bidirectional_iterator_tag, + conditional_t, forward_iterator_tag, input_iterator_tag>>; + using value_type = range_value_t<_Vw>; + using difference_type = range_difference_t<_Vw>; + + _Iterator() = default; + constexpr _Iterator(filter_view& _Parent_, iterator_t<_Vw> _Current_) noexcept( + is_nothrow_move_constructible_v>) // strengthened + : _Current(_STD move(_Current_)), _Parent{_STD addressof(_Parent_)} {} + + _NODISCARD constexpr iterator_t<_Vw> base() const& noexcept(is_nothrow_copy_constructible_v< + iterator_t<_Vw>>) /* strengthened */ requires copyable> { + return _Current; + } + _NODISCARD constexpr iterator_t<_Vw> base() && noexcept( + is_nothrow_move_constructible_v>) /* strengthened */ { + return _STD move(_Current); + } + + _NODISCARD constexpr range_reference_t<_Vw> operator*() const + noexcept(noexcept(*_Current)) /* strengthened */ { +#if _ITERATOR_DEBUG_LEVEL != 0 + _Check_dereference(); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + return *_Current; + } + + // clang-format off + _NODISCARD constexpr iterator_t<_Vw> operator->() const + noexcept(is_nothrow_copy_constructible_v>) /* strengthened */ + requires _Has_arrow> && copyable> { + // clang-format on +#if _ITERATOR_DEBUG_LEVEL != 0 + _Check_dereference(); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + return _Current; + } + + constexpr _Iterator& operator++() { +#if _ITERATOR_DEBUG_LEVEL != 0 + _STL_VERIFY(_Parent != nullptr, "cannot increment value-initialized filter_view iterator"); + _STL_VERIFY(_Current != _RANGES end(_Parent->_Range), "cannot increment filter_view iterator past end"); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + _Current = + _RANGES find_if(_STD move(++_Current), _RANGES end(_Parent->_Range), _STD ref(*_Parent->_Pred)); + return *this; + } + + constexpr decltype(auto) operator++(int) { + if constexpr (forward_range<_Vw>) { + auto _Tmp = *this; + ++*this; + return _Tmp; + } else { + ++*this; + } + } + + constexpr _Iterator& operator--() requires bidirectional_range<_Vw> { +#if _ITERATOR_DEBUG_LEVEL != 0 + _STL_VERIFY(_Parent != nullptr, "cannot decrement value-initialized filter_view iterator"); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + do { +#if _ITERATOR_DEBUG_LEVEL != 0 + _STL_VERIFY(_Current != _RANGES begin(_Parent->_Range), + "cannot decrement filter_view iterator before begin"); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + --_Current; + } while (!_STD invoke(*_Parent->_Pred, *_Current)); + return *this; + } + + constexpr _Iterator operator--(int) requires bidirectional_range<_Vw> { + auto _Tmp = *this; + --*this; + return _Tmp; + } + + _NODISCARD friend constexpr bool operator==( + const _Iterator& _Left, const _Iterator& _Right) requires equality_comparable> { +#if _ITERATOR_DEBUG_LEVEL != 0 + _STL_VERIFY( + _Left._Parent == _Right._Parent, "cannot compare incompatible filter_view iterators for equality"); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + return _Left._Current == _Right._Current; + } + + _NODISCARD friend constexpr range_rvalue_reference_t<_Vw> iter_move(const _Iterator& _It) noexcept( + noexcept(_RANGES iter_move(_It._Current))) { +#if _ITERATOR_DEBUG_LEVEL != 0 + _It._Check_dereference(); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + return _RANGES iter_move(_It._Current); + } + + friend constexpr void iter_swap(const _Iterator& _Left, const _Iterator& _Right) noexcept(noexcept( + _RANGES iter_swap(_Left._Current, _Right._Current))) requires indirectly_swappable> { +#if _ITERATOR_DEBUG_LEVEL != 0 + _Left._Check_dereference(); + _Right._Check_dereference(); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + return _RANGES iter_swap(_Left._Current, _Right._Current); + } + + _NODISCARD constexpr bool _Equal(const sentinel_t<_Vw>& _Last) const { + return _Current == _Last; + } + + private: +#if _ITERATOR_DEBUG_LEVEL != 0 + constexpr void _Check_dereference() const noexcept { + _STL_VERIFY(_Parent != nullptr, "cannot dereference value-initialized filter_view iterator"); + _STL_VERIFY(_Current != _RANGES end(_Parent->_Range), "cannot dereference end filter_view iterator"); + } +#endif // _ITERATOR_DEBUG_LEVEL != 0 + + /* [[no_unique_address]] */ iterator_t<_Vw> _Current{}; + filter_view* _Parent{}; + }; + + class _Sentinel { + public: + _Sentinel() = default; + constexpr explicit _Sentinel(filter_view& _Parent) noexcept( + noexcept(_RANGES end(_Parent._Range))) // strengthened + : _Last(_RANGES end(_Parent._Range)) {} + + _NODISCARD constexpr sentinel_t<_Vw> base() const + noexcept(is_nothrow_copy_constructible_v>) /* strengthened */ { + return _Last; + } + + _NODISCARD friend constexpr bool operator==(const _Iterator& _It, const _Sentinel& _Se) { + return _It._Equal(_Se._Last); + } + + private: + _NODISCARD constexpr bool _Equal(const _Iterator& _It) const { + return _It._Current == _Last; + } + + /* [[no_unique_address]] */ sentinel_t<_Vw> _Last{}; + }; + + public: + filter_view() = default; + constexpr filter_view(_Vw _Range_, _Pr _Pred_) noexcept( + is_nothrow_move_constructible_v<_Vw>&& is_nothrow_move_constructible_v<_Pr>) // strengthened + : _Range(_STD move(_Range_)), _Pred{in_place, _STD move(_Pred_)} {} + + _NODISCARD constexpr _Vw base() const& noexcept( + is_nothrow_copy_constructible_v<_Vw>) /* strengthened */ requires copy_constructible<_Vw> { + return _Range; + } + _NODISCARD constexpr _Vw base() && noexcept(is_nothrow_move_constructible_v<_Vw>) /* strengthened */ { + return _STD move(_Range); + } + + _NODISCARD constexpr const _Pr& pred() const noexcept /* strengthened */ { +#if _CONTAINER_DEBUG_LEVEL > 0 + _STL_VERIFY(_Pred, "value-initialized filter_view has no predicate"); +#endif // _CONTAINER_DEBUG_LEVEL > 0 + return *_Pred; + } + + _NODISCARD constexpr _Iterator begin() { +#if _CONTAINER_DEBUG_LEVEL > 0 + _STL_VERIFY( + _Pred, "N4861 [range.filter.view]/3 forbids calling begin on a filter_view that holds no predicate"); +#endif // _CONTAINER_DEBUG_LEVEL > 0 + if constexpr (forward_range<_Vw>) { + if (this->_Has_cache()) { + return _Iterator{*this, this->_Get_cache(_Range)}; + } + } + + auto _First = _RANGES find_if(_Range, _STD ref(*_Pred)); + if constexpr (forward_range<_Vw>) { + this->_Set_cache(_Range, _First); + } + + return _Iterator{*this, _STD move(_First)}; + } + + _NODISCARD constexpr auto end() { + if constexpr (common_range<_Vw>) { + return _Iterator{*this, _RANGES end(_Range)}; + } else { + return _Sentinel{*this}; + } + } + }; + + template + filter_view(_Rng&&, _Pr) -> filter_view, _Pr>; + + namespace views { + // VARIABLE views::filter + class _Filter_fn { + private: + template + class _Partial : public _Pipe::_Base<_Partial<_Pr>> { + private: + /* [[no_unique_address]] */ _Semiregular_box<_Pr> _Pred; + + friend _Filter_fn; + + constexpr explicit _Partial(_Pr&& _Pred_) noexcept(is_nothrow_move_constructible_v<_Pr>) + : _Pred{in_place, _STD move(_Pred_)} {} + + public: + template + _NODISCARD constexpr auto operator()(_Rng&& _Range) const + noexcept(noexcept(filter_view{_STD forward<_Rng>(_Range), _STD move(*_Pred)})) requires requires { + filter_view{static_cast<_Rng&&>(_Range), _STD move(*_Pred)}; + } + { + // clang-format on + return filter_view{_STD forward<_Rng>(_Range), _STD move(*_Pred)}; + } + }; + + public: + // clang-format off + template + _NODISCARD constexpr auto operator()(_Rng&& _Range, _Pr _Pred) const noexcept(noexcept( + filter_view{_STD forward<_Rng>(_Range), _STD move(_Pred)})) requires requires { + filter_view{static_cast<_Rng&&>(_Range), _STD move(_Pred)}; + } { + // clang-format on + return filter_view{_STD forward<_Rng>(_Range), _STD move(_Pred)}; + } + + template <_Copy_constructible_object _Pr> + _NODISCARD constexpr auto operator()(_Pr _Pred) const noexcept(is_nothrow_move_constructible_v<_Pr>) { + // clang-format on + return _Partial<_Pr>{_STD move(_Pred)}; + } + }; + + inline constexpr _Filter_fn filter; + } // namespace views + // CLASS TEMPLATE ranges::reverse_view // clang-format off template @@ -735,64 +1010,64 @@ namespace ranges { class reverse_view : public _Cached_position_t, _Vw, reverse_view<_Vw>> { // clang-format on private: - /* [[no_unique_address]] */ _Vw _Base{}; + /* [[no_unique_address]] */ _Vw _Range{}; template using _Rev_iter = reverse_iterator>; public: reverse_view() = default; - constexpr explicit reverse_view(_Vw _Base_) noexcept(is_nothrow_move_constructible_v<_Vw>) // strengthened - : _Base(_STD move(_Base_)) {} + constexpr explicit reverse_view(_Vw _Range_) noexcept(is_nothrow_move_constructible_v<_Vw>) // strengthened + : _Range(_STD move(_Range_)) {} _NODISCARD constexpr _Vw base() const& noexcept( is_nothrow_copy_constructible_v<_Vw>) /* strengthened */ requires copy_constructible<_Vw> { - return _Base; + return _Range; } _NODISCARD constexpr _Vw base() && noexcept(is_nothrow_move_constructible_v<_Vw>) /* strengthened */ { - return _STD move(_Base); + return _STD move(_Range); } _NODISCARD constexpr _Rev_iter<_Vw> begin() { if constexpr (common_range<_Vw>) { - return _Rev_iter<_Vw>{_RANGES end(_Base)}; + return _Rev_iter<_Vw>{_RANGES end(_Range)}; } else { if (this->_Has_cache()) { - return _Rev_iter<_Vw>{this->_Get_cache(_Base)}; + return _Rev_iter<_Vw>{this->_Get_cache(_Range)}; } iterator_t<_Vw> _First; if constexpr (sized_range<_Vw>) { - _First = _RANGES next(_RANGES begin(_Base), _RANGES distance(_Base)); + _First = _RANGES next(_RANGES begin(_Range), _RANGES distance(_Range)); } else { - _First = _RANGES next(_RANGES begin(_Base), _RANGES end(_Base)); + _First = _RANGES next(_RANGES begin(_Range), _RANGES end(_Range)); } - this->_Set_cache(_Base, _First); + this->_Set_cache(_Range, _First); return _Rev_iter<_Vw>{_STD move(_First)}; } } _NODISCARD constexpr auto begin() const noexcept( - noexcept(_Rev_iter{_RANGES end(_Base)})) /* strengthened */ requires common_range { - return _Rev_iter{_RANGES end(_Base)}; + noexcept(_Rev_iter{_RANGES end(_Range)})) /* strengthened */ requires common_range { + return _Rev_iter{_RANGES end(_Range)}; } _NODISCARD constexpr _Rev_iter<_Vw> end() noexcept( - noexcept(_Rev_iter<_Vw>{_RANGES begin(_Base)})) /* strengthened */ { - return _Rev_iter<_Vw>{_RANGES begin(_Base)}; + noexcept(_Rev_iter<_Vw>{_RANGES begin(_Range)})) /* strengthened */ { + return _Rev_iter<_Vw>{_RANGES begin(_Range)}; } _NODISCARD constexpr auto end() const noexcept( - noexcept(_Rev_iter{_RANGES begin(_Base)})) /* strengthened */ requires common_range { - return _Rev_iter{_RANGES begin(_Base)}; + noexcept(_Rev_iter{_RANGES begin(_Range)})) /* strengthened */ requires common_range { + return _Rev_iter{_RANGES begin(_Range)}; } _NODISCARD constexpr auto size() noexcept( - noexcept(_RANGES size(_Base))) /* strengthened */ requires sized_range<_Vw> { - return _RANGES size(_Base); + noexcept(_RANGES size(_Range))) /* strengthened */ requires sized_range<_Vw> { + return _RANGES size(_Range); } _NODISCARD constexpr auto size() const - noexcept(noexcept(_RANGES size(_Base))) /* strengthened */ requires sized_range { - return _RANGES size(_Base); + noexcept(noexcept(_RANGES size(_Range))) /* strengthened */ requires sized_range { + return _RANGES size(_Range); } }; diff --git a/stl/inc/xutility b/stl/inc/xutility index 2c9fc57aa95..feeb6b3fe65 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -5773,6 +5773,52 @@ _NODISCARD _CONSTEXPR20 _InIt find_if(_InIt _First, const _InIt _Last, _Pr _Pred return _First; } +#ifdef __cpp_lib_concepts +namespace ranges { + // VARIABLE ranges::find_if + // concept-constrained for strict enforcement as it is used by several algorithms + template _Se, class _Pj, indirect_unary_predicate> _Pr> + _NODISCARD constexpr _It _Find_if_unchecked(_It _First, const _Se _Last, _Pr _Pred, _Pj _Proj) { + for (; _First != _Last; ++_First) { + if (_STD invoke(_Pred, _STD invoke(_Proj, *_First))) { + break; + } + } + + return _First; + } + + class _Find_if_fn : private _Not_quite_object { + public: + using _Not_quite_object::_Not_quite_object; + + template _Se, class _Pj = identity, + indirect_unary_predicate> _Pr> + _NODISCARD constexpr _It operator()(_It _First, _Se _Last, _Pr _Pred, _Pj _Proj = {}) const { + _Adl_verify_range(_First, _Last); + auto _UResult = _RANGES _Find_if_unchecked( + _Get_unwrapped(_STD move(_First)), _Get_unwrapped(_STD move(_Last)), _Pass_fn(_Pred), _Pass_fn(_Proj)); + + _Seek_wrapped(_First, _STD move(_UResult)); + return _First; + } + + template , _Pj>> _Pr> + _NODISCARD constexpr borrowed_iterator_t<_Rng> operator()(_Rng&& _Range, _Pr _Pred, _Pj _Proj = {}) const { + auto _First = _RANGES begin(_Range); + auto _UResult = _RANGES _Find_if_unchecked( + _Get_unwrapped(_STD move(_First)), _Uend(_Range), _Pass_fn(_Pred), _Pass_fn(_Proj)); + + _Seek_wrapped(_First, _STD move(_UResult)); + return _First; + } + }; + + inline constexpr _Find_if_fn find_if{_Not_quite_object::_Construct_tag{}}; +} // namespace ranges +#endif // __cpp_lib_concepts + // FUNCTION TEMPLATE lower_bound template _NODISCARD _CONSTEXPR20 _FwdIt lower_bound(_FwdIt _First, const _FwdIt _Last, const _Ty& _Val, _Pr _Pred) { diff --git a/tests/std/test.lst b/tests/std/test.lst index 933fabbb469..0d8508803c7 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -319,6 +319,8 @@ tests\P0896R4_ranges_test_machinery tests\P0896R4_ranges_to_address tests\P0896R4_views_all tests\P0896R4_views_empty +tests\P0896R4_views_filter +tests\P0896R4_views_filter_death tests\P0896R4_views_reverse tests\P0896R4_views_single tests\P0898R3_concepts diff --git a/tests/std/tests/P0896R4_ranges_range_machinery/test.cpp b/tests/std/tests/P0896R4_ranges_range_machinery/test.cpp index c8d3518b70b..32aa5b7d4ab 100644 --- a/tests/std/tests/P0896R4_ranges_range_machinery/test.cpp +++ b/tests/std/tests/P0896R4_ranges_range_machinery/test.cpp @@ -90,6 +90,7 @@ STATIC_ASSERT(test_cpo(ranges::data)); STATIC_ASSERT(test_cpo(ranges::cdata)); STATIC_ASSERT(test_cpo(ranges::views::all)); +STATIC_ASSERT(test_cpo(ranges::views::filter)); STATIC_ASSERT(test_cpo(ranges::views::reverse)); STATIC_ASSERT(test_cpo(ranges::views::single)); diff --git a/tests/std/tests/P0896R4_views_filter/env.lst b/tests/std/tests/P0896R4_views_filter/env.lst new file mode 100644 index 00000000000..62a24024479 --- /dev/null +++ b/tests/std/tests/P0896R4_views_filter/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\strict_concepts_matrix.lst diff --git a/tests/std/tests/P0896R4_views_filter/test.cpp b/tests/std/tests/P0896R4_views_filter/test.cpp new file mode 100644 index 00000000000..f5d81dbfd87 --- /dev/null +++ b/tests/std/tests/P0896R4_views_filter/test.cpp @@ -0,0 +1,384 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +using namespace std; + +// Test a silly precomposed range adaptor pipeline +constexpr auto is_even = [](const auto& x) { return x % 2 == 0; }; +using Pred = remove_const_t; +STATIC_ASSERT(is_nothrow_copy_constructible_v&& is_nothrow_move_constructible_v); + +constexpr auto pipeline = + views::filter(is_even) | views::filter(is_even) | views::filter(is_even) | views::filter(is_even); + +template > +using pipeline_t = + ranges::filter_view, Pred>, Pred>, Pred>; + +template +concept CanViewFilter = requires(Rng&& r) { + views::filter(static_cast(r), is_even); +}; + +template +constexpr bool test_one(Rng&& rng, Expected&& expected) { + using ranges::filter_view, ranges::common_range, ranges::bidirectional_range, ranges::enable_borrowed_range, + ranges::forward_range, ranges::iterator_t, ranges::prev; + + constexpr bool is_view = ranges::view>; + + using V = views::all_t; + using F = filter_view; + STATIC_ASSERT(ranges::view); + STATIC_ASSERT(ranges::input_range); + STATIC_ASSERT(forward_range == forward_range); + STATIC_ASSERT(bidirectional_range == bidirectional_range); + STATIC_ASSERT(!ranges::random_access_range); + STATIC_ASSERT(!ranges::contiguous_range); + + // Validate range adaptor object and range adaptor closure + constexpr auto filter_even = views::filter(is_even); + + // ... with lvalue argument + STATIC_ASSERT(CanViewFilter == (!is_view || copyable) ); + if constexpr (CanViewFilter) { // Validate lvalue + constexpr bool is_noexcept = !is_view || is_nothrow_copy_constructible_v; + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(views::filter(rng, is_even)) == is_noexcept); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(rng | filter_even) == is_noexcept); + + STATIC_ASSERT(same_as>); + STATIC_ASSERT(noexcept(rng | pipeline) == is_noexcept); + } + + // ... with const lvalue argument + STATIC_ASSERT(CanViewFilter&> == (!is_view || copyable) ); + if constexpr (is_view && copyable) { + constexpr bool is_noexcept = is_nothrow_copy_constructible_v; + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(views::filter(as_const(rng), is_even)) == is_noexcept); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(as_const(rng) | filter_even) == is_noexcept); + + STATIC_ASSERT(same_as&>>); + STATIC_ASSERT(noexcept(as_const(rng) | pipeline) == is_noexcept); + } else if constexpr (!is_view) { + using RC = filter_view&>, Pred>; + constexpr bool is_noexcept = + is_nothrow_constructible_v&, decltype((is_even))>; + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(views::filter(as_const(rng), is_even)) == is_noexcept); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(as_const(rng) | filter_even) == is_noexcept); + + STATIC_ASSERT(same_as&>>); + STATIC_ASSERT(noexcept(as_const(rng) | pipeline) == is_noexcept); + } + + // ... with rvalue argument + STATIC_ASSERT(CanViewFilter> == is_view || enable_borrowed_range>); + if constexpr (is_view) { + constexpr bool is_noexcept = is_nothrow_move_constructible_v; + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(views::filter(move(rng), is_even)) == is_noexcept); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(move(rng) | filter_even) == is_noexcept); + + STATIC_ASSERT(same_as>>); + STATIC_ASSERT(noexcept(move(rng) | pipeline) == is_noexcept); + } else if constexpr (enable_borrowed_range>) { + using S = decltype(ranges::subrange{declval>()}); + using RS = filter_view; + constexpr bool is_noexcept = noexcept(S{declval>()}); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(views::filter(move(rng), is_even)) == is_noexcept); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(move(rng) | filter_even) == is_noexcept); + + STATIC_ASSERT(same_as>>); + STATIC_ASSERT(noexcept(move(rng) | pipeline) == is_noexcept); + } + + // ... with const rvalue argument + STATIC_ASSERT(CanViewFilter> == (is_view && copyable) + || (!is_view && enable_borrowed_range>) ); + if constexpr (is_view && copyable) { + constexpr bool is_noexcept = is_nothrow_copy_constructible_v; + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(views::filter(as_const(rng), is_even)) == is_noexcept); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(as_const(rng) | filter_even) == is_noexcept); + + STATIC_ASSERT(same_as>>); + STATIC_ASSERT(noexcept(move(as_const(rng)) | pipeline) == is_noexcept); + } else if constexpr (!is_view && enable_borrowed_range>) { + using S = decltype(ranges::subrange{declval>()}); + using RS = filter_view; + constexpr bool is_noexcept = noexcept(S{declval>()}); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(views::filter(move(as_const(rng)), is_even)) == is_noexcept); + + STATIC_ASSERT(same_as); + STATIC_ASSERT(noexcept(move(as_const(rng)) | filter_even) == is_noexcept); + + STATIC_ASSERT(same_as>>); + STATIC_ASSERT(noexcept(move(as_const(rng)) | pipeline) == is_noexcept); + } + + // Validate deduction guide +#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, DevCom-1159442 + (void) 42; +#endif // TRANSITION, DevCom-1159442 + same_as auto r = filter_view{forward(rng), is_even}; + assert(ranges::equal(r, expected)); + if constexpr (forward_range) { + // filter_view memoizes the first iterator, let's repeat a few times for coverage. + assert(ranges::equal(r, expected)); + assert(ranges::equal(r, expected)); + assert(ranges::equal(r, expected)); + } + + { // Validate filter_view::pred + [[maybe_unused]] same_as auto pred_copy = as_const(r).pred(); + STATIC_ASSERT(noexcept(as_const(r).pred())); + } + + // Validate view_interface::empty and operator bool + const bool is_empty = ranges::empty(expected); + STATIC_ASSERT(CanMemberEmpty == forward_range); + STATIC_ASSERT(CanBool == CanMemberEmpty); + if constexpr (CanMemberEmpty) { + assert(r.empty() == is_empty); + assert(static_cast(r) == !is_empty); + } + STATIC_ASSERT(!CanMemberEmpty); + STATIC_ASSERT(!CanBool); + + // Validate filter_view::begin + STATIC_ASSERT(CanMemberBegin); + if (forward_range) { // intentionally not if constexpr + // Ditto "let's make some extra calls because memoization" + const same_as> auto i = r.begin(); + if (!is_empty) { + assert(*i == *begin(expected)); + } + assert(*r.begin() == *begin(expected)); + assert(*r.begin() == *begin(expected)); + + if constexpr (copyable) { + auto r2 = r; + const same_as> auto i2 = r2.begin(); + assert(*r2.begin() == *i2); + assert(*r2.begin() == *i2); + if (!is_empty) { + assert(*i2 == *i); + } + } + + STATIC_ASSERT(!CanBegin); + } + + // Validate filter_view::end + STATIC_ASSERT(CanMemberEnd); + if (!is_empty) { + if constexpr (common_range) { + same_as> auto i = r.end(); + if constexpr (bidirectional_range) { + assert(*prev(i) == *prev(end(expected))); + } + } else { + [[maybe_unused]] same_as> auto s = r.end(); + } + + + if constexpr (bidirectional_range && common_range && copyable) { + auto r2 = r; + assert(*prev(r2.end()) == *prev(end(expected))); + } + + STATIC_ASSERT(!CanEnd); + } + + // Validate view_interface::data + STATIC_ASSERT(!CanData); + STATIC_ASSERT(!CanData); + + // Validate view_interface::size + STATIC_ASSERT(!CanSize); + STATIC_ASSERT(!CanSize); + + // Validate view_interface::operator[] + STATIC_ASSERT(!CanIndex); + STATIC_ASSERT(!CanIndex); + + if (!is_empty) { + // Validate view_interface::front and back + STATIC_ASSERT(CanMemberFront == forward_range); + if constexpr (forward_range) { + assert(r.front() == *begin(expected)); + } + + STATIC_ASSERT(CanMemberBack == (bidirectional_range && common_range) ); + if constexpr (CanMemberBack) { + assert(r.back() == *prev(end(expected))); + } + + STATIC_ASSERT(!CanMemberFront); + STATIC_ASSERT(!CanMemberBack); + } + + // Validate filter_view::base() const& + STATIC_ASSERT(CanMemberBase == copy_constructible); + if constexpr (copy_constructible && forward_range) { + same_as auto b1 = as_const(r).base(); + STATIC_ASSERT(noexcept(as_const(r).base()) == is_nothrow_copy_constructible_v); + if (!is_empty) { + assert(*b1.begin() == *begin(expected)); + if constexpr (bidirectional_range && common_range) { + assert(*prev(b1.end(), 2) == *prev(end(expected))); // NB: depends on the test data + } + } + } + + // Validate filter_view::base() && (NB: do this last since it leaves r moved-from) +#if !defined(__clang__) && !defined(__EDG__) // TRANSITION, DevCom-1159442 + (void) 42; +#endif // TRANSITION, DevCom-1159442 + if (forward_range) { // intentionally not if constexpr + same_as auto b2 = move(r).base(); + STATIC_ASSERT(noexcept(move(r).base()) == is_nothrow_move_constructible_v); + if (!is_empty) { + assert(*b2.begin() == *begin(expected)); + if constexpr (bidirectional_range && common_range) { + assert(*prev(b2.end(), 2) == *prev(end(expected))); // NB: depends on the test data + } + } + } + + return true; +} + +static constexpr int some_ints[] = {0, 1, 2, 3, 4, 5, 6, 7}; +static constexpr int only_even_ints[] = {0, 2, 4, 6}; + +struct instantiator { + template + static constexpr void call() { + R r{some_ints}; + test_one(r, only_even_ints); + } +}; + +template +using test_range = test::range}, IsCommon, + test::CanCompare{derived_from || IsCommon == test::Common::yes}, + test::ProxyRef{!derived_from}>; + +constexpr void instantiation_test() { +#ifdef TEST_EVERYTHING + test_in(); +#else // ^^^ test all input range permutations / test only "interesting" permutations vvv + // The view is sensitive to category and commonality, but oblivious to size, differencing, and proxyness. + using test::Common; + + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); + instantiator::call>(); +#endif // TEST_EVERYTHING +} + +template > +using move_only_view = test::range}, + test::ProxyRef{!derived_from}, test::CanView::yes, test::Copyability::move_only>; + +int main() { + // Validate views + { // ... copyable + constexpr span s{some_ints}; + STATIC_ASSERT(test_one(s, only_even_ints)); + test_one(s, only_even_ints); + } + { // ... move-only + test_one(move_only_view{some_ints}, only_even_ints); + test_one(move_only_view{some_ints}, only_even_ints); + test_one(move_only_view{some_ints}, only_even_ints); + test_one(move_only_view{some_ints}, only_even_ints); + test_one(move_only_view{some_ints}, only_even_ints); + test_one(move_only_view{some_ints}, only_even_ints); + test_one(move_only_view{some_ints}, only_even_ints); + } + + // Validate non-views + { + STATIC_ASSERT(test_one(some_ints, only_even_ints)); + test_one(some_ints, only_even_ints); + } + { + vector vec(ranges::begin(some_ints), ranges::end(some_ints)); + test_one(vec, only_even_ints); + } + { + forward_list lst(ranges::begin(some_ints), ranges::end(some_ints)); + test_one(lst, only_even_ints); + } + + // Validate a non-view borrowed range + { + constexpr span s{some_ints}; + STATIC_ASSERT(test_one(s, only_even_ints)); + test_one(s, only_even_ints); + } + + // filter/reverse interaction test + { + auto fr_pipe = views::filter(is_even) | views::reverse; + auto rf_pipe = views::reverse | views::filter(is_even); + + auto r0 = some_ints | fr_pipe; + using R0 = decltype(r0); + STATIC_ASSERT(ranges::bidirectional_range && ranges::view); + assert(ranges::equal(r0, views::reverse(only_even_ints))); + + auto r1 = some_ints | rf_pipe; + using R1 = decltype(r1); + STATIC_ASSERT(ranges::bidirectional_range && ranges::view); + assert(ranges::equal(r1, views::reverse(only_even_ints))); + + assert(ranges::equal(r0, r1)); + } + + STATIC_ASSERT((instantiation_test(), true)); + instantiation_test(); +} diff --git a/tests/std/tests/P0896R4_views_filter_death/env.lst b/tests/std/tests/P0896R4_views_filter_death/env.lst new file mode 100644 index 00000000000..22f1f0230a4 --- /dev/null +++ b/tests/std/tests/P0896R4_views_filter_death/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\strict_winsdk_concepts_matrix.lst diff --git a/tests/std/tests/P0896R4_views_filter_death/test.cpp b/tests/std/tests/P0896R4_views_filter_death/test.cpp new file mode 100644 index 00000000000..e2b7e4f24f6 --- /dev/null +++ b/tests/std/tests/P0896R4_views_filter_death/test.cpp @@ -0,0 +1,156 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +#define _CONTAINER_DEBUG_LEVEL 1 + +#include +#include +#include + +#include +using namespace std; + +static int some_ints[] = {0, 1, 2, 3}; + +[[maybe_unused]] constexpr auto lambda = [x = 42](int) { return x == 42; }; +using FV = decltype(ranges::filter_view{some_ints, lambda}); + +void test_view_predicate() { + FV r; + (void) r.pred(); // value-initialized filter_view has no predicate +} + +void test_view_begin() { + FV r; + (void) r.begin(); // N4861 [range.filter.view]/3 forbids calling begin on a filter_view that holds no predicate +} + +void test_operator_star_value_initialized_iterator() { + ranges::iterator_t i{}; + (void) (*i); // cannot dereference value-initialized filter_view iterator +} + +void test_operator_star_end_iterator() { + FV r{some_ints, lambda}; + ranges::iterator_t i = ranges::next(r.begin(), r.end()); + (void) (*i); // cannot dereference end filter_view iterator +} + +void test_operator_arrow_value_initialized_iterator() { + ranges::iterator_t i{}; + (void) (i.operator->()); // cannot dereference value-initialized filter_view iterator +} + +void test_operator_arrow_end_iterator() { + FV r{some_ints, lambda}; + ranges::iterator_t i = ranges::next(r.begin(), r.end()); + (void) (i.operator->()); // cannot dereference end filter_view iterator +} + +void test_operator_preincrement_value_initialized_iterator() { + ranges::iterator_t i{}; + ++i; // cannot increment value-initialized filter_view iterator +} + +void test_operator_preincrement_after_end() { + FV r{some_ints, lambda}; + ranges::iterator_t i = ranges::next(r.begin(), r.end()); + ++i; // cannot increment filter_view iterator past end +} + +void test_operator_postincrement_value_initialized_iterator() { + ranges::iterator_t i{}; + i++; // cannot increment value-initialized filter_view iterator +} + +void test_operator_postincrement_after_end() { + FV r{some_ints, lambda}; + ranges::iterator_t i = ranges::next(r.begin(), r.end()); + i++; // cannot increment filter_view iterator past end +} + +void test_operator_predecrement_before_begin() { + FV r{some_ints, lambda}; + ranges::iterator_t i = r.begin(); + --i; // cannot increment filter_view iterator before begin +} + +void test_operator_postdecrement_before_begin() { + FV r{some_ints, lambda}; + ranges::iterator_t i = r.begin(); + i--; // cannot increment filter_view iterator before begin +} + +void test_operator_equal_incompatible_different() { + FV r0{some_ints, lambda}; + ranges::iterator_t i0 = r0.begin(); + FV r1{some_ints, lambda}; + ranges::iterator_t i1 = r1.begin(); + (void) (i0 == i1); // cannot compare incompatible filter_view iterators for equality +} + +void test_operator_equal_incompatible_value_initialized() { + FV r{some_ints, lambda}; + ranges::iterator_t i = r.begin(); + (void) (i == ranges::iterator_t{}); // cannot compare incompatible filter_view iterators for equality +} + +void test_iter_move_value_initialized_iterator() { + ranges::iterator_t i{}; + (void) ranges::iter_move(i); // cannot dereference value-initialized filter_view iterator +} + +void test_iter_swap_value_initialized_iterators() { + ranges::iterator_t i0{}; + ranges::iterator_t i1{}; + (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized filter_view iterator +} + +void test_iter_swap_value_initialized_iterator_left() { + ranges::iterator_t i0{}; + FV r{some_ints, lambda}; + ranges::iterator_t i1 = r.begin(); + (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized filter_view iterator +} + +void test_iter_swap_value_initialized_iterator_right() { + FV r{some_ints, lambda}; + ranges::iterator_t i0 = r.begin(); + ranges::iterator_t i1{}; + (void) ranges::iter_swap(i0, i1); // cannot dereference value-initialized filter_view iterator +} + +int main(int argc, char* argv[]) { + std_testing::death_test_executive exec([] {}); + +#if _ITERATOR_DEBUG_LEVEL != 0 + exec.add_death_tests({ + test_view_predicate, + test_view_begin, + + test_operator_star_value_initialized_iterator, + test_operator_star_end_iterator, + test_operator_arrow_value_initialized_iterator, + test_operator_arrow_end_iterator, + test_operator_preincrement_value_initialized_iterator, + test_operator_preincrement_after_end, + test_operator_postincrement_value_initialized_iterator, + test_operator_postincrement_after_end, + test_operator_predecrement_before_begin, + test_operator_postdecrement_before_begin, + test_operator_equal_incompatible_different, + test_operator_equal_incompatible_value_initialized, + test_iter_move_value_initialized_iterator, + test_iter_swap_value_initialized_iterators, + test_iter_swap_value_initialized_iterator_left, + test_iter_swap_value_initialized_iterator_right, + }); +#else // ^^^ test everything / test only _CONTAINER_DEBUG_LEVEL cases vvv + exec.add_death_tests({ + test_view_predicate, + test_view_begin, + }); +#endif // _ITERATOR_DEBUG_LEVEL != 0 + + return exec.run(argc, argv); +} From a569b9872024158af26f30c4b3201d3737865329 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 1 Sep 2020 12:01:39 -0700 Subject: [PATCH 2/8] Remove unused function ranges::filter_view::_Sentinel::_Equal --- stl/inc/ranges | 4 ---- 1 file changed, 4 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 1cb52a234c8..70bdc061aae 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -897,10 +897,6 @@ namespace ranges { } private: - _NODISCARD constexpr bool _Equal(const _Iterator& _It) const { - return _It._Current == _Last; - } - /* [[no_unique_address]] */ sentinel_t<_Vw> _Last{}; }; From cd2807e7791b738d1c13ef25d4d76863fd614324 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 1 Sep 2020 22:23:33 -0700 Subject: [PATCH 3/8] miscco and STL review comments --- stl/inc/ranges | 26 +++++++++++-------- stl/inc/xutility | 4 +-- tests/std/tests/P0896R4_views_filter/test.cpp | 1 - .../tests/P0896R4_views_filter_death/test.cpp | 15 +++++++++-- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 70bdc061aae..5030768fac2 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -29,7 +29,7 @@ namespace ranges { // clang-format off // CONCEPT ranges::viewable_range - template // Per P/R of LWG-3481 + template // Per proposed resolution of LWG-3481 concept viewable_range = range<_Rng> && ((view> && constructible_from, _Rng>) || (!view> && borrowed_range<_Rng>)); @@ -44,8 +44,7 @@ namespace ranges { template concept _Has_arrow = input_iterator<_It> - && (is_pointer_v<_It> - || requires(_It _Iter) { _Iter.operator->(); }); + && (is_pointer_v<_It> || _Has_member_arrow<_It&>); // clang-format on namespace _Pipe { @@ -744,17 +743,17 @@ namespace ranges { /* [[no_unique_address]] */ _Vw _Range{}; /* [[no_unique_address]] */ _Semiregular_box<_Pr> _Pred{}; - template // TRANSITION, LWG-3289 + template // TRANSITION, LWG-3289 struct _Iterator_base {}; // clang-format off - template - requires _Has_member_iterator_category - struct _Iterator_base { + template + requires _Has_member_iterator_category<_Traits> + struct _Iterator_base<_Traits> { // clang-format on using iterator_category = - conditional_t, + conditional_t, bidirectional_iterator_tag, - conditional_t, + conditional_t, forward_iterator_tag, input_iterator_tag>>; }; @@ -768,7 +767,11 @@ namespace ranges { _Iterator() = default; constexpr _Iterator(filter_view& _Parent_, iterator_t<_Vw> _Current_) noexcept( is_nothrow_move_constructible_v>) // strengthened - : _Current(_STD move(_Current_)), _Parent{_STD addressof(_Parent_)} {} + : _Current(_STD move(_Current_)), _Parent{_STD addressof(_Parent_)} { +#if _ITERATOR_DEBUG_LEVEL != 0 + _Adl_verify_range(_Current, _RANGES end(_Parent_._Range)); // FIXME: test coverage +#endif // _ITERATOR_DEBUG_LEVEL != 0 + } _NODISCARD constexpr iterator_t<_Vw> base() const& noexcept(is_nothrow_copy_constructible_v< iterator_t<_Vw>>) /* strengthened */ requires copyable> { @@ -884,7 +887,8 @@ namespace ranges { public: _Sentinel() = default; constexpr explicit _Sentinel(filter_view& _Parent) noexcept( - noexcept(_RANGES end(_Parent._Range))) // strengthened + noexcept(_RANGES end(_Parent._Range)) + && is_nothrow_move_constructible_v>) // strengthened : _Last(_RANGES end(_Parent._Range)) {} _NODISCARD constexpr sentinel_t<_Vw> base() const diff --git a/stl/inc/xutility b/stl/inc/xutility index feeb6b3fe65..75c86d22dd1 100644 --- a/stl/inc/xutility +++ b/stl/inc/xutility @@ -516,7 +516,7 @@ struct _Iter_traits_pointer<_Itraits_pointer_strategy::_Use_decltype> { }; template -concept _Has_op_arrow = requires(_Ty&& __t) { +concept _Has_member_arrow = requires(_Ty&& __t) { static_cast<_Ty&&>(__t).operator->(); }; @@ -620,7 +620,7 @@ struct _Iterator_traits_base<_It> { using value_type = typename indirectly_readable_traits<_It>::value_type; using pointer = typename _Iter_traits_pointer<( _Has_member_pointer<_It> ? _Itraits_pointer_strategy::_Use_member - : _Has_op_arrow<_It&> ? _Itraits_pointer_strategy::_Use_decltype + : _Has_member_arrow<_It&> ? _Itraits_pointer_strategy::_Use_decltype : _Itraits_pointer_strategy::_Use_void)>::template _Apply<_It>; using reference = typename _Iter_traits_reference<_Has_member_reference<_It>>::template _Apply<_It>; }; diff --git a/tests/std/tests/P0896R4_views_filter/test.cpp b/tests/std/tests/P0896R4_views_filter/test.cpp index f5d81dbfd87..d28f5addcc3 100644 --- a/tests/std/tests/P0896R4_views_filter/test.cpp +++ b/tests/std/tests/P0896R4_views_filter/test.cpp @@ -213,7 +213,6 @@ constexpr bool test_one(Rng&& rng, Expected&& expected) { [[maybe_unused]] same_as> auto s = r.end(); } - if constexpr (bidirectional_range && common_range && copyable) { auto r2 = r; assert(*prev(r2.end()) == *prev(end(expected))); diff --git a/tests/std/tests/P0896R4_views_filter_death/test.cpp b/tests/std/tests/P0896R4_views_filter_death/test.cpp index e2b7e4f24f6..875bcfe73f6 100644 --- a/tests/std/tests/P0896R4_views_filter_death/test.cpp +++ b/tests/std/tests/P0896R4_views_filter_death/test.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include using namespace std; @@ -25,6 +26,15 @@ void test_view_begin() { (void) r.begin(); // N4861 [range.filter.view]/3 forbids calling begin on a filter_view that holds no predicate } +void test_constructor_wrong_range() { + vector vec0{0, 1, 2, 3}; + vector vec1{4, 5, 6, 7}; + auto r0 = views::filter(vec0, lambda); + using R = decltype(r0); + same_as auto r1 = views::filter(vec1, lambda); + ranges::iterator_t i{r0, r1.begin().base()}; // vector iterators in range are from different containers +} + void test_operator_star_value_initialized_iterator() { ranges::iterator_t i{}; (void) (*i); // cannot dereference value-initialized filter_view iterator @@ -72,13 +82,13 @@ void test_operator_postincrement_after_end() { void test_operator_predecrement_before_begin() { FV r{some_ints, lambda}; ranges::iterator_t i = r.begin(); - --i; // cannot increment filter_view iterator before begin + --i; // cannot decrement filter_view iterator before begin } void test_operator_postdecrement_before_begin() { FV r{some_ints, lambda}; ranges::iterator_t i = r.begin(); - i--; // cannot increment filter_view iterator before begin + i--; // cannot decrement filter_view iterator before begin } void test_operator_equal_incompatible_different() { @@ -128,6 +138,7 @@ int main(int argc, char* argv[]) { test_view_predicate, test_view_begin, + test_constructor_wrong_range, test_operator_star_value_initialized_iterator, test_operator_star_end_iterator, test_operator_arrow_value_initialized_iterator, From 149c800e44dfb9b5879af67a31b7c30e83bf0abe Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Tue, 1 Sep 2020 22:31:07 -0700 Subject: [PATCH 4/8] Apply suggestions from code review --- stl/inc/ranges | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 5030768fac2..658dbde7bf9 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -769,7 +769,7 @@ namespace ranges { is_nothrow_move_constructible_v>) // strengthened : _Current(_STD move(_Current_)), _Parent{_STD addressof(_Parent_)} { #if _ITERATOR_DEBUG_LEVEL != 0 - _Adl_verify_range(_Current, _RANGES end(_Parent_._Range)); // FIXME: test coverage + _Adl_verify_range(_Current, _RANGES end(_Parent_._Range)); #endif // _ITERATOR_DEBUG_LEVEL != 0 } From 40012f240d3b48dd60d8707eec06a97e05b0328c Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 2 Sep 2020 00:28:32 -0700 Subject: [PATCH 5/8] Make _Filter_fn::_Partial an aggregate (as in the take_drop PR) --- stl/inc/ranges | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 658dbde7bf9..623bfa9c583 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -961,16 +961,9 @@ namespace ranges { class _Filter_fn { private: template - class _Partial : public _Pipe::_Base<_Partial<_Pr>> { - private: + struct _Partial : _Pipe::_Base<_Partial<_Pr>> { /* [[no_unique_address]] */ _Semiregular_box<_Pr> _Pred; - friend _Filter_fn; - - constexpr explicit _Partial(_Pr&& _Pred_) noexcept(is_nothrow_move_constructible_v<_Pr>) - : _Pred{in_place, _STD move(_Pred_)} {} - - public: template _NODISCARD constexpr auto operator()(_Rng&& _Range) const noexcept(noexcept(filter_view{_STD forward<_Rng>(_Range), _STD move(*_Pred)})) requires requires { @@ -996,7 +989,7 @@ namespace ranges { template <_Copy_constructible_object _Pr> _NODISCARD constexpr auto operator()(_Pr _Pred) const noexcept(is_nothrow_move_constructible_v<_Pr>) { // clang-format on - return _Partial<_Pr>{_STD move(_Pred)}; + return _Partial<_Pr>{._Pred = {in_place, _STD move(_Pred)}}; } }; From 6dd968e42331fcbbd7864af33dfd59a811a990be Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 3 Sep 2020 08:21:55 -0700 Subject: [PATCH 6/8] uniformly order privates first --- stl/inc/ranges | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 623bfa9c583..fc962712432 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -758,6 +758,17 @@ namespace ranges { }; class _Iterator : public _Iterator_base>> { + private: + /* [[no_unique_address]] */ iterator_t<_Vw> _Current{}; + filter_view* _Parent{}; + +#if _ITERATOR_DEBUG_LEVEL != 0 + constexpr void _Check_dereference() const noexcept { + _STL_VERIFY(_Parent != nullptr, "cannot dereference value-initialized filter_view iterator"); + _STL_VERIFY(_Current != _RANGES end(_Parent->_Range), "cannot dereference end filter_view iterator"); + } +#endif // _ITERATOR_DEBUG_LEVEL != 0 + public: using iterator_concept = conditional_t, bidirectional_iterator_tag, conditional_t, forward_iterator_tag, input_iterator_tag>>; @@ -870,20 +881,12 @@ namespace ranges { _NODISCARD constexpr bool _Equal(const sentinel_t<_Vw>& _Last) const { return _Current == _Last; } - - private: -#if _ITERATOR_DEBUG_LEVEL != 0 - constexpr void _Check_dereference() const noexcept { - _STL_VERIFY(_Parent != nullptr, "cannot dereference value-initialized filter_view iterator"); - _STL_VERIFY(_Current != _RANGES end(_Parent->_Range), "cannot dereference end filter_view iterator"); - } -#endif // _ITERATOR_DEBUG_LEVEL != 0 - - /* [[no_unique_address]] */ iterator_t<_Vw> _Current{}; - filter_view* _Parent{}; }; class _Sentinel { + private: + /* [[no_unique_address]] */ sentinel_t<_Vw> _Last{}; + public: _Sentinel() = default; constexpr explicit _Sentinel(filter_view& _Parent) noexcept( @@ -899,9 +902,6 @@ namespace ranges { _NODISCARD friend constexpr bool operator==(const _Iterator& _It, const _Sentinel& _Se) { return _It._Equal(_Se._Last); } - - private: - /* [[no_unique_address]] */ sentinel_t<_Vw> _Last{}; }; public: From 400d4da2393a2a1ae58984fbceb927702a277240 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 3 Sep 2020 13:24:07 -0700 Subject: [PATCH 7/8] Trust, but verify --- stl/inc/ranges | 3 +++ 1 file changed, 3 insertions(+) diff --git a/stl/inc/ranges b/stl/inc/ranges index fc962712432..7e03c9e6a03 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -781,6 +781,9 @@ namespace ranges { : _Current(_STD move(_Current_)), _Parent{_STD addressof(_Parent_)} { #if _ITERATOR_DEBUG_LEVEL != 0 _Adl_verify_range(_Current, _RANGES end(_Parent_._Range)); + if constexpr (forward_range<_Vw>) { + _Adl_verify_range(_RANGES begin(_Parent_._Range), _Current); + } #endif // _ITERATOR_DEBUG_LEVEL != 0 } From 74138f55d2eedc90c04badd44f3089657e56862f Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 3 Sep 2020 14:14:45 -0700 Subject: [PATCH 8/8] [NFC] extra space in comment --- tests/std/tests/P0896R4_views_filter_death/test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/std/tests/P0896R4_views_filter_death/test.cpp b/tests/std/tests/P0896R4_views_filter_death/test.cpp index 875bcfe73f6..01c4017ca98 100644 --- a/tests/std/tests/P0896R4_views_filter_death/test.cpp +++ b/tests/std/tests/P0896R4_views_filter_death/test.cpp @@ -32,7 +32,7 @@ void test_constructor_wrong_range() { auto r0 = views::filter(vec0, lambda); using R = decltype(r0); same_as auto r1 = views::filter(vec1, lambda); - ranges::iterator_t i{r0, r1.begin().base()}; // vector iterators in range are from different containers + ranges::iterator_t i{r0, r1.begin().base()}; // vector iterators in range are from different containers } void test_operator_star_value_initialized_iterator() {