[libcxx-commits] [libcxx] eadf726 - [libc++] Fix bugs in common_iterator; add test coverage.

Arthur O'Dwyer via libcxx-commits libcxx-commits at lists.llvm.org
Mon Jan 24 10:46:18 PST 2022


Author: Arthur O'Dwyer
Date: 2022-01-24T13:45:38-05:00
New Revision: eadf7268d578396d4f2fc2a0f7eda8096c041007

URL: https://github.com/llvm/llvm-project/commit/eadf7268d578396d4f2fc2a0f7eda8096c041007
DIFF: https://github.com/llvm/llvm-project/commit/eadf7268d578396d4f2fc2a0f7eda8096c041007.diff

LOG: [libc++] Fix bugs in common_iterator; add test coverage.

Differential Revision: https://reviews.llvm.org/D117400

Added: 
    libcxx/test/std/iterators/predef.iterators/iterators.common/constraints.compile.pass.cpp
    libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.converting.pass.cpp
    libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.default.pass.cpp
    libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.iter.pass.cpp
    libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.sentinel.pass.cpp

Modified: 
    libcxx/docs/Status/Cxx2bIssues.csv
    libcxx/include/__iterator/common_iterator.h
    libcxx/test/std/iterators/predef.iterators/iterators.common/iter_move.pass.cpp
    libcxx/test/std/iterators/predef.iterators/iterators.common/iter_swap.pass.cpp
    libcxx/test/std/iterators/predef.iterators/iterators.common/types.h
    libcxx/test/support/test_iterators.h

Removed: 
    libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.pass.cpp


################################################################################
diff  --git a/libcxx/docs/Status/Cxx2bIssues.csv b/libcxx/docs/Status/Cxx2bIssues.csv
index cbc12d27a5614..9baefabe05da8 100644
--- a/libcxx/docs/Status/Cxx2bIssues.csv
+++ b/libcxx/docs/Status/Cxx2bIssues.csv
@@ -126,7 +126,7 @@
 `3571 <https://wg21.link/LWG3571>`__,"``flush_emit`` should set ``badbit`` if the ``emit`` call fails","October 2021","",""
 `3572 <https://wg21.link/LWG3572>`__,"``copyable-box`` should be fully ``constexpr``","October 2021","","","|ranges|"
 `3573 <https://wg21.link/LWG3573>`__,"Missing Throws element for ``basic_string_view(It begin, End end)``","October 2021","|Complete|","14.0"
-`3574 <https://wg21.link/LWG3574>`__,"``common_iterator`` should be completely ``constexpr``-able","October 2021","","","|ranges|"
+`3574 <https://wg21.link/LWG3574>`__,"``common_iterator`` should be completely ``constexpr``-able","October 2021","|Complete|","14.0","|ranges|"
 `3580 <https://wg21.link/LWG3580>`__,"``iota_view``'s ``iterator``'s binary ``operator+`` should be improved","October 2021","","","|ranges|"
 `3581 <https://wg21.link/LWG3581>`__,"The range constructor makes ``basic_string_view`` not trivially move constructible","October 2021","","","|ranges|"
 `3585 <https://wg21.link/LWG3585>`__,"``variant`` converting assignment with immovable alternative","October 2021","",""
@@ -135,7 +135,7 @@
 `3591 <https://wg21.link/LWG3591>`__,"``lazy_split_view<input_view>::inner-iterator::base() &&`` invalidates outer iterators","October 2021","","","|ranges|"
 `3592 <https://wg21.link/LWG3592>`__,"``lazy_split_view`` needs to check the simpleness of Pattern","October 2021","","","|ranges|"
 `3593 <https://wg21.link/LWG3593>`__,"Several iterators' ``base() const &`` and ``lazy_split_view::outer-iterator::value_type::end()`` missing ``noexcept``","October 2021","","","|ranges|"
-`3595 <https://wg21.link/LWG3595>`__,"Exposition-only classes proxy and postfix-proxy for ``common_iterator`` should be fully ``constexpr``","October 2021","","","|ranges|"
+`3595 <https://wg21.link/LWG3595>`__,"Exposition-only classes proxy and postfix-proxy for ``common_iterator`` should be fully ``constexpr``","October 2021","|Complete|","14.0","|ranges|"
 "","","","",""
 `3645 <https://wg21.link/LWG3645>`__,"``resize_and_overwrite`` is overspecified to call its callback with lvalues", "Not voted in","|Complete|","14.0",""
 "","","","",""

