From 66a42c5c71011f072becaf2e78016cf916abe639 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Fri, 4 Sep 2020 22:54:15 -0700 Subject: [PATCH 1/3] P0896R4 stream iterator changes Affects `istream_iterator`, `ostream_iterator`, `istreambuf_iterator`, and `ostreambuf_iterator`. --- stl/inc/iterator | 126 +++++++++++------- tests/std/test.lst | 1 + .../tests/P0896R4_stream_iterators/env.lst | 4 + .../tests/P0896R4_stream_iterators/test.cpp | 125 +++++++++++++++++ 4 files changed, 211 insertions(+), 45 deletions(-) create mode 100644 tests/std/tests/P0896R4_stream_iterators/env.lst create mode 100644 tests/std/tests/P0896R4_stream_iterators/test.cpp diff --git a/stl/inc/iterator b/stl/inc/iterator index 6c36d930059..e2fec87f17a 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -227,34 +227,38 @@ private: // CLASS TEMPLATE istream_iterator template , class _Diff = ptrdiff_t> -class istream_iterator { // wrap _Ty extracts from input stream as input iterator +class istream_iterator { public: using iterator_category = input_iterator_tag; using value_type = _Ty; using difference_type = _Diff; using pointer = const _Ty*; using reference = const _Ty&; - - using char_type = _Elem; - using traits_type = _Traits; - using istream_type = basic_istream<_Elem, _Traits>; + using char_type = _Elem; + using traits_type = _Traits; + using istream_type = basic_istream<_Elem, _Traits>; static_assert(conjunction_v, is_copy_constructible<_Ty>, is_copy_assignable<_Ty>>, "istream_iterator requires T to be default constructible, copy constructible, and copy assignable. " "(N4835 [istream.iterator]/2)"); - constexpr istream_iterator() {} + constexpr istream_iterator() noexcept(is_nothrow_default_constructible_v<_Ty>) /* strengthened */ {} + +#ifdef __cpp_lib_concepts + constexpr istream_iterator(default_sentinel_t) noexcept(is_nothrow_default_constructible_v<_Ty>) // strengthened + {} +#endif // __cpp_lib_concepts istream_iterator(istream_type& _Istr) : _Myistr(_STD addressof(_Istr)) { _Getval(); } - _NODISCARD const _Ty& operator*() const { + _NODISCARD const _Ty& operator*() const noexcept /* strengthened */ { _STL_ASSERT(_Myistr, "The stored stream pointer in_stream must be non-null"); return _Myval; } - _NODISCARD const _Ty* operator->() const { + _NODISCARD const _Ty* operator->() const noexcept /* strengthened */ { _STL_ASSERT(_Myistr, "The stored stream pointer in_stream must be non-null"); return _STD addressof(_Myval); } @@ -270,10 +274,16 @@ public: return _Tmp; } - bool _Equal(const istream_iterator& _Right) const { + _NODISCARD bool _Equal(const istream_iterator& _Right) const noexcept { return _Myistr == _Right._Myistr; } +#ifdef __cpp_lib_concepts + _NODISCARD friend bool operator==(const istream_iterator& _Left, default_sentinel_t) noexcept /* strengthened */ { + return !_Left._Myistr; + } +#endif // __cpp_lib_concepts + private: void _Getval() { // get a _Ty value if possible _STL_ASSERT(_Myistr, "The stored stream pointer in_stream must be non-null"); @@ -288,31 +298,37 @@ private: template _NODISCARD bool operator==(const istream_iterator<_Ty, _Elem, _Traits, _Diff>& _Left, - const istream_iterator<_Ty, _Elem, _Traits, _Diff>& _Right) { + const istream_iterator<_Ty, _Elem, _Traits, _Diff>& _Right) noexcept /* strengthened */ { return _Left._Equal(_Right); } template _NODISCARD bool operator!=(const istream_iterator<_Ty, _Elem, _Traits, _Diff>& _Left, - const istream_iterator<_Ty, _Elem, _Traits, _Diff>& _Right) { + const istream_iterator<_Ty, _Elem, _Traits, _Diff>& _Right) noexcept /* strengthened */ { return !(_Left == _Right); } // CLASS TEMPLATE ostream_iterator template > -class ostream_iterator { // wrap _Ty inserts to output stream as output iterator +class ostream_iterator { public: using iterator_category = output_iterator_tag; using value_type = void; - using difference_type = void; - using pointer = void; - using reference = void; - +#ifdef __cpp_lib_concepts + using difference_type = ptrdiff_t; +#else + using difference_type = void; +#endif // __cpp_lib_concepts + using pointer = void; + using reference = void; using char_type = _Elem; using traits_type = _Traits; using ostream_type = basic_ostream<_Elem, _Traits>; - ostream_iterator(ostream_type& _Ostr, const _Elem* const _Delim = nullptr) +#ifdef __cpp_lib_concepts + constexpr ostream_iterator() noexcept = default; +#endif // __cpp_lib_concepts + ostream_iterator(ostream_type& _Ostr, const _Elem* const _Delim = nullptr) noexcept /* strengthened */ : _Mydelim(_Delim), _Myostr(_STD addressof(_Ostr)) {} ostream_iterator& operator=(const _Ty& _Val) { // insert value into output stream, followed by delimiter @@ -324,21 +340,21 @@ public: return *this; } - _NODISCARD ostream_iterator& operator*() { // pretend to return designated value + _NODISCARD ostream_iterator& operator*() noexcept /* strengthened */ { return *this; } - ostream_iterator& operator++() { // pretend to preincrement + ostream_iterator& operator++() noexcept /* strengthened */ { return *this; } - ostream_iterator& operator++(int) { // pretend to postincrement + ostream_iterator& operator++(int) noexcept /* strengthened */ { return *this; } -protected: - const _Elem* _Mydelim; // pointer to delimiter string (NB: not freed) - ostream_type* _Myostr; // pointer to output stream +private: + const _Elem* _Mydelim = nullptr; // pointer to delimiter string (NB: not freed) + ostream_type* _Myostr = nullptr; // pointer to output stream }; // CLASS TEMPLATE istreambuf_iterator @@ -350,30 +366,33 @@ public: using difference_type = typename _Traits::off_type; using pointer = const _Elem*; using reference = _Elem; - - using char_type = _Elem; - using traits_type = _Traits; - using streambuf_type = basic_streambuf<_Elem, _Traits>; - using istream_type = basic_istream<_Elem, _Traits>; - - using int_type = typename traits_type::int_type; + using char_type = _Elem; + using traits_type = _Traits; + using int_type = typename traits_type::int_type; + using streambuf_type = basic_streambuf<_Elem, _Traits>; + using istream_type = basic_istream<_Elem, _Traits>; constexpr istreambuf_iterator() noexcept : _Strbuf(nullptr), _Got(true), _Val() {} - - istreambuf_iterator(streambuf_type* _Sb) noexcept : _Strbuf(_Sb), _Got(!_Sb), _Val() {} +#ifdef __cpp_lib_concepts + constexpr istreambuf_iterator(default_sentinel_t) noexcept : _Strbuf(nullptr), _Got(true), _Val() {} +#endif // __cpp_lib_concepts istreambuf_iterator(istream_type& _Istr) noexcept : _Strbuf(_Istr.rdbuf()), _Got(!_Strbuf), _Val() {} + istreambuf_iterator(streambuf_type* _Sb) noexcept : _Strbuf(_Sb), _Got(!_Sb), _Val() {} + private: class _Istreambuf_proxy { public: - _NODISCARD _Elem operator*() const { + _NODISCARD _Elem operator*() const noexcept(is_nothrow_copy_constructible_v<_Elem>) /* strengthened */ { return _Keep; } private: friend istreambuf_iterator; - _Istreambuf_proxy(streambuf_type* _Strbuf_, _Elem _Keep_) : _Strbuf(_Strbuf_), _Keep(_Keep_) {} + _Istreambuf_proxy(streambuf_type* _Strbuf_, _Elem _Keep_) noexcept( + is_nothrow_copy_constructible_v<_Elem>) // strengthened + : _Strbuf(_Strbuf_), _Keep(_Keep_) {} streambuf_type* _Strbuf; _Elem _Keep; @@ -425,6 +444,16 @@ public: return (!_Strbuf && !_Right._Strbuf) || (_Strbuf && _Right._Strbuf); } +#ifdef __cpp_lib_concepts + _NODISCARD friend bool operator==(const istreambuf_iterator& _Left, default_sentinel_t) { + if (!_Left._Got) { + _Left._Peek(); + } + + return !_Left._Strbuf; + } +#endif // __cpp_lib_concepts + private: void _Inc() { // skip to next input element if (!_Strbuf || traits_type::eq_int_type(traits_type::eof(), _Strbuf->sbumpc())) { @@ -470,18 +499,25 @@ class ostreambuf_iterator { // wrap stream buffer as output iterator public: using iterator_category = output_iterator_tag; using value_type = void; - using difference_type = void; - using pointer = void; - using reference = void; - +#ifdef __cpp_lib_concepts + using difference_type = ptrdiff_t; +#else + using difference_type = void; +#endif // __cpp_lib_concepts + using pointer = void; + using reference = void; using char_type = _Elem; using traits_type = _Traits; using streambuf_type = basic_streambuf<_Elem, _Traits>; using ostream_type = basic_ostream<_Elem, _Traits>; - ostreambuf_iterator(streambuf_type* _Sb) noexcept : _Failed(false), _Strbuf(_Sb) {} +#ifdef __cpp_lib_concepts + constexpr ostreambuf_iterator() noexcept = default; +#endif // __cpp_lib_concepts + + ostreambuf_iterator(streambuf_type* _Sb) noexcept : _Strbuf(_Sb) {} - ostreambuf_iterator(ostream_type& _Ostr) noexcept : _Failed(false), _Strbuf(_Ostr.rdbuf()) {} + ostreambuf_iterator(ostream_type& _Ostr) noexcept : _Strbuf(_Ostr.rdbuf()) {} ostreambuf_iterator& operator=(_Elem _Right) { // store element and increment if (!_Strbuf || traits_type::eq_int_type(_Traits::eof(), _Strbuf->sputc(_Right))) { @@ -491,15 +527,15 @@ public: return *this; } - _NODISCARD ostreambuf_iterator& operator*() { // pretend to get designated element + _NODISCARD ostreambuf_iterator& operator*() noexcept /* strengthened */ { return *this; } - ostreambuf_iterator& operator++() { // pretend to preincrement + ostreambuf_iterator& operator++() noexcept /* strengthened */ { return *this; } - ostreambuf_iterator& operator++(int) { // pretend to postincrement + ostreambuf_iterator& operator++(int) noexcept /* strengthened */ { return *this; } @@ -508,8 +544,8 @@ public: } private: - bool _Failed; // true if any stores have failed - streambuf_type* _Strbuf; // the wrapped stream buffer + bool _Failed = false; // true if any stores have failed + streambuf_type* _Strbuf = nullptr; }; #ifdef __cpp_lib_concepts diff --git a/tests/std/test.lst b/tests/std/test.lst index da7a77478ed..f988e97f204 100644 --- a/tests/std/test.lst +++ b/tests/std/test.lst @@ -319,6 +319,7 @@ tests\P0896R4_ranges_ref_view tests\P0896R4_ranges_subrange tests\P0896R4_ranges_test_machinery tests\P0896R4_ranges_to_address +tests\P0896R4_stream_iterators tests\P0896R4_views_all tests\P0896R4_views_drop tests\P0896R4_views_empty diff --git a/tests/std/tests/P0896R4_stream_iterators/env.lst b/tests/std/tests/P0896R4_stream_iterators/env.lst new file mode 100644 index 00000000000..19f025bd0e6 --- /dev/null +++ b/tests/std/tests/P0896R4_stream_iterators/env.lst @@ -0,0 +1,4 @@ +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +RUNALL_INCLUDE ..\usual_matrix.lst diff --git a/tests/std/tests/P0896R4_stream_iterators/test.cpp b/tests/std/tests/P0896R4_stream_iterators/test.cpp new file mode 100644 index 00000000000..e4e2a8fd725 --- /dev/null +++ b/tests/std/tests/P0896R4_stream_iterators/test.cpp @@ -0,0 +1,125 @@ +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception + +// Covers Ranges changes to istream_iterator, ostream_iterator, istreambuf_iterator, and ostreambuf_iterator + +#include +#include +#include +#include +#include +#include + +using namespace std; + +#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__) + +template , class Diff = ptrdiff_t> +void test_istream() { + using I = istream_iterator; + + // Also test strengthened noexcept on some pre-existing operations + const I i{}; + STATIC_ASSERT(is_nothrow_default_constructible_v == is_nothrow_default_constructible_v); + STATIC_ASSERT(noexcept(*i)); + STATIC_ASSERT(noexcept(i.operator->())); + + STATIC_ASSERT(noexcept(i == i)); + STATIC_ASSERT(noexcept(i != i)); + +#ifdef __cpp_lib_concepts + STATIC_ASSERT(input_iterator); + + if constexpr (is_scalar_v) { + constexpr I fromDefaultSentinel{default_sentinel}; + assert(fromDefaultSentinel == I{}); + assert(!(fromDefaultSentinel != I{})); + STATIC_ASSERT(is_nothrow_constructible_v); + } + + assert(i == default_sentinel); + STATIC_ASSERT(noexcept(i == default_sentinel)); + assert(default_sentinel == i); + STATIC_ASSERT(noexcept(default_sentinel == i)); + assert(!(i != default_sentinel)); + STATIC_ASSERT(noexcept(!(i != default_sentinel))); + assert(!(default_sentinel != i)); + STATIC_ASSERT(noexcept(!(default_sentinel != i))); +#endif // __cpp_lib_concepts +} + +template > +void test_ostream(basic_ostream& os) { + using I = ostream_iterator; + + // Also tests strengthened noexcept on some pre-existing operations + I i{os}; + STATIC_ASSERT(is_nothrow_constructible_v&>); + STATIC_ASSERT(is_nothrow_constructible_v&, const CharT*>); + STATIC_ASSERT(noexcept(*i)); + STATIC_ASSERT(noexcept(++i)); + STATIC_ASSERT(noexcept(i++)); + +#ifdef __cpp_lib_concepts + STATIC_ASSERT(output_iterator); + + STATIC_ASSERT(is_same_v); + { [[maybe_unused]] constexpr I constexprConstructed{}; } + STATIC_ASSERT(is_nothrow_default_constructible_v); +#endif // __cpp_lib_concepts +} + +template > +void test_istreambuf() { +#ifdef __cpp_lib_concepts + using I = istreambuf_iterator; + STATIC_ASSERT(input_iterator); + + constexpr I i{default_sentinel}; + assert(i == I{}); + assert(!(i != I{})); + STATIC_ASSERT(is_nothrow_constructible_v); + + assert(i == default_sentinel); + assert(default_sentinel == i); + assert(!(i != default_sentinel)); + assert(!(default_sentinel != i)); +#endif // __cpp_lib_concepts +} + +template > +void test_ostreambuf(basic_ostream& os) { + using I = ostreambuf_iterator; + + // Also tests strengthened noexcept on some pre-existing operations + I i{os}; + STATIC_ASSERT(noexcept(*i)); + STATIC_ASSERT(noexcept(++i)); + STATIC_ASSERT(noexcept(i++)); + +#ifdef __cpp_lib_concepts + STATIC_ASSERT(output_iterator); + + STATIC_ASSERT(is_same_v); + { [[maybe_unused]] constexpr I constexprConstructed{}; } + STATIC_ASSERT(is_nothrow_default_constructible_v); +#endif // __cpp_lib_concepts +} + +int main() { + test_istream(); + test_istream(); + test_istream(); + test_istream(); + + test_ostream(cout); + test_ostream(cout); + test_ostream(wcout); + test_ostream(wcout); + + test_istreambuf(); + test_istreambuf(); + + test_ostreambuf(cout); + test_ostreambuf(wcout); +} From 7f2e171c2926a3ca11c4af496d458d6e408895b7 Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Thu, 10 Sep 2020 08:32:50 -0700 Subject: [PATCH 2/3] XFAIL libc++ tests that expect ostream{,buf}_iterator::difference_type to be void --- tests/libcxx/expected_results.txt | 4 +++- tests/libcxx/skipped_tests.txt | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index 594943c6aee..3c3018a6afd 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -751,10 +751,12 @@ std/language.support/cmp/cmp.weakord/weakord.pass.cpp FAIL # error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax std/containers/sequences/array/array.creation/to_array.pass.cpp:0 FAIL -# Tests that need to learn that insert iterators have non-void difference type in C++20 +# Tests that need to learn that iterators have non-void difference types in C++20 std/iterators/predef.iterators/insert.iterators/back.insert.iterator/types.pass.cpp FAIL std/iterators/predef.iterators/insert.iterators/front.insert.iterator/types.pass.cpp FAIL std/iterators/predef.iterators/insert.iterators/insert.iterator/types.pass.cpp FAIL +std/iterators/stream.iterators/ostream.iterator/types.pass.cpp FAIL +std/iterators/stream.iterators/ostreambuf.iterator/types.pass.cpp FAIL # Tests emit warning C4244: 'argument': conversion from 'T' to 'const std::complex::_Ty', possible loss of data std/numerics/complex.number/cmplx.over/conj.pass.cpp:0 FAIL diff --git a/tests/libcxx/skipped_tests.txt b/tests/libcxx/skipped_tests.txt index 2ad721b7ee6..307b20f5561 100644 --- a/tests/libcxx/skipped_tests.txt +++ b/tests/libcxx/skipped_tests.txt @@ -751,10 +751,12 @@ language.support\cmp\cmp.weakord\weakord.pass.cpp # error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax containers\sequences\array\array.creation\to_array.pass.cpp -# Tests that need to learn that insert iterators have non-void difference type in C++20 +# Tests that need to learn that iterators have non-void difference types in C++20 iterators\predef.iterators\insert.iterators\back.insert.iterator\types.pass.cpp iterators\predef.iterators\insert.iterators\front.insert.iterator\types.pass.cpp iterators\predef.iterators\insert.iterators\insert.iterator\types.pass.cpp +iterators\stream.iterators\ostream.iterator\types.pass.cpp +iterators\stream.iterators\ostreambuf.iterator\types.pass.cpp # Tests emit warning C4244: 'argument': conversion from 'T' to 'const std::complex::_Ty', possible loss of data numerics\complex.number\cmplx.over\conj.pass.cpp From cef68fc4e17d940301960616803f4da9275055ca Mon Sep 17 00:00:00 2001 From: Casey Carter Date: Wed, 16 Sep 2020 21:51:20 -0700 Subject: [PATCH 3/3] Review comments --- stl/inc/iterator | 9 ++--- .../tests/P0896R4_stream_iterators/test.cpp | 33 ++++++++++--------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/stl/inc/iterator b/stl/inc/iterator index e2fec87f17a..8df1937d8b3 100644 --- a/stl/inc/iterator +++ b/stl/inc/iterator @@ -318,7 +318,7 @@ public: using difference_type = ptrdiff_t; #else using difference_type = void; -#endif // __cpp_lib_concepts +#endif using pointer = void; using reference = void; using char_type = _Elem; @@ -328,6 +328,7 @@ public: #ifdef __cpp_lib_concepts constexpr ostream_iterator() noexcept = default; #endif // __cpp_lib_concepts + ostream_iterator(ostream_type& _Ostr, const _Elem* const _Delim = nullptr) noexcept /* strengthened */ : _Mydelim(_Delim), _Myostr(_STD addressof(_Ostr)) {} @@ -359,7 +360,7 @@ private: // CLASS TEMPLATE istreambuf_iterator template -class istreambuf_iterator { // wrap stream buffer as input iterator +class istreambuf_iterator { public: using iterator_category = input_iterator_tag; using value_type = _Elem; @@ -495,7 +496,7 @@ _NODISCARD bool operator!=( // CLASS TEMPLATE ostreambuf_iterator template -class ostreambuf_iterator { // wrap stream buffer as output iterator +class ostreambuf_iterator { public: using iterator_category = output_iterator_tag; using value_type = void; @@ -503,7 +504,7 @@ public: using difference_type = ptrdiff_t; #else using difference_type = void; -#endif // __cpp_lib_concepts +#endif using pointer = void; using reference = void; using char_type = _Elem; diff --git a/tests/std/tests/P0896R4_stream_iterators/test.cpp b/tests/std/tests/P0896R4_stream_iterators/test.cpp index e4e2a8fd725..e41a74fdb1d 100644 --- a/tests/std/tests/P0896R4_stream_iterators/test.cpp +++ b/tests/std/tests/P0896R4_stream_iterators/test.cpp @@ -4,6 +4,7 @@ // Covers Ranges changes to istream_iterator, ostream_iterator, istreambuf_iterator, and ostreambuf_iterator #include +#include #include #include #include @@ -15,7 +16,7 @@ using namespace std; #define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__) template , class Diff = ptrdiff_t> -void test_istream() { +void test_istream_iterator() { using I = istream_iterator; // Also test strengthened noexcept on some pre-existing operations @@ -49,7 +50,7 @@ void test_istream() { } template > -void test_ostream(basic_ostream& os) { +void test_ostream_iterator(basic_ostream& os) { using I = ostream_iterator; // Also tests strengthened noexcept on some pre-existing operations @@ -70,7 +71,7 @@ void test_ostream(basic_ostream& os) { } template > -void test_istreambuf() { +void test_istreambuf_iterator() { #ifdef __cpp_lib_concepts using I = istreambuf_iterator; STATIC_ASSERT(input_iterator); @@ -88,7 +89,7 @@ void test_istreambuf() { } template > -void test_ostreambuf(basic_ostream& os) { +void test_ostreambuf_iterator(basic_ostream& os) { using I = ostreambuf_iterator; // Also tests strengthened noexcept on some pre-existing operations @@ -107,19 +108,19 @@ void test_ostreambuf(basic_ostream& os) { } int main() { - test_istream(); - test_istream(); - test_istream(); - test_istream(); + test_istream_iterator(); + test_istream_iterator(); + test_istream_iterator(); + test_istream_iterator(); - test_ostream(cout); - test_ostream(cout); - test_ostream(wcout); - test_ostream(wcout); + test_ostream_iterator(cout); + test_ostream_iterator(cout); + test_ostream_iterator(wcout); + test_ostream_iterator(wcout); - test_istreambuf(); - test_istreambuf(); + test_istreambuf_iterator(); + test_istreambuf_iterator(); - test_ostreambuf(cout); - test_ostreambuf(wcout); + test_ostreambuf_iterator(cout); + test_ostreambuf_iterator(wcout); }