[libc++] Make flat_(multi)map's iterators require operator<=>

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.

Author: frederick-vs-ja

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_;
   }
 

libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.pass.cpp

@@ -28,7 +28,6 @@ void test() {
   using Key   = typename KeyContainer::value_type;
   using Value = typename ValueContainer::value_type;
   using M     = std::flat_map<Key, Value, std::less<Key>, KeyContainer, ValueContainer>;
-  using KI    = typename KeyContainer::iterator;
   using I     = M::iterator;
   using CI    = M::const_iterator;
   using RI    = M::reverse_iterator;
@@ -115,34 +114,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**) {

libcxx/test/std/containers/container.adaptors/flat.map/flat.map.iterators/iterator_comparison.verify.cpp

@@ -0,0 +1,81 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 <flat_map>
+#include <functional>
+#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:
+  using iterator_category = std::random_access_iterator_tag;
+  using value_type        = typename std::iterator_traits<It>::value_type;
+  using difference_type   = typename std::iterator_traits<It>::difference_type;
+  using pointer           = It;
+  using reference         = typename std::iterator_traits<It>::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>&);
+
+  template <class U, class = typename std::enable_if<std::is_default_constructible<U>::value>::type>
+  bad_3way_random_access_iterator(bad_3way_random_access_iterator<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<int, int, std::less<int>, KeyCont, MinSequenceContainer<int>>;
+    FMap m;
+    // expected-error@*:* {{static assertion failed: random accesss iterator not supporting three-way comparison is invalid for container}}
+    // expected-error@*:* {{invalid operands to binary expression}}
+    (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<int, int, std::less<int>, KeyCont, MinSequenceContainer<int>>;
+    FMap m;
+    // expected-error-re@*:* {{static assertion failed due to requirement 'is_same_v<std::weak_ordering, std::strong_ordering>'{{.*}}three-way comparison between random accesss container iterators must return std::strong_ordering}}
+    // expected-error-re@*:* {{no viable conversion from returned value of type{{.*}}}}
+    (void)(m.begin() <=> m.begin());
+  }
+}

libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.pass.cpp

@@ -28,7 +28,6 @@ void test() {
   using Key   = typename KeyContainer::value_type;
   using Value = typename ValueContainer::value_type;
   using M     = std::flat_multimap<Key, Value, std::less<Key>, KeyContainer, ValueContainer>;
-  using KI    = typename KeyContainer::iterator;
   using I     = M::iterator;
   using CI    = M::const_iterator;
   using RI    = M::reverse_iterator;
@@ -115,34 +114,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**) {

libcxx/test/std/containers/container.adaptors/flat.multimap/flat.multimap.iterators/iterator_comparison.verify.cpp

@@ -0,0 +1,81 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 <flat_map>
+#include <functional>
+#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:
+using iterator_category = std::random_access_iterator_tag;
+using value_type        = typename std::iterator_traits<It>::value_type;
+using difference_type   = typename std::iterator_traits<It>::difference_type;
+using pointer           = It;
+using reference         = typename std::iterator_traits<It>::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>&);
+
+  template <class U, class = typename std::enable_if<std::is_default_constructible<U>::value>::type>
+  bad_3way_random_access_iterator(bad_3way_random_access_iterator<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<int, int, std::less<int>, KeyCont, MinSequenceContainer<int>>;
+    FMap m;
+    // expected-error@*:* {{static assertion failed: random accesss iterator not supporting three-way comparison is invalid for container}}
+    // expected-error@*:* {{invalid operands to binary expression}}
+    (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<int, int, std::less<int>, KeyCont, MinSequenceContainer<int>>;
+    FMap m;
+    // expected-error-re@*:* {{static assertion failed due to requirement 'is_same_v<std::weak_ordering, std::strong_ordering>'{{.*}}three-way comparison between random accesss container iterators must return std::strong_ordering}}
+    // expected-error-re@*:* {{no viable conversion from returned value of type{{.*}}}}
+    (void)(m.begin() <=> m.begin());
+  }
+}

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;