From d2554d1231d164d1d9bed312155630b093c58cd5 Mon Sep 17 00:00:00 2001 From: Michael Schellenberger Costa Date: Fri, 2 Oct 2020 09:30:58 +0200 Subject: [PATCH] Address review comments --- stl/inc/ranges | 29 +++++++++----- .../std/tests/P0896R4_views_istream/test.cpp | 38 ++++++++++++------- .../P0896R4_views_istream_death/test.cpp | 31 ++++++++------- 3 files changed, 59 insertions(+), 39 deletions(-) diff --git a/stl/inc/ranges b/stl/inc/ranges index 5d431ca260..ba8b74dbc8 100644 --- a/stl/inc/ranges +++ b/stl/inc/ranges @@ -657,7 +657,12 @@ namespace ranges { _Iterator& operator++() { #if _ITERATOR_DEBUG_LEVEL != 0 - _STL_VERIFY(*this != default_sentinel, "cannot increment istream_view iterator at end of stream"); + _STL_VERIFY(_Parent != nullptr, + "cannot increment default initialized istream_view iterator"); // Per LWG-XXXX + _STL_VERIFY( + _Parent->_Stream != nullptr, "cannot increment istream_view iterator with uninitialized stream"); + _STL_VERIFY(!_Parent->_Stream_at_end(), + "cannot increment istream_view iterator at end of stream"); // Per LWG-XXXX #endif // _ITERATOR_DEBUG_LEVEL != 0 *_Parent->_Stream >> _Parent->_Val; return *this; @@ -667,14 +672,19 @@ namespace ranges { ++*this; } - _Ty& operator*() const { + _NODISCARD _Ty& operator*() const noexcept /* strengthened */ { #if _ITERATOR_DEBUG_LEVEL != 0 - _STL_VERIFY(*this != default_sentinel, "cannot dereference istream_view iterator at end of stream"); + _STL_VERIFY(_Parent != nullptr, + "cannot dereference default initialized istream_view iterator"); // Per LWG-XXXX + _STL_VERIFY( + _Parent->_Stream != nullptr, "cannot dereference istream_view iterator with uninitialized stream"); + _STL_VERIFY(!_Parent->_Stream_at_end(), + "cannot dereference istream_view iterator at end of stream"); // Per LWG-XXXX #endif // _ITERATOR_DEBUG_LEVEL != 0 return _Parent->_Val; } - friend bool operator==(const _Iterator& _Left, default_sentinel_t) { + _NODISCARD friend bool operator==(const _Iterator& _Left, default_sentinel_t) noexcept /* strengthened */ { return _Left._Parent == nullptr || _Left._Parent->_Stream_at_end(); } }; @@ -684,27 +694,28 @@ namespace ranges { public: basic_istream_view() = default; - constexpr explicit basic_istream_view(basic_istream<_Elem, _Traits>& _Stream_) noexcept // strengthened + constexpr explicit basic_istream_view(basic_istream<_Elem, _Traits>& _Stream_) noexcept( + is_nothrow_default_constructible_v<_Ty>) // strengthened : _Stream{_STD addressof(_Stream_)} {} - constexpr auto begin() { + _NODISCARD constexpr auto begin() { if (_Stream) { *_Stream >> _Val; } return _Iterator{*this}; } - constexpr default_sentinel_t end() const noexcept { + _NODISCARD constexpr default_sentinel_t end() const noexcept { return default_sentinel; } - constexpr bool _Stream_at_end() const noexcept { + _NODISCARD constexpr bool _Stream_at_end() const noexcept { return _Stream == nullptr || !*_Stream; } }; template - basic_istream_view<_Ty, _Elem, _Traits> istream_view(basic_istream<_Elem, _Traits>& _Stream) { + _NODISCARD basic_istream_view<_Ty, _Elem, _Traits> istream_view(basic_istream<_Elem, _Traits>& _Stream) { return basic_istream_view<_Ty, _Elem, _Traits>{_Stream}; } diff --git a/tests/std/tests/P0896R4_views_istream/test.cpp b/tests/std/tests/P0896R4_views_istream/test.cpp index 2d35fed60e..a8f09acf3e 100644 --- a/tests/std/tests/P0896R4_views_istream/test.cpp +++ b/tests/std/tests/P0896R4_views_istream/test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -11,8 +12,7 @@ using namespace std; -static constexpr int expected_empty[] = {-1, -1, -1, -1, -1}; -static constexpr int expected_copied[] = {0, 1, 2, 3, -1}; +constexpr int expected_empty[] = {-1, -1, -1, -1, -1}; struct streamable { streamable() = default; @@ -29,7 +29,7 @@ struct streamable { }; template -bool test_one_type() { +constexpr bool test_one_type() { using ranges::basic_istream_view; // validate type properties @@ -42,17 +42,21 @@ bool test_one_type() { static_assert(!ranges::common_range); // validate constructor - auto nonempty_stream = istringstream{"0"}; - auto empty_intstream = istringstream{}; - same_as auto default_constructed = ranges::basic_istream_view{}; - same_as auto empty_constructed = ranges::basic_istream_view{empty_intstream}; - same_as auto value_constructed = ranges::basic_istream_view{nonempty_stream}; + auto nonempty_stream = istringstream{"0"}; + auto empty_intstream = istringstream{}; + same_as auto default_constructed = basic_istream_view{}; + same_as auto empty_constructed = basic_istream_view{empty_intstream}; + same_as auto non_empty_constructed = basic_istream_view{nonempty_stream}; // validate member begin - // NOTE: This moves the stream one element int front + // NOTE: begin() consumes the first token assert(default_constructed.begin() == default_sentinel); assert(empty_constructed.begin() == default_sentinel); - assert(value_constructed.begin() != default_sentinel); + assert(non_empty_constructed.begin() != default_sentinel); + + // validate default constructed istream::iterator + const auto default_constructed_it = ranges::iterator_t(); + assert(default_constructed_it == default_sentinel); // validate member end static_assert(same_as); @@ -75,10 +79,16 @@ bool test_one_type() { ranges::copy(empty_constructed, input_empty); assert(ranges::equal(input_empty, expected_empty)); - auto intstream = istringstream{"0 1 2 3"}; - T input_value[] = {-1, -1, -1, -1, -1}; - ranges::copy(ranges::basic_istream_view{intstream}, input_value); - assert(ranges::equal(input_value, expected_copied)); + const T expected[] = {0, 1, 2, 3, -1}; + auto intstream = istringstream{"0 1 2 3"}; + T input_value[] = {-1, -1, -1, -1, -1}; + ranges::copy(basic_istream_view{intstream}, input_value); + assert(ranges::equal(input_value, expected)); + + auto intstream_view = istringstream{"0 1 2 3"}; + T input_value_view[] = {-1, -1, -1, -1, -1}; + ranges::copy(ranges::istream_view(intstream_view), input_value_view); + assert(ranges::equal(input_value_view, expected)); return true; } diff --git a/tests/std/tests/P0896R4_views_istream_death/test.cpp b/tests/std/tests/P0896R4_views_istream_death/test.cpp index 1ee68d3dac..004652c473 100644 --- a/tests/std/tests/P0896R4_views_istream_death/test.cpp +++ b/tests/std/tests/P0896R4_views_istream_death/test.cpp @@ -13,28 +13,28 @@ using namespace std; using iview = ranges::basic_istream_view; -void test_predecrement_default_initialized() { - decltype(std::declval().begin()) it; +void test_preincrement_default_initialized() { + ranges::iterator_t it; (void) ++it; } -void test_postdecrement_default_initialized() { - decltype(std::declval().begin()) it; +void test_postincrement_default_initialized() { + ranges::iterator_t it; (void) it++; } void test_dereference_default_initialized() { - decltype(std::declval().begin()) it; + ranges::iterator_t it; (void) *it; } -void test_predecrement_no_stream() { +void test_preincrement_no_stream() { iview v; auto it = v.begin(); (void) ++it; } -void test_postdecrement_no_stream() { +void test_postincrement_no_stream() { iview v; auto it = v.begin(); (void) it++; @@ -46,15 +46,14 @@ void test_dereference_no_stream() { (void) *it; } - -void test_predecrement_end_of_stream() { +void test_preincrement_end_of_stream() { istringstream stream; iview view{stream}; auto it = view.begin(); (void) ++it; } -void test_postdecrement_end_of_stream() { +void test_postincrement_end_of_stream() { istringstream stream; iview view{stream}; auto it = view.begin(); @@ -73,14 +72,14 @@ int main(int argc, char* argv[]) { #if _ITERATOR_DEBUG_LEVEL != 0 exec.add_death_tests({ - test_predecrement_default_initialized, - test_postdecrement_default_initialized, + test_preincrement_default_initialized, + test_postincrement_default_initialized, test_dereference_default_initialized, - test_predecrement_no_stream, - test_postdecrement_no_stream, + test_preincrement_no_stream, + test_postincrement_no_stream, test_dereference_no_stream, - test_predecrement_end_of_stream, - test_postdecrement_end_of_stream, + test_preincrement_end_of_stream, + test_postincrement_end_of_stream, test_dereference_end_of_stream, }); #endif // _ITERATOR_DEBUG_LEVEL != 0