[libcxx-commits] [libcxx] 0902eb3 - [libc++] Fix common_iterator for output_iterators
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Jan 27 07:57:09 PST 2022
Author: Louis Dionne
Date: 2022-01-27T10:57:04-05:00
New Revision: 0902eb30ad714da3ed6c6a744337c9b52427f366
URL: https://github.com/llvm/llvm-project/commit/0902eb30ad714da3ed6c6a744337c9b52427f366
DIFF: https://github.com/llvm/llvm-project/commit/0902eb30ad714da3ed6c6a744337c9b52427f366.diff
LOG: [libc++] Fix common_iterator for output_iterators
We were missing a constraint in common_iterator's iterator_traits and
we were eagerly instantiating iter_value_t even when invalid.
Thanks to Casey Carter for finding this bug.
Differential Revision: https://reviews.llvm.org/D117449
Added:
Modified:
libcxx/include/__iterator/common_iterator.h
libcxx/test/std/iterators/predef.iterators/iterators.common/iterator_traits.compile.pass.cpp
libcxx/test/std/iterators/predef.iterators/iterators.common/plus_plus.pass.cpp
libcxx/test/std/iterators/predef.iterators/iterators.common/types.h
Removed:
################################################################################
diff --git a/libcxx/include/__iterator/common_iterator.h b/libcxx/include/__iterator/common_iterator.h
index df4c7bd18e8d2..605071d70928e 100644
--- a/libcxx/include/__iterator/common_iterator.h
+++ b/libcxx/include/__iterator/common_iterator.h
@@ -29,6 +29,11 @@ _LIBCPP_BEGIN_NAMESPACE_STD
#if !defined(_LIBCPP_HAS_NO_RANGES)
+template<class _Iter>
+concept __can_use_postfix_proxy =
+ constructible_from<iter_value_t<_Iter>, iter_reference_t<_Iter>> &&
+ move_constructible<iter_value_t<_Iter>>;
+
template<input_or_output_iterator _Iter, sentinel_for<_Iter> _Sent>
requires (!same_as<_Iter, _Sent> && copyable<_Iter>)
class common_iterator {
@@ -54,10 +59,6 @@ class common_iterator {
: __value(_VSTD::forward<iter_reference_t<_Iter>>(__x)) {}
public:
- constexpr static bool __valid_for_iter =
- constructible_from<iter_value_t<_Iter>, iter_reference_t<_Iter>> &&
- move_constructible<iter_value_t<_Iter>>;
-
constexpr const iter_value_t<_Iter>& operator*() const noexcept {
return __value;
}
@@ -148,7 +149,7 @@ class common_iterator {
++*this;
return __tmp;
} else if constexpr (requires (_Iter& __i) { { *__i++ } -> __referenceable; } ||
- !__postfix_proxy::__valid_for_iter) {
+ !__can_use_postfix_proxy<_Iter>) {
return _VSTD::__unchecked_get<_Iter>(__hold_)++;
} else {
__postfix_proxy __p(**this);
@@ -261,7 +262,7 @@ struct __arrow_type_or_void<_Iter, _Sent> {
using type = decltype(declval<const common_iterator<_Iter, _Sent>&>().operator->());
};
-template<class _Iter, class _Sent>
+template<input_iterator _Iter, class _Sent>
struct iterator_traits<common_iterator<_Iter, _Sent>> {
using iterator_concept = _If<forward_iterator<_Iter>,
forward_iterator_tag,
@@ -275,7 +276,6 @@ struct iterator_traits<common_iterator<_Iter, _Sent>> {
using reference = iter_reference_t<_Iter>;
};
-
#endif // !defined(_LIBCPP_HAS_NO_RANGES)
_LIBCPP_END_NAMESPACE_STD
diff --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/iterator_traits.compile.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/iterator_traits.compile.pass.cpp
index 384c8c7491f25..c55285b798f30 100644
--- a/libcxx/test/std/iterators/predef.iterators/iterators.common/iterator_traits.compile.pass.cpp
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/iterator_traits.compile.pass.cpp
@@ -14,9 +14,23 @@
#include <iterator>
+#include <cstddef>
+#include "test_iterators.h"
#include "test_macros.h"
#include "types.h"
+template<class T>
+concept HasIteratorConcept = requires { typename T::iterator_concept; };
+
+struct NonVoidOutputIterator {
+ using value_type = int;
+ using
diff erence_type = std::ptr
diff _t;
+ const NonVoidOutputIterator& operator*() const;
+ NonVoidOutputIterator& operator++();
+ NonVoidOutputIterator& operator++(int);
+ void operator=(int) const;
+};
+
void test() {
{
using Iter = simple_iterator<int*>;
@@ -43,17 +57,30 @@ void test() {
static_assert(!std::same_as<IterTraits::pointer, int*>);
static_assert(std::same_as<IterTraits::reference, int>);
}
+ // Test with an output_iterator that has a void value_type
{
- using Iter = non_const_deref_iterator<int*>;
+ using Iter = output_iterator<int*>;
using CommonIter = std::common_iterator<Iter, sentinel_type<int*>>;
using IterTraits = std::iterator_traits<CommonIter>;
- static_assert(std::same_as<IterTraits::iterator_concept, std::input_iterator_tag>);
- static_assert(std::same_as<IterTraits::iterator_category, std::input_iterator_tag>);
- static_assert(std::same_as<IterTraits::value_type, int>);
+ static_assert(!HasIteratorConcept<IterTraits>);
+ static_assert(std::same_as<IterTraits::iterator_category, std::output_iterator_tag>);
+ static_assert(std::same_as<IterTraits::value_type, void>);
static_assert(std::same_as<IterTraits::
diff erence_type, std::ptr
diff _t>);
static_assert(std::same_as<IterTraits::pointer, void>);
- static_assert(std::same_as<IterTraits::reference, int&>);
+ static_assert(std::same_as<IterTraits::reference, void>);
+ }
+ // Test with an output_iterator that has a non-void value_type
+ {
+ using CommonIter = std::common_iterator<NonVoidOutputIterator, sentinel_wrapper<NonVoidOutputIterator>>;
+ using IterTraits = std::iterator_traits<CommonIter>;
+
+ static_assert(!HasIteratorConcept<IterTraits>);
+ static_assert(std::same_as<IterTraits::iterator_category, std::output_iterator_tag>);
+ static_assert(std::same_as<IterTraits::value_type, void>);
+ static_assert(std::same_as<IterTraits::
diff erence_type, std::ptr
diff _t>);
+ static_assert(std::same_as<IterTraits::pointer, void>);
+ static_assert(std::same_as<IterTraits::reference, void>);
}
{
using Iter = cpp17_input_iterator<int*>;
diff --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/plus_plus.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/plus_plus.pass.cpp
index f3c1b77ec15ad..257c7ff5e89ef 100644
--- a/libcxx/test/std/iterators/predef.iterators/iterators.common/plus_plus.pass.cpp
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/plus_plus.pass.cpp
@@ -21,11 +21,10 @@
struct Incomplete;
void test() {
- int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
-
// Reference: http://eel.is/c++draft/iterators.common#common.iter.nav-5
// Case 2: can-reference
{
+ int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
auto iter1 = simple_iterator<int*>(buffer);
auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
auto commonSent1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(sentinel_type<int*>{buffer + 8});
@@ -43,6 +42,7 @@ void test() {
// Case 2: can-reference
{
+ int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
auto iter1 = value_iterator<int*>(buffer);
auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
auto commonSent1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(sentinel_type<int*>{buffer + 8});
@@ -60,6 +60,7 @@ void test() {
// Case 3: postfix-proxy
{
+ int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
auto iter1 = void_plus_plus_iterator<int*>(buffer);
auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
auto commonSent1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(sentinel_type<int*>{buffer + 8});
@@ -77,13 +78,13 @@ void test() {
// Case 2: where this is not referencable or move constructible
{
+ int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
auto iter1 = value_type_not_move_constructible_iterator<int*>(buffer);
auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
auto commonSent1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(sentinel_type<int*>{buffer + 8});
commonIter1++;
- // Note: postfix operator++ returns void.
- // assert(*(commonIter1++) == 1);
+ ASSERT_SAME_TYPE(decltype(commonIter1++), void);
assert(*commonIter1 == 2);
assert(*(++commonIter1) == 3);
assert(*commonIter1 == 3);
@@ -97,6 +98,7 @@ void test() {
// Case 2: can-reference
{
+ int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
auto iter1 = cpp17_input_iterator<int*>(buffer);
auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
auto commonSent1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(sentinel_type<int*>{buffer + 8});
@@ -114,6 +116,7 @@ void test() {
// Case 1: forward_iterator
{
+ int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
auto iter1 = forward_iterator<int*>(buffer);
auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
auto commonSent1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(sentinel_type<int*>{buffer + 8});
@@ -131,6 +134,7 @@ void test() {
// Case 1: forward_iterator
{
+ int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
auto iter1 = random_access_iterator<int*>(buffer);
auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
auto commonSent1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(sentinel_type<int*>{buffer + 8});
@@ -145,6 +149,31 @@ void test() {
}
assert(commonIter1 == commonSent1);
}
+
+ // Increment a common_iterator<output_iterator>: iter_value_t is not always valid for
+ // output iterators (it isn't for our test output_iterator). This is worth testing
+ // because it gets tricky when we define operator++(int).
+ {
+ int buffer[] = {0, 1, 2, 3, 4};
+ using Common = std::common_iterator<output_iterator<int*>, sentinel_type<int*>>;
+ auto iter = Common(output_iterator<int*>(buffer));
+ auto sent = Common(sentinel_type<int*>{buffer + 5});
+
+ *iter++ = 90;
+ assert(buffer[0] == 90);
+
+ *iter = 91;
+ assert(buffer[1] == 91);
+
+ *++iter = 92;
+ assert(buffer[2] == 92);
+
+ iter++;
+ iter++;
+ assert(iter != sent);
+ iter++;
+ assert(iter == sent);
+ }
}
int main(int, char**) {
diff --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/types.h b/libcxx/test/std/iterators/predef.iterators/iterators.common/types.h
index 03b94f63f631e..76981c1e1a01e 100644
--- a/libcxx/test/std/iterators/predef.iterators/iterators.common/types.h
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/types.h
@@ -157,30 +157,6 @@ class comparable_iterator
}
};
-template <class It>
-class non_const_deref_iterator
-{
- It it_;
-
-public:
- typedef std::input_iterator_tag iterator_category;
- typedef typename std::iterator_traits<It>::value_type value_type;
- typedef typename std::iterator_traits<It>::
diff erence_type
diff erence_type;
- typedef It pointer;
- typedef typename std::iterator_traits<It>::reference reference;
-
- constexpr It base() const {return it_;}
-
- non_const_deref_iterator() = default;
- explicit constexpr non_const_deref_iterator(It it) : it_(it) {}
-
- constexpr reference operator*() {return *it_;} // Note: non-const.
-
- constexpr non_const_deref_iterator& operator++() {++it_; return *this;}
- constexpr non_const_deref_iterator operator++(int)
- {non_const_deref_iterator tmp(*this); ++(*this); return tmp;}
-};
-
template<class T>
struct sentinel_type {
T base;
More information about the libcxx-commits
mailing list