[libcxx-commits] [libcxx] [libc++] Make `flat_(multi)map`'s iterators require `operator<=> ` (PR #131290)
via libcxx-commits
libcxx-commits at lists.llvm.org
Fri Mar 14 00:33:03 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: A. Jiang (frederick-vs-ja)
<details>
<summary>Changes</summary>
Flat container adaptors require the iterators of underlying containers to be random access, and it is required that random access container iterators must support three-way comparison
([container.reqmts]/39 - /41).
As a result, we can and perhaps should reject containers with unexpected iterator types, as they're invalid since C++20.
Closes #<!-- -->126660.
---
Patch is 21.57 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/131290.diff
7 Files Affected:
- (modified) libcxx/include/__flat_map/key_value_iterator.h (+8-3)
- (modified) libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.pass.cpp (+26-28)
- (added) libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.verify.cpp (+77)
- (modified) libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.pass.cpp (+26-28)
- (added) libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.verify.cpp (+77)
- (modified) libcxx/test/support/MinSequenceContainer.h (+3-1)
- (modified) libcxx/test/support/test_iterators.h (+129)
``````````diff
diff --git a/libcxx/include/__flat_map/key_value_iterator.h b/libcxx/include/__flat_map/key_value_iterator.h
index 3ebb653deb197..e7c093a8e52c7 100644
--- a/libcxx/include/__flat_map/key_value_iterator.h
+++ b/libcxx/include/__flat_map/key_value_iterator.h
@@ -10,12 +10,14 @@
#ifndef _LIBCPP___FLAT_MAP_KEY_VALUE_ITERATOR_H
#define _LIBCPP___FLAT_MAP_KEY_VALUE_ITERATOR_H
+#include <__compare/ordering.h>
#include <__compare/three_way_comparable.h>
#include <__concepts/convertible_to.h>
#include <__config>
#include <__iterator/iterator_traits.h>
#include <__memory/addressof.h>
#include <__type_traits/conditional.h>
+#include <__type_traits/is_same.h>
#include <__utility/move.h>
#include <__utility/pair.h>
@@ -139,9 +141,12 @@ struct __key_value_iterator {
return !(__x < __y);
}
- _LIBCPP_HIDE_FROM_ABI friend auto operator<=>(const __key_value_iterator& __x, const __key_value_iterator& __y)
- requires three_way_comparable<__key_iterator>
- {
+ _LIBCPP_HIDE_FROM_ABI friend strong_ordering
+ operator<=>(const __key_value_iterator& __x, const __key_value_iterator& __y) {
+ static_assert(three_way_comparable<__key_iterator>,
+ "random accesss iterator not supporting three-way comparison is invalid for container");
+ static_assert(is_same_v<decltype(__x.__key_iter_ <=> __y.__key_iter_), strong_ordering>,
+ "three-way comparison between random accesss container iterators must return std::strong_ordering");
return __x.__key_iter_ <=> __y.__key_iter_;
}
diff --git a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.pass.cpp
index 1975d0ed86cc8..67e904ea60de6 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.pass.cpp
@@ -115,34 +115,32 @@ void test() {
assert(cri2 >= cri2);
assert(!(cri1 >= cri2));
- if constexpr (std::three_way_comparable<KI>) {
- static_assert(std::three_way_comparable<I>); // ...of course the wrapped iterators still support <=>.
- static_assert(std::three_way_comparable<CI>);
- static_assert(std::three_way_comparable<RI>);
- static_assert(std::three_way_comparable<CRI>);
- static_assert(std::same_as<decltype(I() <=> I()), std::strong_ordering>);
- static_assert(std::same_as<decltype(I() <=> CI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(CI() <=> CI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(RI() <=> RI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(RI() <=> CRI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(CRI() <=> CRI()), std::strong_ordering>);
-
- assert(i1 <=> i1 == std::strong_ordering::equivalent);
- assert(i1 <=> i2 == std::strong_ordering::less);
- assert(i2 <=> i1 == std::strong_ordering::greater);
-
- assert(ci1 <=> ci1 == std::strong_ordering::equivalent);
- assert(ci1 <=> ci2 == std::strong_ordering::less);
- assert(ci2 <=> ci1 == std::strong_ordering::greater);
-
- assert(ri1 <=> ri1 == std::strong_ordering::equivalent);
- assert(ri1 <=> ri2 == std::strong_ordering::less);
- assert(ri2 <=> ri1 == std::strong_ordering::greater);
-
- assert(cri1 <=> cri1 == std::strong_ordering::equivalent);
- assert(cri1 <=> cri2 == std::strong_ordering::less);
- assert(cri2 <=> cri1 == std::strong_ordering::greater);
- }
+ static_assert(std::three_way_comparable<I>); // ...of course the wrapped iterators still support <=>.
+ static_assert(std::three_way_comparable<CI>);
+ static_assert(std::three_way_comparable<RI>);
+ static_assert(std::three_way_comparable<CRI>);
+ static_assert(std::same_as<decltype(I() <=> I()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(I() <=> CI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(CI() <=> CI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(RI() <=> RI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(RI() <=> CRI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(CRI() <=> CRI()), std::strong_ordering>);
+
+ assert(i1 <=> i1 == std::strong_ordering::equivalent);
+ assert(i1 <=> i2 == std::strong_ordering::less);
+ assert(i2 <=> i1 == std::strong_ordering::greater);
+
+ assert(ci1 <=> ci1 == std::strong_ordering::equivalent);
+ assert(ci1 <=> ci2 == std::strong_ordering::less);
+ assert(ci2 <=> ci1 == std::strong_ordering::greater);
+
+ assert(ri1 <=> ri1 == std::strong_ordering::equivalent);
+ assert(ri1 <=> ri2 == std::strong_ordering::less);
+ assert(ri2 <=> ri1 == std::strong_ordering::greater);
+
+ assert(cri1 <=> cri1 == std::strong_ordering::equivalent);
+ assert(cri1 <=> cri2 == std::strong_ordering::less);
+ assert(cri2 <=> cri1 == std::strong_ordering::greater);
}
int main(int, char**) {
diff --git a/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.verify.cpp b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.verify.cpp
new file mode 100644
index 0000000000000..34a2a32dadf74
--- /dev/null
+++ b/libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.verify.cpp
@@ -0,0 +1,77 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: std-at-least-c++23
+
+// <flat_map>
+
+// For underlying iterators i and j, i <=> j must be well-formed and return std::strong_ordering.
+
+#include <iterator>
+#include <type_traits>
+
+#include "MinSequenceContainer.h"
+#include "test_iterators.h"
+
+template <class It>
+class bad_3way_random_access_iterator {
+ template <class U>
+ friend class bad_3way_random_access_iterator;
+
+public:
+ typedef std::random_access_iterator_tag iterator_category;
+ typedef typename std::iterator_traits<It>::value_type value_type;
+ typedef typename std::iterator_traits<It>::difference_type difference_type;
+ typedef It pointer;
+ typedef typename std::iterator_traits<It>::reference reference;
+
+ bad_3way_random_access_iterator();
+ explicit bad_3way_random_access_iterator(It);
+
+ template <class U>
+ bad_3way_random_access_iterator(const bad_3way_random_access_iterator<U>& u);
+
+ template <class U, class = typename std::enable_if<std::is_default_constructible<U>::value>::type>
+ TEST_CONSTEXPR_CXX14 bad_3way_random_access_iterator(bad_3way_random_access_iterator<U>&& u);
+
+ reference operator*() const;
+ reference operator[](difference_type) const;
+
+ bad_3way_random_access_iterator& operator++();
+ bad_3way_random_access_iterator& operator--();
+ bad_3way_random_access_iterator operator++(int);
+ bad_3way_random_access_iterator operator--(int);
+
+ bad_3way_random_access_iterator& operator+=(difference_type);
+ bad_3way_random_access_iterator& operator-=(difference_type);
+ friend bad_3way_random_access_iterator operator+(bad_3way_random_access_iterator, difference_type);
+ friend bad_3way_random_access_iterator operator+(difference_type, bad_3way_random_access_iterator);
+ friend bad_3way_random_access_iterator operator-(bad_3way_random_access_iterator, difference_type);
+ friend difference_type operator-(bad_3way_random_access_iterator, bad_3way_random_access_iterator);
+
+ friend bool operator==(const bad_3way_random_access_iterator&, const bad_3way_random_access_iterator&);
+ friend std::weak_ordering operator<=>(const bad_3way_random_access_iterator&, const bad_3way_random_access_iterator&);
+};
+
+void test() {
+ {
+ using KeyCont = MinSequenceContainer<int, random_access_iterator<int*>, random_access_iterator<const int*>>;
+ using FMap = std::flat_map<KeyCont, MinSequenceContainer<int>>;
+ FMap m;
+ // expected-error@*:* {{static assertion failed: random accesss iterator not supporting three-way comparison is invalid for container}}
+ (void)(m.begin() <=> m.begin());
+ }
+ {
+ using KeyCont =
+ MinSequenceContainer<int, bad_3way_random_access_iterator<int*>, bad_3way_random_access_iterator<const int*>>;
+ using FMap = std::flat_map<KeyCont, MinSequenceContainer<int>>;
+ FMap m;
+ // expected-error@*:* {{static assertion failed: three-way comparison between random accesss container iterators must return std::strong_ordering}}
+ (void)(m.begin() <=> m.begin());
+ }
+}
diff --git a/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.pass.cpp b/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.pass.cpp
index f1b2cad743e23..bde8fc54dd5e8 100644
--- a/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.pass.cpp
+++ b/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.pass.cpp
@@ -115,34 +115,32 @@ void test() {
assert(cri2 >= cri2);
assert(!(cri1 >= cri2));
- if constexpr (std::three_way_comparable<KI>) {
- static_assert(std::three_way_comparable<I>); // ...of course the wrapped iterators still support <=>.
- static_assert(std::three_way_comparable<CI>);
- static_assert(std::three_way_comparable<RI>);
- static_assert(std::three_way_comparable<CRI>);
- static_assert(std::same_as<decltype(I() <=> I()), std::strong_ordering>);
- static_assert(std::same_as<decltype(I() <=> CI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(CI() <=> CI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(RI() <=> RI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(RI() <=> CRI()), std::strong_ordering>);
- static_assert(std::same_as<decltype(CRI() <=> CRI()), std::strong_ordering>);
-
- assert(i1 <=> i1 == std::strong_ordering::equivalent);
- assert(i1 <=> i2 == std::strong_ordering::less);
- assert(i2 <=> i1 == std::strong_ordering::greater);
-
- assert(ci1 <=> ci1 == std::strong_ordering::equivalent);
- assert(ci1 <=> ci2 == std::strong_ordering::less);
- assert(ci2 <=> ci1 == std::strong_ordering::greater);
-
- assert(ri1 <=> ri1 == std::strong_ordering::equivalent);
- assert(ri1 <=> ri2 == std::strong_ordering::less);
- assert(ri2 <=> ri1 == std::strong_ordering::greater);
-
- assert(cri1 <=> cri1 == std::strong_ordering::equivalent);
- assert(cri1 <=> cri2 == std::strong_ordering::less);
- assert(cri2 <=> cri1 == std::strong_ordering::greater);
- }
+ static_assert(std::three_way_comparable<I>); // ...of course the wrapped iterators still support <=>.
+ static_assert(std::three_way_comparable<CI>);
+ static_assert(std::three_way_comparable<RI>);
+ static_assert(std::three_way_comparable<CRI>);
+ static_assert(std::same_as<decltype(I() <=> I()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(I() <=> CI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(CI() <=> CI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(RI() <=> RI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(RI() <=> CRI()), std::strong_ordering>);
+ static_assert(std::same_as<decltype(CRI() <=> CRI()), std::strong_ordering>);
+
+ assert(i1 <=> i1 == std::strong_ordering::equivalent);
+ assert(i1 <=> i2 == std::strong_ordering::less);
+ assert(i2 <=> i1 == std::strong_ordering::greater);
+
+ assert(ci1 <=> ci1 == std::strong_ordering::equivalent);
+ assert(ci1 <=> ci2 == std::strong_ordering::less);
+ assert(ci2 <=> ci1 == std::strong_ordering::greater);
+
+ assert(ri1 <=> ri1 == std::strong_ordering::equivalent);
+ assert(ri1 <=> ri2 == std::strong_ordering::less);
+ assert(ri2 <=> ri1 == std::strong_ordering::greater);
+
+ assert(cri1 <=> cri1 == std::strong_ordering::equivalent);
+ assert(cri1 <=> cri2 == std::strong_ordering::less);
+ assert(cri2 <=> cri1 == std::strong_ordering::greater);
}
int main(int, char**) {
diff --git a/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.verify.cpp b/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.verify.cpp
new file mode 100644
index 0000000000000..9e0bf0a5ff2ec
--- /dev/null
+++ b/libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.verify.cpp
@@ -0,0 +1,77 @@
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: std-at-least-c++23
+
+// <flat_map>
+
+// For underlying iterators i and j, i <=> j must be well-formed and return std::strong_ordering.
+
+#include <iterator>
+#include <type_traits>
+
+#include "MinSequenceContainer.h"
+#include "test_iterators.h"
+
+template <class It>
+class bad_3way_random_access_iterator {
+ template <class U>
+ friend class bad_3way_random_access_iterator;
+
+public:
+ typedef std::random_access_iterator_tag iterator_category;
+ typedef typename std::iterator_traits<It>::value_type value_type;
+ typedef typename std::iterator_traits<It>::difference_type difference_type;
+ typedef It pointer;
+ typedef typename std::iterator_traits<It>::reference reference;
+
+ bad_3way_random_access_iterator();
+ explicit bad_3way_random_access_iterator(It);
+
+ template <class U>
+ bad_3way_random_access_iterator(const bad_3way_random_access_iterator<U>& u);
+
+ template <class U, class = typename std::enable_if<std::is_default_constructible<U>::value>::type>
+ TEST_CONSTEXPR_CXX14 bad_3way_random_access_iterator(bad_3way_random_access_iterator<U>&& u);
+
+ reference operator*() const;
+ reference operator[](difference_type) const;
+
+ bad_3way_random_access_iterator& operator++();
+ bad_3way_random_access_iterator& operator--();
+ bad_3way_random_access_iterator operator++(int);
+ bad_3way_random_access_iterator operator--(int);
+
+ bad_3way_random_access_iterator& operator+=(difference_type);
+ bad_3way_random_access_iterator& operator-=(difference_type);
+ friend bad_3way_random_access_iterator operator+(bad_3way_random_access_iterator, difference_type);
+ friend bad_3way_random_access_iterator operator+(difference_type, bad_3way_random_access_iterator);
+ friend bad_3way_random_access_iterator operator-(bad_3way_random_access_iterator, difference_type);
+ friend difference_type operator-(bad_3way_random_access_iterator, bad_3way_random_access_iterator);
+
+ friend bool operator==(const bad_3way_random_access_iterator&, const bad_3way_random_access_iterator&);
+ friend std::weak_ordering operator<=>(const bad_3way_random_access_iterator&, const bad_3way_random_access_iterator&);
+};
+
+void test() {
+ {
+ using KeyCont = MinSequenceContainer<int, random_access_iterator<int*>, random_access_iterator<const int*>>;
+ using FMap = std::flat_multimap<KeyCont, MinSequenceContainer<int>>;
+ FMap m;
+ // expected-error@*:* {{static assertion failed: random accesss iterator not supporting three-way comparison is invalid for container}}
+ (void)(m.begin() <=> m.begin());
+ }
+ {
+ using KeyCont =
+ MinSequenceContainer<int, bad_3way_random_access_iterator<int*>, bad_3way_random_access_iterator<const int*>>;
+ using FMap = std::flat_multimap<KeyCont, MinSequenceContainer<int>>;
+ FMap m;
+ // expected-error@*:* {{static assertion failed: three-way comparison between random accesss container iterators must return std::strong_ordering}}
+ (void)(m.begin() <=> m.begin());
+ }
+}
diff --git a/libcxx/test/support/MinSequenceContainer.h b/libcxx/test/support/MinSequenceContainer.h
index 7fee4dd0fbdc1..6e61aff06344b 100644
--- a/libcxx/test/support/MinSequenceContainer.h
+++ b/libcxx/test/support/MinSequenceContainer.h
@@ -14,7 +14,9 @@
#include "test_iterators.h"
-template <class T, class Iterator = random_access_iterator<T*>, class ConstIterator = random_access_iterator<const T*>>
+template <class T,
+ class Iterator = three_way_random_access_iterator<T*>,
+ class ConstIterator = three_way_random_access_iterator<const T*>>
struct MinSequenceContainer {
using value_type = T;
using difference_type = int;
diff --git a/libcxx/test/support/test_iterators.h b/libcxx/test/support/test_iterators.h
index ead8a3e8f87d2..1c5ccee110578 100644
--- a/libcxx/test/support/test_iterators.h
+++ b/libcxx/test/support/test_iterators.h
@@ -267,6 +267,135 @@ template <class It>
random_access_iterator(It) -> random_access_iterator<It>;
#endif
+// Since C++20, a container iterator type that is random access is also required to support three-way comparison.
+// See C++20 [tab:container.req], C++23 [container.reqmts]/39 - /41.
+template <class It>
+class three_way_random_access_iterator {
+ It it_;
+ support::double_move_tracker tracker_;
+
+ template <class U>
+ friend class three_way_random_access_iterator;
+
+public:
+ typedef std::random_access_iterator_tag iterator_category;
+ typedef typename std::iterator_traits<It>::value_type value_type;
+ typedef typename std::iterator_traits<It>::difference_type difference_type;
+ typedef It pointer;
+ typedef typename std::iterator_traits<It>::reference reference;
+
+ TEST_CONSTEXPR three_way_random_access_iterator() : it_() {}
+ TEST_CONSTEXPR explicit three_way_random_access_iterator(It it) : it_(it) {}
+
+ template <class U>
+ TEST_CONSTEXPR three_way_random_access_iterator(const three_way_random_access_iterator<U>& u)
+ : it_(u.it_), tracker_(u.tracker_) {}
+
+ template <class U, class = typename std::enable_if<std::is_default_constructible<U>::value>::type>
+ TEST_CONSTEXPR_CXX14 three_way_random_access_iterator(three_way_random_access_iterator<U>&& u)
+ : it_(std::move(u.it_)), tracker_(std::move(u.tracker_)) {
+ u.it_ = U();
+ }
+
+ TEST_CONSTEXPR_CXX14 reference operator*() const { return *it_; }
+ TEST_CONSTEXPR_CXX14 reference operator[](difference_type n) const { return it_[n]; }
+
+ TEST_CONSTEXPR_CXX14 three_way_random_access_iterator& operator++() {
+ ++it_;
+ return *this;
+ }
+ TEST_CONSTEXPR_CXX14 three_way_random_access_iterator& operator--() {
+ --it_;
+ return *this;
+ }
+ TEST_CONSTEXPR_CXX14 three_way_random_access_iterator operator++(int) {
+ return three_way_random_access_iterator(it_++);
+ }
+ TEST_CONSTEXPR_CXX14 three_way_random_access_iterator operator--(int) {
+ return three_way_random_access_iterator(it_--);
+ }
+
+ TEST_CONSTEXPR_CXX14 random_access_ithree_way_random_access_iteratorterator& operator+=(difference_type n) {
+ it_ += n;
+ return *this;
+ }
+ TEST_CONSTEXPR_CXX14 three_way_random_access_iterator& operator-=(difference_type n) {
+ it_ -= n;
+ return *this;
+ }
+ friend TEST_CONSTEXPR_CXX14 three_way_random_access_iterator
+ operator+(three_way_random_access_iterator x, difference_type n) {
+ x += n;
+ return x;
+ }
+ friend TEST_CONSTEXPR_CXX14 three_way_random_access_iterator
+ operator+(difference_type n, three_way_random_access_iterator x) {
+ x += n;
+ return x;
+ }
+ friend TEST_CONSTEXPR_CXX14 three_way_random_access_iterator
+ operator-(three_way_random_access_iterator x, difference_type n) {
+ x -= n;
+ return x;
+ }
+ friend TEST_CONSTEXPR difference_type
+ operator-(three_way_random_access_iterator x, three_way_random_access_iterator y) {
+ return x.it_ - y.it_;
+ }
+
+ friend TEST_CONSTEXPR bool
+ operator==(const three_way_random_access_iterator& x, const three_way_random_access_iterator& y) {
+ return x.it_ == y.it_;
+ }
+#if TEST_STD_VER < 20
+ friend TEST_CONSTEXPR bool
+ operator!=(const three_way_random_access_iterator& x, const th...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/131290
More information about the libcxx-commits
mailing list