[libc++][test] Test `flat_meow` with proper underlying iterators #131290

frederick-vs-ja · 2025-03-14T07:32:22Z

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 should at least avoid testing "containers" with random access but not three-way comparable iterators for flat container adaptors.

This patch adds a new class template three_way_random_access_iterator to test_iterators.h and fixes some usages of MinSequenceContainer with the new iterators.

llvmbot · 2025-03-14T07:32:56Z

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

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 --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]

github-actions · 2025-03-14T17:38:19Z

✅ With the latest revision this PR passed the C/C++ code formatter.

libcxx/include/__flat_map/key_value_iterator.h

frederick-vs-ja · 2025-03-29T08:18:07Z

@mordante @huixie90 Now I've made the changes test-only.

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 should at least avoid testing "containers" with random access but not three-way comparable iterators for flat container adaptors. This patch adds a new class template `three_way_random_access_iterator` to `test_iterators.h` and fixes some usages of `MinSequenceContainer` with the new iterators.

mordante · 2025-04-09T16:46:16Z

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.


I wonder, would it make sense to guard this with #if TEST_STD_VER >= 20? Is there a use-case for this iterator given that we already have a random_access_iterator?

The (possibly only) use case is making MinSequenceContainer<T> a proper sequence container since C++20. An alternative approach IMO would be adding operator<=> to random_access_iterator, but I don't know whether this is desired.

huixie90

Thanks. LGTM with Mark's comments resolved

frederick-vs-ja requested a review from a team as a code owner March 14, 2025 07:32

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Mar 14, 2025

frederick-vs-ja changed the title ~~[libc++] Make flat_(multi)map's iterators require operator<=>~~ [libc++] Make flat_(multi)map's iterators require operator<=> Mar 14, 2025

frederick-vs-ja force-pushed the flat-always-3way branch 9 times, most recently from e650ba5 to a967f96 Compare March 14, 2025 17:34

frederick-vs-ja force-pushed the flat-always-3way branch from a967f96 to e37f2a6 Compare March 15, 2025 13:58

mordante reviewed Mar 17, 2025

View reviewed changes

libcxx/include/__flat_map/key_value_iterator.h Outdated Show resolved Hide resolved

frederick-vs-ja force-pushed the flat-always-3way branch from e37f2a6 to 73c8388 Compare March 29, 2025 08:16

frederick-vs-ja changed the title ~~[libc++] Make flat_(multi)map's iterators require operator<=>~~ [libc++][test] Test flat_meow with proper underlying iterators Mar 29, 2025

frederick-vs-ja added the test-suite label Mar 29, 2025

frederick-vs-ja force-pushed the flat-always-3way branch from 73c8388 to 7d17ded Compare April 7, 2025 07:27

mordante reviewed Apr 9, 2025

View reviewed changes

huixie90 approved these changes Apr 12, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[libc++][test] Test `flat_meow` with proper underlying iterators #131290

[libc++][test] Test `flat_meow` with proper underlying iterators #131290

frederick-vs-ja commented Mar 14, 2025 •

edited

Loading

llvmbot commented Mar 14, 2025

github-actions bot commented Mar 14, 2025 •

edited

Loading

frederick-vs-ja commented Mar 29, 2025

mordante Apr 9, 2025

frederick-vs-ja Apr 19, 2025

huixie90 left a comment

[libc++][test] Test flat_meow with proper underlying iterators #131290

Are you sure you want to change the base?

[libc++][test] Test flat_meow with proper underlying iterators #131290

Conversation

frederick-vs-ja commented Mar 14, 2025 • edited Loading

llvmbot commented Mar 14, 2025

github-actions bot commented Mar 14, 2025 • edited Loading

frederick-vs-ja commented Mar 29, 2025

mordante Apr 9, 2025

Choose a reason for hiding this comment

frederick-vs-ja Apr 19, 2025

Choose a reason for hiding this comment

huixie90 left a comment

Choose a reason for hiding this comment

[libc++][test] Test `flat_meow` with proper underlying iterators #131290

[libc++][test] Test `flat_meow` with proper underlying iterators #131290

frederick-vs-ja commented Mar 14, 2025 •

edited

Loading

github-actions bot commented Mar 14, 2025 •

edited

Loading