[libcxx-commits] [libcxx] 4118858 - [libc++] NFCI: Restore code duplication in wrap_iter, with test.
Arthur O'Dwyer via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Jul 14 17:11:46 PDT 2021
Author: Arthur O'Dwyer
Date: 2021-07-14T20:10:52-04:00
New Revision: 4118858b4e4d072ac2ceef6cbc52088438781f39
URL: https://github.com/llvm/llvm-project/commit/4118858b4e4d072ac2ceef6cbc52088438781f39
DIFF: https://github.com/llvm/llvm-project/commit/4118858b4e4d072ac2ceef6cbc52088438781f39.diff
LOG: [libc++] NFCI: Restore code duplication in wrap_iter, with test.
It turns out that D105040 broke `std::rel_ops`; we actually do need
both a one-template-parameter and a two-template-parameter version of
all the comparison operators, because if we have only the heterogeneous
two-parameter version, then `x > x` is ambiguous:
template<class T, class U> int f(S<T>, S<U>) { return 1; }
template<class T> int f(T, T) { return 2; } // rel_ops
S<int> s; f(s,s); // ambiguous between #1 and #2
Adding the one-template-parameter version fixes the ambiguity:
template<class T, class U> int f(S<T>, S<U>) { return 1; }
template<class T> int f(T, T) { return 2; } // rel_ops
template<class T> int f(S<T>, S<T>) { return 3; }
S<int> s; f(s,s); // #3 beats both #1 and #2
We have the same problem with `reverse_iterator` as with `__wrap_iter`.
But so do libstdc++ and Microsoft, so we're not going to worry about it.
Differential Revision: https://reviews.llvm.org/D105894
Added:
libcxx/test/std/containers/iterator.rel_ops.compile.pass.cpp
Modified:
libcxx/include/__iterator/wrap_iter.h
Removed:
################################################################################
diff --git a/libcxx/include/__iterator/wrap_iter.h b/libcxx/include/__iterator/wrap_iter.h
index 4f2228c893d76..e35a372b42678 100644
--- a/libcxx/include/__iterator/wrap_iter.h
+++ b/libcxx/include/__iterator/wrap_iter.h
@@ -164,6 +164,13 @@ class __wrap_iter
template <class _Tp, size_t> friend class _LIBCPP_TEMPLATE_VIS span;
};
+template <class _Iter1>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator==(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
+{
+ return __x.base() == __y.base();
+}
+
template <class _Iter1, class _Iter2>
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
bool operator==(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
@@ -171,6 +178,17 @@ bool operator==(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y)
return __x.base() == __y.base();
}
+template <class _Iter1>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator<(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
+{
+#if _LIBCPP_DEBUG_LEVEL == 2
+ _LIBCPP_ASSERT(__get_const_db()->__less_than_comparable(&__x, &__y),
+ "Attempted to compare incomparable iterators");
+#endif
+ return __x.base() < __y.base();
+}
+
template <class _Iter1, class _Iter2>
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
bool operator<(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
@@ -182,6 +200,13 @@ bool operator<(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _
return __x.base() < __y.base();
}
+template <class _Iter1>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
+{
+ return !(__x == __y);
+}
+
template <class _Iter1, class _Iter2>
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
bool operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
@@ -189,6 +214,13 @@ bool operator!=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y)
return !(__x == __y);
}
+template <class _Iter1>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
+{
+ return __y < __x;
+}
+
template <class _Iter1, class _Iter2>
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
bool operator>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
@@ -196,6 +228,13 @@ bool operator>(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _
return __y < __x;
}
+template <class _Iter1>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator>=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
+{
+ return !(__x < __y);
+}
+
template <class _Iter1, class _Iter2>
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
bool operator>=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
@@ -203,6 +242,13 @@ bool operator>=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y)
return !(__x < __y);
}
+template <class _Iter1>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
+bool operator<=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter1>& __y) _NOEXCEPT
+{
+ return !(__y < __x);
+}
+
template <class _Iter1, class _Iter2>
_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_IF_NODEBUG
bool operator<=(const __wrap_iter<_Iter1>& __x, const __wrap_iter<_Iter2>& __y) _NOEXCEPT
diff --git a/libcxx/test/std/containers/iterator.rel_ops.compile.pass.cpp b/libcxx/test/std/containers/iterator.rel_ops.compile.pass.cpp
new file mode 100644
index 0000000000000..12efbd3fee98f
--- /dev/null
+++ b/libcxx/test/std/containers/iterator.rel_ops.compile.pass.cpp
@@ -0,0 +1,140 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: libcpp-has-no-filesystem-library
+
+// Make sure the various containers' iterators are not broken by the use of `std::rel_ops`.
+
+#include <utility> // for std::rel_ops
+
+#include <array>
+#include <deque>
+#include <forward_list>
+#include <list>
+#include <map>
+#include <set>
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <vector>
+
+#include "test_macros.h"
+
+#if TEST_STD_VER >= 11
+#include "filesystem_include.h"
+#endif
+
+#if TEST_STD_VER >= 17
+#include <string_view>
+#endif
+
+#if TEST_STD_VER >= 20
+#include <span>
+#endif
+
+using namespace std::rel_ops;
+
+template<class It, class ConstIt>
+void test_eq(It it, ConstIt cit) {
+ (void)(it == it);
+ (void)(it != it);
+ (void)(it == cit);
+ (void)(it != cit);
+ (void)(cit == it);
+ (void)(cit != it);
+ (void)(cit == cit);
+ (void)(cit != cit);
+}
+
+template<class It, class ConstIt>
+void test_lt(It it, ConstIt cit) {
+ (void)(it < it);
+ (void)(it <= it);
+ (void)(it > it);
+ (void)(it >= it);
+ (void)(it < cit);
+ (void)(it <= cit);
+ (void)(it > cit);
+ (void)(it >= cit);
+ (void)(cit < it);
+ (void)(cit <= it);
+ (void)(cit > it);
+ (void)(cit >= it);
+ (void)(cit < cit);
+ (void)(cit <= cit);
+ (void)(cit > cit);
+ (void)(cit >= cit);
+
+ // Test subtraction too, even though std::rel_ops shouldn't affect it.
+
+ (void)(it - it);
+ (void)(it - cit);
+ (void)(cit - it);
+ (void)(cit - cit);
+}
+
+template<class Container>
+void test_forward() {
+ // There is no need to distinguish "forward" from "bidirectional."
+ // libc++ already can't handle `c.rbegin() >= c.rbegin()` in the
+ // presence of std::rel_ops, and neither can Microsoft nor libstdc++.
+
+ Container c;
+ typename Container::iterator it = c.begin();
+ typename Container::const_iterator cit = c.begin();
+ test_eq(it, cit);
+}
+
+template<class Container>
+void test_random_access() {
+ Container c;
+ typename Container::iterator it = c.begin();
+ typename Container::const_iterator cit = c.begin();
+ test_eq(it, cit);
+ test_lt(it, cit);
+}
+
+template void test_random_access<std::array<int, 10> >();
+template void test_random_access<std::deque<int> >();
+template void test_forward<std::forward_list<int> >();
+template void test_forward<std::list<int> >();
+template void test_forward<std::map<int, int> >();
+template void test_forward<std::multimap<int, int> >();
+template void test_forward<std::multiset<int> >();
+template void test_forward<std::set<int> >();
+template void test_random_access<std::string>();
+template void test_forward<std::unordered_map<int, int> >();
+template void test_forward<std::unordered_multimap<int, int> >();
+template void test_forward<std::unordered_multiset<int> >();
+template void test_forward<std::unordered_set<int> >();
+template void test_random_access<std::vector<int> >();
+
+#if TEST_STD_VER >= 11
+void test_directory_iterators() {
+ fs::directory_iterator it;
+ test_eq(it, it);
+
+ fs::recursive_directory_iterator rdit;
+ test_eq(rdit, rdit);
+}
+
+template void test_forward<fs::path>();
+#endif
+
+#if TEST_STD_VER >= 17
+template void test_random_access<std::string_view>();
+#endif
+
+#if TEST_STD_VER >= 20
+void test_span() {
+ std::span<int> c;
+ std::span<int>::iterator it = c.begin(); // span has no const_iterator
+ test_eq(it, it);
+ test_lt(it, it);
+}
+#endif
More information about the libcxx-commits
mailing list