diff  --git a/libcxx/include/__iterator/common_iterator.h b/libcxx/include/__iterator/common_iterator.h
index 9a142769e55a7..df4c7bd18e8d2 100644
--- a/libcxx/include/__iterator/common_iterator.h
+++ b/libcxx/include/__iterator/common_iterator.h
@@ -41,7 +41,7 @@ class common_iterator {
       : __value(_VSTD::move(__x)) {}
 
   public:
-    const iter_value_t<_Iter>* operator->() const {
+    constexpr const iter_value_t<_Iter>* operator->() const noexcept {
       return _VSTD::addressof(__value);
     }
   };
@@ -58,7 +58,7 @@ class common_iterator {
       constructible_from<iter_value_t<_Iter>, iter_reference_t<_Iter>> &&
       move_constructible<iter_value_t<_Iter>>;
 
-    const iter_value_t<_Iter>& operator*() const {
+    constexpr const iter_value_t<_Iter>& operator*() const noexcept {
       return __value;
     }
   };
@@ -75,7 +75,7 @@ class common_iterator {
     requires convertible_to<const _I2&, _Iter> && convertible_to<const _S2&, _Sent>
   constexpr common_iterator(const common_iterator<_I2, _S2>& __other)
     : __hold_([&]() -> variant<_Iter, _Sent> {
-      _LIBCPP_ASSERT(!__other.__hold_.valueless_by_exception(), "Constructed from valueless iterator.");
+      _LIBCPP_ASSERT(!__other.__hold_.valueless_by_exception(), "Attempted to construct from a valueless common_iterator");
       if (__other.__hold_.index() == 0)
         return variant<_Iter, _Sent>{in_place_index<0>, _VSTD::__unchecked_get<0>(__other.__hold_)};
       return variant<_Iter, _Sent>{in_place_index<1>, _VSTD::__unchecked_get<1>(__other.__hold_)};
@@ -85,7 +85,7 @@ class common_iterator {
     requires convertible_to<const _I2&, _Iter> && convertible_to<const _S2&, _Sent> &&
              assignable_from<_Iter&, const _I2&> && assignable_from<_Sent&, const _S2&>
   common_iterator& operator=(const common_iterator<_I2, _S2>& __other) {
-    _LIBCPP_ASSERT(!__other.__hold_.valueless_by_exception(), "Assigned from valueless iterator.");
+    _LIBCPP_ASSERT(!__other.__hold_.valueless_by_exception(), "Attempted to assign from a valueless common_iterator");
 
     auto __idx = __hold_.index();
     auto __other_idx = __other.__hold_.index();
@@ -105,18 +105,16 @@ class common_iterator {
     return *this;
   }
 
-  decltype(auto) operator*()
+  constexpr decltype(auto) operator*()
   {
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_),
-                   "Cannot dereference sentinel. Common iterator not holding an iterator.");
+    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to dereference a non-dereferenceable common_iterator");
     return *_VSTD::__unchecked_get<_Iter>(__hold_);
   }
 
-  decltype(auto) operator*() const
+  constexpr decltype(auto) operator*() const
     requires __dereferenceable<const _Iter>
   {
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_),
-                   "Cannot dereference sentinel. Common iterator not holding an iterator.");
+    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to dereference a non-dereferenceable common_iterator");
     return *_VSTD::__unchecked_get<_Iter>(__hold_);
   }
 
@@ -127,9 +125,7 @@ class common_iterator {
      is_reference_v<iter_reference_t<_I2>> ||
      constructible_from<iter_value_t<_I2>, iter_reference_t<_I2>>)
   {
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_),
-                   "Cannot dereference sentinel. Common iterator not holding an iterator.");
-
+    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to dereference a non-dereferenceable common_iterator");
     if constexpr (is_pointer_v<_Iter> || requires(const _Iter& __i) { __i.operator->(); })    {
       return _VSTD::__unchecked_get<_Iter>(__hold_);
     } else if constexpr (is_reference_v<iter_reference_t<_Iter>>) {
@@ -141,15 +137,12 @@ class common_iterator {
   }
 
   common_iterator& operator++() {
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_),
-                   "Cannot increment sentinel. Common iterator not holding an iterator.");
+    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to increment a non-dereferenceable common_iterator");
     ++_VSTD::__unchecked_get<_Iter>(__hold_); return *this;
   }
 
   decltype(auto) operator++(int) {
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_),
-                   "Cannot increment sentinel. Common iterator not holding an iterator.");
-
+    _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to increment a non-dereferenceable common_iterator");
     if constexpr (forward_iterator<_Iter>) {
       auto __tmp = *this;
       ++*this;
@@ -166,10 +159,9 @@ class common_iterator {
 
   template<class _I2, sentinel_for<_Iter> _S2>
     requires sentinel_for<_Sent, _I2>
-  friend bool operator==(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
-    _LIBCPP_ASSERT(!__x.__hold_.valueless_by_exception() &&
-                   !__y.__hold_.valueless_by_exception(),
-                   "One or both common_iterators are valueless. (Cannot compare valueless iterators.)");
+  friend constexpr bool operator==(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
+    _LIBCPP_ASSERT(!__x.__hold_.valueless_by_exception(), "Attempted to compare a valueless common_iterator");
+    _LIBCPP_ASSERT(!__y.__hold_.valueless_by_exception(), "Attempted to compare a valueless common_iterator");
 
     auto __x_index = __x.__hold_.index();
     auto __y_index = __y.__hold_.index();
@@ -185,10 +177,9 @@ class common_iterator {
 
   template<class _I2, sentinel_for<_Iter> _S2>
     requires sentinel_for<_Sent, _I2> && equality_comparable_with<_Iter, _I2>
-  friend bool operator==(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
-    _LIBCPP_ASSERT(!__x.__hold_.valueless_by_exception() &&
-                   !__y.__hold_.valueless_by_exception(),
-                   "One or both common_iterators are valueless. (Cannot compare valueless iterators.)");
+  friend constexpr bool operator==(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
+    _LIBCPP_ASSERT(!__x.__hold_.valueless_by_exception(), "Attempted to compare a valueless common_iterator");
+    _LIBCPP_ASSERT(!__y.__hold_.valueless_by_exception(), "Attempted to compare a valueless common_iterator");
 
     auto __x_index = __x.__hold_.index();
     auto __y_index = __y.__hold_.index();
@@ -207,10 +198,9 @@ class common_iterator {
 
   template<sized_sentinel_for<_Iter> _I2, sized_sentinel_for<_Iter> _S2>
     requires sized_sentinel_for<_Sent, _I2>
-  friend iter_
diff erence_t<_I2> operator-(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
-    _LIBCPP_ASSERT(!__x.__hold_.valueless_by_exception() &&
-                   !__y.__hold_.valueless_by_exception(),
-                   "One or both common_iterators are valueless. (Cannot subtract valueless iterators.)");
+  friend constexpr iter_
diff erence_t<_I2> operator-(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {
+    _LIBCPP_ASSERT(!__x.__hold_.valueless_by_exception(), "Attempted to subtract from a valueless common_iterator");
+    _LIBCPP_ASSERT(!__y.__hold_.valueless_by_exception(), "Attempted to subtract a valueless common_iterator");
 
     auto __x_index = __x.__hold_.index();
     auto __y_index = __y.__hold_.index();
@@ -227,24 +217,21 @@ class common_iterator {
     return _VSTD::__unchecked_get<_Sent>(__x.__hold_) - _VSTD::__unchecked_get<_I2>(__y.__hold_);
   }
 
-  friend iter_rvalue_reference_t<_Iter> iter_move(const common_iterator& __i)
+  friend constexpr iter_rvalue_reference_t<_Iter> iter_move(const common_iterator& __i)
     noexcept(noexcept(ranges::iter_move(declval<const _Iter&>())))
       requires input_iterator<_Iter>
   {
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__i.__hold_),
-                   "Cannot iter_move a sentinel. Common iterator not holding an iterator.");
+    _LIBCPP_ASSERT(holds_alternative<_Iter>(__i.__hold_), "Attempted to iter_move a non-dereferenceable common_iterator");
     return ranges::iter_move( _VSTD::__unchecked_get<_Iter>(__i.__hold_));
   }
 
   template<indirectly_swappable<_Iter> _I2, class _S2>
-  friend void iter_swap(const common_iterator& __x, const common_iterator<_I2, _S2>& __y)
+  friend constexpr void iter_swap(const common_iterator& __x, const common_iterator<_I2, _S2>& __y)
       noexcept(noexcept(ranges::iter_swap(declval<const _Iter&>(), declval<const _I2&>())))
   {
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__x.__hold_),
-                   "Cannot swap __y with a sentinel. Common iterator (__x) not holding an iterator.");
-    _LIBCPP_ASSERT(holds_alternative<_Iter>(__y.__hold_),
-                   "Cannot swap __x with a sentinel. Common iterator (__y) not holding an iterator.");
-    return ranges::iter_swap( _VSTD::__unchecked_get<_Iter>(__x.__hold_),  _VSTD::__unchecked_get<_Iter>(__y.__hold_));
+    _LIBCPP_ASSERT(holds_alternative<_Iter>(__x.__hold_), "Attempted to iter_swap a non-dereferenceable common_iterator");
+    _LIBCPP_ASSERT(holds_alternative<_I2>(__y.__hold_), "Attempted to iter_swap a non-dereferenceable common_iterator");
+    return ranges::iter_swap(_VSTD::__unchecked_get<_Iter>(__x.__hold_), _VSTD::__unchecked_get<_I2>(__y.__hold_));
   }
 };
 
@@ -271,7 +258,7 @@ struct __arrow_type_or_void {
 template<class _Iter, class _Sent>
   requires __common_iter_has_ptr_op<_Iter, _Sent>
 struct __arrow_type_or_void<_Iter, _Sent> {
-    using type = decltype(declval<const common_iterator<_Iter, _Sent>>().operator->());
+    using type = decltype(declval<const common_iterator<_Iter, _Sent>&>().operator->());
 };
 
 template<class _Iter, class _Sent>

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/constraints.compile.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/constraints.compile.pass.cpp
new file mode 100644
index 0000000000000..3eb99e1428673
--- /dev/null
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/constraints.compile.pass.cpp
@@ -0,0 +1,28 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+
+// template<input_or_output_iterator I, sentinel_for<I> S>
+//   requires (!same_as<I, S> && copyable<I>)
+
+#include <iterator>
+
+#include "test_iterators.h"
+
+template<class I, class S>
+concept ValidCommonIterator = requires {
+  typename std::common_iterator<I, S>;
+};
+
+static_assert( ValidCommonIterator<int*, const int*>);
+static_assert(!ValidCommonIterator<int, int>); // !input_or_output_iterator<I>
+static_assert(!ValidCommonIterator<int*, float*>); // !sentinel_for<S, I>
+static_assert(!ValidCommonIterator<int*, int*>); // !same_as<I, S>
+static_assert(!ValidCommonIterator<cpp20_input_iterator<int*>, sentinel_wrapper<cpp20_input_iterator<int*>>>); // !copyable<I>

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.converting.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.converting.pass.cpp
new file mode 100644
index 0000000000000..b28120537b5a4
--- /dev/null
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.converting.pass.cpp
@@ -0,0 +1,48 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+
+// template<class I2, class S2>
+//   requires convertible_to<const I2&, I> && convertible_to<const S2&, S>
+//     constexpr common_iterator(const common_iterator<I2, S2>& x);
+
+#include <iterator>
+#include <cassert>
+
+#include "test_macros.h"
+
+constexpr bool test()
+{
+  struct Base {};
+  struct Derived : Base {};
+
+  using BaseIt = std::common_iterator<Base*, const Base*>;
+  using DerivedIt = std::common_iterator<Derived*, const Derived*>;
+  static_assert(std::is_convertible_v<DerivedIt, BaseIt>); // Derived* to Base*
+  static_assert(!std::is_constructible_v<DerivedIt, BaseIt>); // Base* to Derived*
+
+  Derived a[10] = {};
+  DerivedIt it = DerivedIt(a); // the iterator type
+  BaseIt jt = BaseIt(it);
+  assert(jt == BaseIt(a));
+
+  it = DerivedIt((const Derived*)a); // the sentinel type
+  jt = BaseIt(it);
+  assert(jt == BaseIt(a));
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.default.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.default.pass.cpp
new file mode 100644
index 0000000000000..199ceb66893e6
--- /dev/null
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.default.pass.cpp
@@ -0,0 +1,41 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+
+// constexpr common_iterator() requires default_initializable<I> = default;
+
+#include <iterator>
+#include <cassert>
+
+#include "test_iterators.h"
+
+constexpr bool test()
+{
+  {
+    using It = cpp17_input_iterator<int*>;
+    using CommonIt = std::common_iterator<It, sentinel_wrapper<It>>;
+    static_assert(!std::is_default_constructible_v<It>); // premise
+    static_assert(!std::is_default_constructible_v<CommonIt>); // conclusion
+  }
+  {
+    // The base iterator is value-initialized.
+    std::common_iterator<int*, sentinel_wrapper<int*>> c;
+    assert(c == nullptr);
+  }
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.iter.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.iter.pass.cpp
new file mode 100644
index 0000000000000..fd47612eca389
--- /dev/null
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.iter.pass.cpp
@@ -0,0 +1,50 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+
+// constexpr common_iterator(I i);
+
+#include <iterator>
+#include <cassert>
+
+#include "test_iterators.h"
+
+template<class It>
+constexpr bool test() {
+  using CommonIt = std::common_iterator<It, sentinel_wrapper<It>>;
+  int a[] = {1,2,3};
+  It it = It(a);
+  CommonIt lv = CommonIt(it);
+  assert(&*lv == a);
+  CommonIt rv = CommonIt(std::move(it));
+  assert(&*rv == a);
+
+  return true;
+}
+
+int main(int, char**) {
+  test<cpp17_input_iterator<int*>>();
+  test<forward_iterator<int*>>();
+  test<bidirectional_iterator<int*>>();
+  test<random_access_iterator<int*>>();
+  test<contiguous_iterator<int*>>();
+  test<int*>();
+  test<const int*>();
+
+  static_assert(test<cpp17_input_iterator<int*>>());
+  static_assert(test<forward_iterator<int*>>());
+  static_assert(test<bidirectional_iterator<int*>>());
+  static_assert(test<random_access_iterator<int*>>());
+  static_assert(test<contiguous_iterator<int*>>());
+  static_assert(test<int*>());
+  static_assert(test<const int*>());
+
+  return 0;
+}

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.pass.cpp
deleted file mode 100644
index c329e8b8a81a7..0000000000000
--- a/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.pass.cpp
+++ /dev/null
@@ -1,90 +0,0 @@
-//===----------------------------------------------------------------------===//
-//
-// 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: c++03, c++11, c++14, c++17
-// UNSUPPORTED: libcpp-no-concepts
-
-// constexpr common_iterator() requires default_initializable<I> = default;
-// constexpr common_iterator(I i);
-// constexpr common_iterator(S s);
-// template<class I2, class S2>
-//   requires convertible_to<const I2&, I> && convertible_to<const S2&, S>
-//     constexpr common_iterator(const common_iterator<I2, S2>& x);
-
-#include <iterator>
-#include <cassert>
-
-#include "test_macros.h"
-#include "types.h"
-
-template<class I, class S>
-concept ValidCommonIterator = requires {
-  typename std::common_iterator<I, S>;
-};
-
-template<class I, class I2>
-concept ConvCtorEnabled = requires(std::common_iterator<I2, sentinel_type<int*>> ci) {
-  std::common_iterator<I, sentinel_type<int*>>(ci);
-};
-
-void test() {
-  int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
-
-  static_assert( std::is_default_constructible_v<std::common_iterator<int*, sentinel_type<int*>>>);
-  static_assert(!std::is_default_constructible_v<std::common_iterator<non_default_constructible_iterator<int*>, sentinel_type<int*>>>);
-
-  // Not copyable:
-  static_assert(!ValidCommonIterator<cpp20_input_iterator<int*>, sentinel_type<int*>>);
-  // Same iter and sent:
-  static_assert(!ValidCommonIterator<cpp17_input_iterator<int*>, cpp17_input_iterator<int*>>);
-
-  {
-    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});
-
-    assert(*iter1 == 1);
-    assert(*commonIter1 == 1);
-    assert(commonIter1 != commonSent1);
-  }
-  {
-    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});
-
-    assert(*iter1 == 1);
-    assert(*commonIter1 == 1);
-    assert(commonIter1 != commonSent1);
-  }
-  {
-    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});
-
-    assert(*iter1 == 1);
-    assert(*commonIter1 == 1);
-    assert(commonIter1 != commonSent1);
-  }
-
-  // Conversion constructor:
-  {
-    convertible_iterator<int*> conv{buffer};
-    auto commonIter1 = std::common_iterator<convertible_iterator<int*>, sentinel_type<int*>>(conv);
-    auto commonIter2 = std::common_iterator<forward_iterator<int*>, sentinel_type<int*>>(commonIter1);
-    assert(*commonIter2 == 1);
-
-    static_assert( ConvCtorEnabled<forward_iterator<int*>, convertible_iterator<int*>>);
-    static_assert(!ConvCtorEnabled<forward_iterator<int*>, random_access_iterator<int*>>);
-  }
-}
-
-int main(int, char**) {
-  test();
-
-  return 0;
-}

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.sentinel.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.sentinel.pass.cpp
new file mode 100644
index 0000000000000..c6c0f301ead99
--- /dev/null
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/ctor.sentinel.pass.cpp
@@ -0,0 +1,63 @@
+//===----------------------------------------------------------------------===//
+//
+// 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: c++03, c++11, c++14, c++17
+// UNSUPPORTED: libcpp-no-concepts
+
+// constexpr common_iterator(S s);
+
+#include <iterator>
+#include <cassert>
+#include <type_traits>
+
+#include "test_iterators.h"
+
+template<class It>
+constexpr bool test() {
+  using Sent = sentinel_wrapper<It>;
+  using CommonIt = std::common_iterator<It, Sent>;
+  int a[] = {1,2,3};
+  It it = It(a);
+  Sent sent = Sent(It(a+1));
+
+  CommonIt lv = CommonIt(sent);
+  assert(lv == CommonIt(sent));
+  assert(lv != CommonIt(it));
+  if (!std::is_constant_evaluated()) {
+    assert(lv == std::next(CommonIt(it)));
+  }
+
+  CommonIt rv = CommonIt(std::move(sent));
+  assert(rv == CommonIt(sent));
+  assert(rv != CommonIt(it));
+  if (!std::is_constant_evaluated()) {
+    assert(rv == std::next(CommonIt(it)));
+  }
+
+  return true;
+}
+
+int main(int, char**) {
+  test<cpp17_input_iterator<int*>>();
+  test<forward_iterator<int*>>();
+  test<bidirectional_iterator<int*>>();
+  test<random_access_iterator<int*>>();
+  test<contiguous_iterator<int*>>();
+  test<int*>();
+  test<const int*>();
+
+  static_assert(test<cpp17_input_iterator<int*>>());
+  static_assert(test<forward_iterator<int*>>());
+  static_assert(test<bidirectional_iterator<int*>>());
+  static_assert(test<random_access_iterator<int*>>());
+  static_assert(test<contiguous_iterator<int*>>());
+  static_assert(test<int*>());
+  static_assert(test<const int*>());
+
+  return 0;
+}

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_move.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_move.pass.cpp
index 0bb4d455c34d3..4c8ae2dae729d 100644
--- a/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_move.pass.cpp
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_move.pass.cpp
@@ -15,35 +15,79 @@
 
 #include <iterator>
 #include <cassert>
+#include <type_traits>
 
+#include "test_iterators.h"
 #include "test_macros.h"
-#include "types.h"
 
-void test() {
-  int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
+struct IterMovingIt {
+  using value_type = int;
+  using 
diff erence_type = int;
+  explicit IterMovingIt() = default;
+  IterMovingIt(const IterMovingIt&); // copyable, but this test shouldn't make copies
+  IterMovingIt(IterMovingIt&&) = default;
+  IterMovingIt& operator=(const IterMovingIt&);
+  int& operator*() const;
+  constexpr IterMovingIt& operator++() { return *this; }
+  IterMovingIt operator++(int);
+  friend constexpr int iter_move(const IterMovingIt&) {
+    return 42;
+  }
+  bool operator==(std::default_sentinel_t) const;
+};
+static_assert(std::input_iterator<IterMovingIt>);
 
+constexpr bool test() {
   {
-    auto iter1 = cpp17_input_iterator<int*>(buffer);
-    auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    assert(std::ranges::iter_move(commonIter1) == 1);
-    ASSERT_SAME_TYPE(decltype(std::ranges::iter_move(commonIter1)), int&&);
+    using It = int*;
+    using CommonIt = std::common_iterator<It, sentinel_wrapper<It>>;
+    int a[] = {1, 2, 3};
+    CommonIt it = CommonIt(It(a));
+    ASSERT_NOEXCEPT(iter_move(it));
+    ASSERT_NOEXCEPT(std::ranges::iter_move(it));
+    ASSERT_SAME_TYPE(decltype(iter_move(it)), int&&);
+    ASSERT_SAME_TYPE(decltype(std::ranges::iter_move(it)), int&&);
+    assert(iter_move(it) == 1);
+    if (!std::is_constant_evaluated()) {
+      ++it;
+      assert(iter_move(it) == 2);
+    }
   }
   {
-    auto iter1 = forward_iterator<int*>(buffer);
-    auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    assert(std::ranges::iter_move(commonIter1) == 1);
-    ASSERT_SAME_TYPE(decltype(std::ranges::iter_move(commonIter1)), int&&);
+    using It = const int*;
+    using CommonIt = std::common_iterator<It, sentinel_wrapper<It>>;
+    int a[] = {1, 2, 3};
+    CommonIt it = CommonIt(It(a));
+    ASSERT_NOEXCEPT(iter_move(it));
+    ASSERT_NOEXCEPT(std::ranges::iter_move(it));
+    ASSERT_SAME_TYPE(decltype(iter_move(it)), const int&&);
+    ASSERT_SAME_TYPE(decltype(std::ranges::iter_move(it)), const int&&);
+    assert(iter_move(it) == 1);
+    if (!std::is_constant_evaluated()) {
+      ++it;
+      assert(iter_move(it) == 2);
+    }
   }
   {
-    auto iter1 = random_access_iterator<int*>(buffer);
-    auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    assert(std::ranges::iter_move(commonIter1) == 1);
-    ASSERT_SAME_TYPE(decltype(std::ranges::iter_move(commonIter1)), int&&);
+    using It = IterMovingIt;
+    using CommonIt = std::common_iterator<It, std::default_sentinel_t>;
+    CommonIt it = CommonIt(It());
+    ASSERT_NOT_NOEXCEPT(iter_move(it));
+    ASSERT_NOT_NOEXCEPT(std::ranges::iter_move(it));
+    ASSERT_SAME_TYPE(decltype(iter_move(it)), int);
+    ASSERT_SAME_TYPE(decltype(std::ranges::iter_move(it)), int);
+    assert(iter_move(it) == 42);
+    if (!std::is_constant_evaluated()) {
+      ++it;
+      assert(iter_move(it) == 42);
+    }
   }
+  return true;
 }
 
 int main(int, char**) {
   test();
+  static_assert(test());
 
   return 0;
 }

diff  --git a/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_swap.pass.cpp b/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_swap.pass.cpp
index f649d334b9cb6..3b69a6d7bd4b5 100644
--- a/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_swap.pass.cpp
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/iter_swap.pass.cpp
@@ -10,55 +10,114 @@
 // UNSUPPORTED: libcpp-no-concepts
 
 // template<indirectly_swappable<I> I2, class S2>
-//   friend void iter_swap(const common_iterator& x, const common_iterator<I2, S2>& y)
+//   friend constexpr void iter_swap(const common_iterator& x, const common_iterator<I2, S2>& y)
 //     noexcept(noexcept(ranges::iter_swap(declval<const I&>(), declval<const I2&>())));
 
 #include <iterator>
 #include <cassert>
+#include <type_traits>
 
+#include "test_iterators.h"
 #include "test_macros.h"
-#include "types.h"
 
-void test() {
-  int buffer[8] = {1, 2, 3, 4, 5, 6, 7, 8};
+template<int K>
+struct IterSwappingIt {
+  using value_type = int;
+  using 
diff erence_type = int;
+  constexpr explicit IterSwappingIt(int *swaps) : swaps_(swaps) {}
+  IterSwappingIt(const IterSwappingIt&); // copyable, but this test shouldn't make copies
+  IterSwappingIt(IterSwappingIt&&) = default;
+  IterSwappingIt& operator=(const IterSwappingIt&);
+  int& operator*() const;
+  constexpr IterSwappingIt& operator++() { return *this; }
+  IterSwappingIt operator++(int);
 
+  template<int L>
+  friend constexpr int iter_swap(const IterSwappingIt<K>& lhs, const IterSwappingIt<L>& rhs) {
+    *lhs.swaps_ += 10;
+    *rhs.swaps_ += 1;
+    return 42; // should be accepted but ignored
+  }
+
+  bool operator==(std::default_sentinel_t) const;
+
+  int *swaps_ = nullptr;
+};
+static_assert(std::input_iterator<IterSwappingIt<0>>);
+static_assert(std::indirectly_swappable<IterSwappingIt<0>, IterSwappingIt<0>>);
+static_assert(std::indirectly_swappable<IterSwappingIt<0>, IterSwappingIt<1>>);
+
+constexpr bool test() {
   {
-    auto iter1 = cpp17_input_iterator<int*>(buffer);
-    auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    auto commonIter2 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    for (auto i = 0; i < 4; ++i) ++commonIter2;
-    assert(*commonIter2 == 5);
-    std::ranges::iter_swap(commonIter1, commonIter2);
-    assert(*commonIter1 == 5);
-    assert(*commonIter2 == 1);
-    std::ranges::iter_swap(commonIter2, commonIter1);
+    using It = int*;
+    using CommonIt = std::common_iterator<It, sentinel_wrapper<It>>;
+    static_assert(std::indirectly_swappable<CommonIt, CommonIt>);
+
+    int a[] = {1, 2, 3};
+    CommonIt it = CommonIt(It(a));
+    CommonIt jt = CommonIt(It(a+1));
+    ASSERT_NOEXCEPT(iter_swap(it, jt));
+    ASSERT_SAME_TYPE(decltype(iter_swap(it, jt)), void);
+    iter_swap(it, jt);
+    assert(a[0] == 2);
+    assert(a[1] == 1);
   }
   {
-    auto iter1 = forward_iterator<int*>(buffer);
-    auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    auto commonIter2 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    for (auto i = 0; i < 4; ++i) ++commonIter2;
-    assert(*commonIter2 == 5);
-    std::ranges::iter_swap(commonIter1, commonIter2);
-    assert(*commonIter1 == 5);
-    assert(*commonIter2 == 1);
-    std::ranges::iter_swap(commonIter2, commonIter1);
+    using It = const int*;
+    using CommonIt = std::common_iterator<It, sentinel_wrapper<It>>;
+    static_assert(!std::indirectly_swappable<CommonIt, CommonIt>);
   }
   {
-    auto iter1 = random_access_iterator<int*>(buffer);
-    auto commonIter1 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    auto commonIter2 = std::common_iterator<decltype(iter1), sentinel_type<int*>>(iter1);
-    for (auto i = 0; i < 4; ++i) ++commonIter2;
-    assert(*commonIter2 == 5);
-    std::ranges::iter_swap(commonIter1, commonIter2);
-    assert(*commonIter1 == 5);
-    assert(*commonIter2 == 1);
-    std::ranges::iter_swap(commonIter2, commonIter1);
+    using It = IterSwappingIt<0>;
+    using CommonIt = std::common_iterator<It, std::default_sentinel_t>;
+    static_assert(std::indirectly_swappable<CommonIt, CommonIt>);
+
+    int iswaps = 100;
+    int jswaps = 100;
+    CommonIt it = CommonIt(It(&iswaps));
+    CommonIt jt = CommonIt(It(&jswaps));
+    ASSERT_NOT_NOEXCEPT(iter_swap(it, jt));
+    ASSERT_SAME_TYPE(decltype(iter_swap(it, jt)), void);
+    iter_swap(it, jt); // lvalue iterators
+    assert(iswaps == 110);
+    assert(jswaps == 101);
+    iter_swap(CommonIt(It(&iswaps)), CommonIt(It(&jswaps))); // rvalue iterators
+    assert(iswaps == 120);
+    assert(jswaps == 102);
+    std::ranges::iter_swap(it, jt);
+    assert(iswaps == 130);
+    assert(jswaps == 103);
+  }
+  {
+    using It = IterSwappingIt<0>;
+    using Jt = IterSwappingIt<1>;
+    static_assert(std::indirectly_swappable<It, Jt>);
+    using CommonIt = std::common_iterator<It, std::default_sentinel_t>;
+    using CommonJt = std::common_iterator<Jt, std::default_sentinel_t>;
+    static_assert(std::indirectly_swappable<CommonIt, CommonJt>);
+
+    int iswaps = 100;
+    int jswaps = 100;
+    CommonIt it = CommonIt(It(&iswaps));
+    CommonJt jt = CommonJt(Jt(&jswaps));
+    ASSERT_NOT_NOEXCEPT(iter_swap(it, jt));
+    ASSERT_SAME_TYPE(decltype(iter_swap(it, jt)), void);
+    iter_swap(it, jt); // lvalue iterators
+    assert(iswaps == 110);
+    assert(jswaps == 101);
+    iter_swap(CommonIt(It(&iswaps)), CommonJt(Jt(&jswaps))); // rvalue iterators
+    assert(iswaps == 120);
+    assert(jswaps == 102);
+    std::ranges::iter_swap(it, jt);
+    assert(iswaps == 130);
+    assert(jswaps == 103);
   }
+  return true;
 }
 
 int main(int, char**) {
   test();
+  static_assert(test());
 
   return 0;
 }

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 d5068b3cf013f..03b94f63f631e 100644
--- a/libcxx/test/std/iterators/predef.iterators/iterators.common/types.h
+++ b/libcxx/test/std/iterators/predef.iterators/iterators.common/types.h
@@ -157,32 +157,6 @@ class comparable_iterator
     }
 };
 
-template <class It>
-class convertible_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_;}
-
-    convertible_iterator() = default;
-    explicit constexpr convertible_iterator(It it) : it_(it) {}
-
-    constexpr reference operator*() const {return *it_;}
-
-    constexpr convertible_iterator& operator++() {++it_; return *this;}
-    constexpr convertible_iterator operator++(int)
-        {convertible_iterator tmp(*this); ++(*this); return tmp;}
-
-    operator forward_iterator<It>() const { return forward_iterator<It>(it_); }
-};
-
 template <class It>
 class non_const_deref_iterator
 {

diff  --git a/libcxx/test/support/test_iterators.h b/libcxx/test/support/test_iterators.h
index 5e084d3222c68..60313a5889a52 100644
--- a/libcxx/test/support/test_iterators.h
+++ b/libcxx/test/support/test_iterators.h
@@ -117,38 +117,6 @@ class forward_iterator
     void operator,(T const &) = delete;
 };
 
-template <class It>
-class non_default_constructible_iterator
-{
-    It it_;
-
-    template <class U> friend class non_default_constructible_iterator;
-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;
-
-    non_default_constructible_iterator() = delete;
-
-    TEST_CONSTEXPR explicit non_default_constructible_iterator(It it) : it_(it) {}
-    template <class U>
-        TEST_CONSTEXPR non_default_constructible_iterator(const non_default_constructible_iterator<U>& u) : it_(u.it_) {}
-
-    TEST_CONSTEXPR reference operator*() const {return *it_;}
-    TEST_CONSTEXPR pointer operator->() const {return it_;}
-
-    TEST_CONSTEXPR_CXX14 non_default_constructible_iterator& operator++() {++it_; return *this;}
-    TEST_CONSTEXPR_CXX14 non_default_constructible_iterator operator++(int) {return non_default_constructible_iterator(it_++);}
-
-    friend TEST_CONSTEXPR bool operator==(const non_default_constructible_iterator& x, const non_default_constructible_iterator& y) {return x.it_ == y.it_;}
-    friend TEST_CONSTEXPR bool operator!=(const non_default_constructible_iterator& x, const non_default_constructible_iterator& y) {return x.it_ != y.it_;}
-
-    template <class T>
-    void operator,(T const &) = delete;
-};
-
 template <class It>
 class bidirectional_iterator
 {


        


More information about the libcxx-commits mailing list