[libc++] Clean up test_iterators.h. NFCI.

[• Quuxplusone]

Summary

The majority of the changes here are whitespace.
Also simplify `ThrowingIterator`'s bookkeeping (NFC).
Also move some free operators into hidden friends, for sanity's sake.
Also `=delete` some more comma operators.
Also use `constexpr` in C++20 instead of `TEST_CONSTEXPR_CXX14`.

[zoecarver]

libcxx/test/support/test_iterators.h

693

I need to find and remove the matching comment.

722

We need some similar changes here.

[cjdb]

The suggested changes from D102020 don't properly consider the design of this type, nor its surrounding ecosystem. Please do not merge it, and instead use the appropriate helper types that are already in test_iterators.h.

libcxx/test/support/test_iterators.h

635–638

Why? This is a move-only iterator, which only makes sense in ranges land.

639–642

Why?

666

This overload needs to keep its [[nodiscard]].

667–669

Iterators don't have sentinel sub-types. We have a sentinel_wrapper for that.

[cjdb]

libcxx/test/support/test_iterators.h

639–642

Upon further thought, removing this opens a gaping hole for when a I satisfies std::input_iterator, but not _`cpp17-input-iterator`_ and doesn't result in the primary template for iterator_traits.

[• Quuxplusone]

libcxx/test/support/test_iterators.h

635–638

It makes sense anywhere it compiles, which is to say >=14.

639–642

'14-compatibility.

666

IMO, no, the [[nodiscard]] was just noise. We don't need to [[nodiscard]] everything in sight: we don't [[nodiscard]] operator*, for example, even though "obviously" any test code that actually discarded the result of *it would probably have a bug, just as much as any test code that actually discarded the result of it.base().

667–669

Hm. I don't particularly dislike sentinel_wrapper. At worst it's a tiny bit more verbose: compare the angle-bracketiness of alternatives

some_function<cpp20_input_iterator<int*>, sentinel_wrapper<cpp20_input_iterator<int*>>>();
some_function<cpp20_input_iterator<int*>, cpp20_input_iterator<int*>::sentinel>();

(Right now sentinel_wrapper is used only in "range.iter.ops.advance/advance.pass.cpp".)

I'm okay eliminating this diff for now, pending some examples of how sentinel{,_wrapper} would be used in real test cases. Certainly if we're adding sentinel we should also be refactoring some existing tests to show how it's an improvement — if it's not an improvement then we shouldn't do it.

859–860

Please void operator&() const DELETE_FUNCTION; while you're at it.

[cjdb]

libcxx/test/support/test_iterators.h

635–638

Yeah, no. This isn't useful in C++14, so it shouldn't be available. Not to mention that it depends on std::iter_value_t and std::iter_difference_t.

667–669

Hm. I don't particularly dislike sentinel_wrapper. At worst it's a tiny bit more verbose: compare the angle-bracketiness of alternatives

sentinel_wrapper is the sentinel version of our iterator wrappers; if your hard disk has a limited amount of free space where each byte counts, you'd be doing yourself a disservice as we'd need to duplicate this code for each of our other iterators too.

(Right now sentinel_wrapper is used only in "range.iter.ops.advance/advance.pass.cpp".)

It's also used in range.iter.ops/range.iter.ops.next. I should fix up range.iter.ops/range.iter.ops.prev to use it too.

I'm okay eliminating this diff for now, pending some examples of how sentinel{,_wrapper} would be used in real test cases.

If, by "real test cases", you mean "more test cases" (those places where it's used today seem real to me): those should start to flow in when algorithms start to get committed.

Certainly if we're adding sentinel we should also be refactoring some existing tests to show how it's an improvement — if it's not an improvement then we shouldn't do it.

It's a bit hard to refactor code that doesn't exist yet, but here's an example of what might exist in the near future.

auto v = std::vector{1, 2, 3};
std::ranges::find(v.begin(), sentinel_wrapper(v.end()), 0);

[ldionne]

Gentle ping -- are we doing this?

[• Quuxplusone]

Are we doing this?

Basically nah; some of this has already been committed elsewhere (e.g. removing [[nodiscard]]) and the rest is not obviously good anymore.
But I've just done a new pass over test_iterators.h and recommend these new changes. There was a surprising amount of misindentation in ThrowingIterator!

[ldionne]

LGTM when CI passes.

I'd rather keep the old formulations for operations in ThrowingIterator, but it's non-blocking.

libcxx/test/support/test_iterators.h

564

I'd rather not make this change here and instead keep the previous formulations for if (action_ == ...) if (index_ == 0) ...

[• Quuxplusone]

Oops, contiguous_iterator's operator, was already deleted (just not at the bottom of the class like the others).
Considered moving all the other iterators' operators into hidden friends (above their deleted operator,s), and then decided that that was too big an edit; might make that PR later. I don't immediately see why random_access_iterator<T> should be comparable to random_access_iterator<U>, but I could imagine us relying on that somehow in some test.

[• Quuxplusone]

This time for sure!
(Rename a nested template parameter T to T2.)

[• Quuxplusone]

Rebase on main after landing D109414. Buildkite should be happy now.

[• Quuxplusone]

n+1th time's the charm!

libcxx/test/support/test_iterators.h

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef ITERATORS_H
-#define ITERATORS_H
+#ifndef SUPPORT_TEST_ITERATORS_H
+#define SUPPORT_TEST_ITERATORS_H
 
 #include <iterator>
 #include <stdexcept>
 #include <cstddef>
 #include <cassert>
 #include <utility>
 
 #include "test_macros.h"
public:
     typedef          std::output_iterator_tag                  iterator_category;
     typedef void                                               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_CXX14 It base() const {return it_;}
 
-    TEST_CONSTEXPR_CXX14 output_iterator () {}
+    TEST_CONSTEXPR_CXX14 output_iterator() {}
     explicit TEST_CONSTEXPR_CXX14 output_iterator(It it) : it_(it) {}
     template <class U>
         TEST_CONSTEXPR_CXX14 output_iterator(const output_iterator<U>& u) :it_(u.it_) {}
 
     TEST_CONSTEXPR_CXX14 reference operator*() const {return *it_;}
 
     TEST_CONSTEXPR_CXX14 output_iterator& operator++() {++it_; return *this;}
     TEST_CONSTEXPR_CXX14 output_iterator operator++(int)
         {output_iterator tmp(*this); ++(*this); return tmp;}
 
     template <class T>
     void operator,(T const &) DELETE_FUNCTION;
 };
 
 // This is the Cpp17InputIterator requirement as described in Table 87 ([input.iterators]),
 // formerly known as InputIterator prior to C++20.
-template <class It,
-    class ItTraits = It>
+template <class It, class ItTraits = It>
 class cpp17_input_iterator
 {
     typedef std::iterator_traits<ItTraits> Traits;
     It it_;
 
     template <class U, class T> friend class cpp17_input_iterator;
 public:
     typedef          std::input_iterator_tag                   iterator_category;
public:
     friend TEST_CONSTEXPR_CXX14 bool operator!=(const cpp17_input_iterator& x, const cpp17_input_iterator& y)
         {return !(x == y);}
 
     template <class T>
     void operator,(T const &) DELETE_FUNCTION;
 };
 
 template <class T, class TV, class U, class UV>
-inline
-bool
-operator==(const cpp17_input_iterator<T, TV>& x, const cpp17_input_iterator<U, UV>& y)
+bool operator==(const cpp17_input_iterator<T, TV>& x, const cpp17_input_iterator<U, UV>& y)
 {
     return x.base() == y.base();
 }
 
 template <class T, class TV, class U, class UV>
-inline
-bool
-operator!=(const cpp17_input_iterator<T, TV>& x, const cpp17_input_iterator<U, UV>& y)
+bool operator!=(const cpp17_input_iterator<T, TV>& x, const cpp17_input_iterator<U, UV>& y)
 {
     return !(x == y);
 }
 
 template <class It>
 class forward_iterator
 {
     It it_;
public:
     friend TEST_CONSTEXPR_CXX14 bool operator!=(const forward_iterator& x, const forward_iterator& y)
         {return !(x == y);}
 
     template <class T>
     void operator,(T const &) DELETE_FUNCTION;
 };
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator==(const forward_iterator<T>& x, const forward_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator==(const forward_iterator<T>& x, const forward_iterator<U>& y)
 {
     return x.base() == y.base();
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator!=(const forward_iterator<T>& x, const forward_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator!=(const forward_iterator<T>& x, const forward_iterator<U>& y)
 {
     return !(x == y);
 }
 
 template <class It>
 class non_default_constructible_iterator
 {
     It it_;
public:
     friend TEST_CONSTEXPR_CXX14 bool operator!=(const non_default_constructible_iterator& x, const non_default_constructible_iterator& y)
         {return !(x == y);}
 
     template <class T>
     void operator,(T const &) DELETE_FUNCTION;
 };
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator==(const non_default_constructible_iterator<T>& x, const non_default_constructible_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator==(const non_default_constructible_iterator<T>& x, const non_default_constructible_iterator<U>& y)
 {
     return x.base() == y.base();
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator!=(const non_default_constructible_iterator<T>& x, const non_default_constructible_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator!=(const non_default_constructible_iterator<T>& x, const non_default_constructible_iterator<U>& y)
 {
     return !(x == y);
 }
 
 template <class It>
 class bidirectional_iterator
 {
     It it_;
public:
     TEST_CONSTEXPR_CXX14 bidirectional_iterator operator--(int)
         {bidirectional_iterator tmp(*this); --(*this); return tmp;}
 
     template <class T>
     void operator,(T const &) DELETE_FUNCTION;
 };
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator==(const bidirectional_iterator<T>& x, const bidirectional_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator==(const bidirectional_iterator<T>& x, const bidirectional_iterator<U>& y)
 {
     return x.base() == y.base();
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator!=(const bidirectional_iterator<T>& x, const bidirectional_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator!=(const bidirectional_iterator<T>& x, const bidirectional_iterator<U>& y)
 {
     return !(x == y);
 }
 
 template <class It>
 class random_access_iterator
 {
     It it_;
public:
 
     TEST_CONSTEXPR_CXX14 reference operator[](difference_type n) const {return it_[n];}
 
     template <class T>
     void operator,(T const &) DELETE_FUNCTION;
 };
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator==(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator==(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
 {
     return x.base() == y.base();
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator!=(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator!=(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
 {
     return !(x == y);
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator<(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator<(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
 {
     return x.base() < y.base();
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator<=(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator<=(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
 {
     return !(y < x);
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator>(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator>(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
 {
     return y < x;
 }
 
 template <class T, class U>
-inline
-bool TEST_CONSTEXPR_CXX14
-operator>=(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
+TEST_CONSTEXPR_CXX14
+bool operator>=(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
 {
     return !(x < y);
 }
 
 template <class T, class U>
-inline TEST_CONSTEXPR_CXX14
+TEST_CONSTEXPR_CXX14
 typename std::iterator_traits<T>::difference_type
 operator-(const random_access_iterator<T>& x, const random_access_iterator<U>& y)
 {
     return x.base() - y.base();
 }
 
-#if TEST_STD_VER >= 20
+#if TEST_STD_VER > 17
 template <class It>
 class contiguous_iterator
 {
     static_assert(std::is_pointer_v<It>, "Things probably break in this case");
 
     It it_;
 
     template <class U> friend class contiguous_iterator;
public:
     friend TEST_CONSTEXPR_CXX14 contiguous_iterator operator+(difference_type n, contiguous_iterator x)
         {x += n; return x;}
     TEST_CONSTEXPR_CXX14 contiguous_iterator& operator-=(difference_type n) {return *this += -n;}
     TEST_CONSTEXPR_CXX14 contiguous_iterator operator-(difference_type n) const
         {contiguous_iterator tmp(*this); tmp -= n; return tmp;}
 
     TEST_CONSTEXPR_CXX14 reference operator[](difference_type n) const {return it_[n];}
 
-    template <class T>
-    void operator,(T const &) DELETE_FUNCTION;
-
     friend TEST_CONSTEXPR_CXX14
     difference_type operator-(const contiguous_iterator& x, const contiguous_iterator& y) {
         return x.base() - y.base();
     }
 
     friend TEST_CONSTEXPR_CXX14
     difference_type operator<(const contiguous_iterator& x, const contiguous_iterator& y) {
         return x.base() < y.base();
public:
     friend TEST_CONSTEXPR_CXX14
     difference_type operator==(const contiguous_iterator& x, const contiguous_iterator& y) {
         return x.base() == y.base();
     }
     friend TEST_CONSTEXPR_CXX14
     difference_type operator!=(const contiguous_iterator& x, const contiguous_iterator& y) {
         return x.base() != y.base();
     }
+
+    template <class T>
+    void operator,(T const &) DELETE_FUNCTION;
 };
 #endif
 
 template <class Iter>
-inline TEST_CONSTEXPR_CXX14 Iter base(output_iterator<Iter> i) { return i.base(); }
+TEST_CONSTEXPR_CXX14 Iter base(output_iterator<Iter> i) { return i.base(); }
 
 template <class Iter>
-inline TEST_CONSTEXPR_CXX14 Iter base(cpp17_input_iterator<Iter> i) { return i.base(); }
+TEST_CONSTEXPR_CXX14 Iter base(cpp17_input_iterator<Iter> i) { return i.base(); }
 
 template <class Iter>
-inline TEST_CONSTEXPR_CXX14 Iter base(forward_iterator<Iter> i) { return i.base(); }
+TEST_CONSTEXPR_CXX14 Iter base(forward_iterator<Iter> i) { return i.base(); }
 
 template <class Iter>
-inline TEST_CONSTEXPR_CXX14 Iter base(bidirectional_iterator<Iter> i) { return i.base(); }
+TEST_CONSTEXPR_CXX14 Iter base(bidirectional_iterator<Iter> i) { return i.base(); }
 
 template <class Iter>
-inline TEST_CONSTEXPR_CXX14 Iter base(random_access_iterator<Iter> i) { return i.base(); }
+TEST_CONSTEXPR_CXX14 Iter base(random_access_iterator<Iter> i) { return i.base(); }
 
-#if TEST_STD_VER >= 20
+#if TEST_STD_VER > 17
 template <class Iter>
-inline TEST_CONSTEXPR_CXX14 Iter base(contiguous_iterator<Iter> i) { return i.base(); }
+TEST_CONSTEXPR_CXX14 Iter base(contiguous_iterator<Iter> i) { return i.base(); }
 #endif
 
 template <class Iter>    // everything else
-inline TEST_CONSTEXPR_CXX14 Iter base(Iter i) { return i; }
+TEST_CONSTEXPR_CXX14 Iter base(Iter i) { return i; }
 
 template <typename T>
 struct ThrowingIterator {
     typedef std::bidirectional_iterator_tag iterator_category;
     typedef ptrdiff_t                       difference_type;
     typedef const T                         value_type;
     typedef const T *                       pointer;
     typedef const T &                       reference;
 
     enum ThrowingAction { TAIncrement, TADecrement, TADereference, TAAssignment, TAComparison };
 
-//  Constructors
-    ThrowingIterator ()
+    ThrowingIterator()
         : begin_(nullptr), end_(nullptr), current_(nullptr), action_(TADereference), index_(0) {}
-    ThrowingIterator (const T *first, const T *last, size_t index = 0, ThrowingAction action = TADereference)
+    explicit ThrowingIterator(const T *first, const T *last, size_t index = 0, ThrowingAction action = TADereference)
         : begin_(first), end_(last), current_(first), action_(action), index_(index) {}
-    ThrowingIterator (const ThrowingIterator &rhs)
+    ThrowingIterator(const ThrowingIterator &rhs)
         : begin_(rhs.begin_), end_(rhs.end_), current_(rhs.current_), action_(rhs.action_), index_(rhs.index_) {}
-    ThrowingIterator & operator= (const ThrowingIterator &rhs)
-    {
-    if (action_ == TAAssignment)
-    {
-        if (index_ == 0)
+
+    ThrowingIterator& operator=(const ThrowingIterator& rhs) {
+        if (action_ == TAAssignment && --index_ < 0) {
 #ifndef TEST_HAS_NO_EXCEPTIONS
-            throw std::runtime_error ("throw from iterator assignment");
+            throw std::runtime_error("throw from iterator assignment");
 #else
             assert(false);
 #endif
-
-        else
-            --index_;
         }
-    begin_   = rhs.begin_;
-    end_     = rhs.end_;
+        begin_ = rhs.begin_;
+        end_ = rhs.end_;
         current_ = rhs.current_;
-    action_  = rhs.action_;
-    index_   = rhs.index_;
+        action_ = rhs.action_;
+        index_ = rhs.index_;
         return *this;
     }
 
-//  iterator operations
-    reference operator*() const
-    {
-    if (action_ == TADereference)
-    {
-        if (index_ == 0)
+    reference operator*() const {
+        if (action_ == TADereference && --index_ < 0) {
 #ifndef TEST_HAS_NO_EXCEPTIONS
-            throw std::runtime_error ("throw from iterator dereference");
+            throw std::runtime_error("throw from iterator dereference");
 #else
             assert(false);
 #endif
-        else
-            --index_;
         }
         return *current_;
     }
 
-    ThrowingIterator & operator++()
-    {
-    if (action_ == TAIncrement)
-    {
-        if (index_ == 0)
+    ThrowingIterator& operator++() {
+        if (action_ == TAIncrement && --index_ < 0) {
 #ifndef TEST_HAS_NO_EXCEPTIONS
-            throw std::runtime_error ("throw from iterator increment");
+            throw std::runtime_error("throw from iterator increment");
 #else
             assert(false);
 #endif
-        else
-            --index_;
         }
         ++current_;
         return *this;
     }
 
-    ThrowingIterator operator++(int)
-    {
+    ThrowingIterator operator++(int) {
         ThrowingIterator temp = *this;
         ++(*this);
         return temp;
     }
 
-    ThrowingIterator & operator--()
-    {
-    if (action_ == TADecrement)
-    {
-        if (index_ == 0)
+    ThrowingIterator& operator--() {
+        if (action_ == TADecrement && --index_ < 0) {
 #ifndef TEST_HAS_NO_EXCEPTIONS
-            throw std::runtime_error ("throw from iterator decrement");
+            throw std::runtime_error("throw from iterator decrement");
 #else
             assert(false);
 #endif
-        else
-            --index_;
         }
         --current_;
         return *this;
     }
 
     ThrowingIterator operator--(int) {
         ThrowingIterator temp = *this;
         --(*this);
         return temp;
     }
 
-    bool operator== (const ThrowingIterator &rhs) const
-    {
-    if (action_ == TAComparison)
-    {
-        if (index_ == 0)
+    friend bool operator==(const ThrowingIterator& a, const ThrowingIterator& b) {
+        if (a.action_ == TAComparison && --a.index_ < 0) {
 #ifndef TEST_HAS_NO_EXCEPTIONS
-            throw std::runtime_error ("throw from iterator comparison");
+            throw std::runtime_error("throw from iterator comparison");
 #else
             assert(false);
 #endif
-        else
-            --index_;
         }
-    bool atEndL =     current_ == end_;
-    bool atEndR = rhs.current_ == rhs.end_;
+        bool atEndL = a.current_ == a.end_;
+        bool atEndR = b.current_ == b.end_;
         if (atEndL != atEndR) return false;  // one is at the end (or empty), the other is not.
         if (atEndL) return true;             // both are at the end (or empty)
-    return current_ == rhs.current_;
+        return a.current_ == b.current_;
     }
 
+    friend bool operator!=(const ThrowingIterator& a, const ThrowingIterator& b) {
+        return !(a == b);
+    }
+
+    template <class T2>
+    void operator,(T2 const &) DELETE_FUNCTION;
+
 private:
     const T* begin_;
     const T* end_;
     const T* current_;
     ThrowingAction action_;
-    mutable size_t index_;
+    mutable int index_;
 };
 
 template <typename T>
-bool operator== (const ThrowingIterator<T>& a, const ThrowingIterator<T>& b)
-{   return a.operator==(b); }
-
-template <typename T>
-bool operator!= (const ThrowingIterator<T>& a, const ThrowingIterator<T>& b)
-{   return !a.operator==(b); }
-
-template <typename T>
 struct NonThrowingIterator {
     typedef std::bidirectional_iterator_tag iterator_category;
     typedef ptrdiff_t                       difference_type;
     typedef const T                         value_type;
     typedef const T *                       pointer;
     typedef const T &                       reference;
 
-//  Constructors
-    NonThrowingIterator ()
+    NonThrowingIterator()
         : begin_(nullptr), end_(nullptr), current_(nullptr) {}
-    NonThrowingIterator (const T *first, const T* last)
+    explicit NonThrowingIterator(const T *first, const T *last)
         : begin_(first), end_(last), current_(first) {}
-    NonThrowingIterator (const NonThrowingIterator &rhs)
+    NonThrowingIterator(const NonThrowingIterator& rhs)
         : begin_(rhs.begin_), end_(rhs.end_), current_(rhs.current_) {}
-    NonThrowingIterator & operator= (const NonThrowingIterator &rhs) TEST_NOEXCEPT
-    {
-    begin_   = rhs.begin_;
-    end_     = rhs.end_;
+
+    NonThrowingIterator& operator=(const NonThrowingIterator& rhs) TEST_NOEXCEPT {
+        begin_ = rhs.begin_;
+        end_ = rhs.end_;
         current_ = rhs.current_;
         return *this;
     }
 
-//  iterator operations
-    reference operator*() const TEST_NOEXCEPT
-    {
+    reference operator*() const TEST_NOEXCEPT {
         return *current_;
     }
 
-    NonThrowingIterator & operator++() TEST_NOEXCEPT
-    {
+    NonThrowingIterator& operator++() TEST_NOEXCEPT {
         ++current_;
         return *this;
     }
 
-    NonThrowingIterator operator++(int) TEST_NOEXCEPT
-    {
+    NonThrowingIterator operator++(int) TEST_NOEXCEPT {
         NonThrowingIterator temp = *this;
         ++(*this);
         return temp;
     }
 
-    NonThrowingIterator & operator--() TEST_NOEXCEPT
-    {
+    NonThrowingIterator & operator--() TEST_NOEXCEPT {
         --current_;
         return *this;
     }
 
-    NonThrowingIterator operator--(int) TEST_NOEXCEPT
-    {
+    NonThrowingIterator operator--(int) TEST_NOEXCEPT {
         NonThrowingIterator temp = *this;
         --(*this);
         return temp;
     }
 
-    bool operator== (const NonThrowingIterator &rhs) const TEST_NOEXCEPT
-    {
-    bool atEndL =     current_ == end_;
-    bool atEndR = rhs.current_ == rhs.end_;
+    friend bool operator==(const NonThrowingIterator& a, const NonThrowingIterator& b) TEST_NOEXCEPT {
+        bool atEndL = a.current_ == a.end_;
+        bool atEndR = b.current_ == b.end_;
         if (atEndL != atEndR) return false;  // one is at the end (or empty), the other is not.
         if (atEndL) return true;             // both are at the end (or empty)
-    return current_ == rhs.current_;
+        return a.current_ == b.current_;
     }
 
+    friend bool operator==(const NonThrowingIterator& a, const NonThrowingIterator& b) TEST_NOEXCEPT {
+        return !(a == b);
+    }
+
+    template <class T2>
+    void operator,(T2 const &) DELETE_FUNCTION;
+
 private:
-    const T* begin_;
-    const T* end_;
-    const T* current_;
+    const T *begin_;
+    const T *end_;
+    const T *current_;
 };
 
-template <typename T>
-bool operator== (const NonThrowingIterator<T>& a, const NonThrowingIterator<T>& b) TEST_NOEXCEPT
-{   return a.operator==(b); }
-
-template <typename T>
-bool operator!= (const NonThrowingIterator<T>& a, const NonThrowingIterator<T>& b) TEST_NOEXCEPT
-{   return !a.operator==(b); }
-
 #ifdef TEST_SUPPORTS_RANGES
 
-// clang-format off
-
 template <class I>
 struct cpp20_input_iterator {
     using value_type = std::iter_value_t<I>;
     using difference_type = std::iter_difference_t<I>;
     using iterator_concept = std::input_iterator_tag;
 
     cpp20_input_iterator() = delete;
 
     cpp20_input_iterator(cpp20_input_iterator&&) = default;
     cpp20_input_iterator& operator=(cpp20_input_iterator&&) = default;
 
     cpp20_input_iterator(cpp20_input_iterator const&) = delete;
     cpp20_input_iterator& operator=(cpp20_input_iterator const&) = delete;
 
     explicit constexpr cpp20_input_iterator(I base) : base_(std::move(base)) {}
 
     constexpr decltype(auto) operator*() const { return *base_; }
 
     constexpr cpp20_input_iterator& operator++() {
         ++base_;
         return *this;
     }
 
     constexpr void operator++(int) { ++base_; }
 
-  constexpr I const& base() const& { return base_; }
+    constexpr const I& base() const& { return base_; }
 
     constexpr I base() && { return std::move(base_); }
 
+    template <class T>
+    void operator,(T const &) DELETE_FUNCTION;
+
 private:
     I base_ = I();
 };
 
 template <std::input_or_output_iterator I>
 struct iterator_concept {
     using type = std::output_iterator_tag;
 };
 
 template <std::input_iterator I>
 struct iterator_concept<I> {
     using type = std::input_iterator_tag;
 };
 
 template <std::forward_iterator I>
 struct iterator_concept<I> {
     using type = std::forward_iterator_tag;
 };
 
 template <std::bidirectional_iterator I>
 struct iterator_concept<I> {
     using type = std::bidirectional_iterator_tag;
 };
 
 template <std::random_access_iterator I>
 struct iterator_concept<I> {
     using type = std::random_access_iterator_tag;
 };
 
 template<std::contiguous_iterator I>
 struct iterator_concept<I> {
     using type = std::contiguous_iterator_tag;
 };
 
 template <std::input_or_output_iterator I>
 using iterator_concept_t = typename iterator_concept<I>::type;
 
 template<std::input_or_output_iterator>
 struct iter_value_or_void { using type = void; };
 
 template<std::input_iterator I>
 struct iter_value_or_void<I> {
     using type = std::iter_value_t<I>;
 };
 
 // Iterator adaptor that counts the number of times the iterator has had a successor/predecessor
 // operation called. Has two recorders:
 // * `stride_count`, which records the total number of calls to an op++, op--, op+=, or op-=.
 // * `stride_displacement`, which records the displacement of the calls. This means that both
 //   op++/op+= will increase the displacement counter by 1, and op--/op-= will decrease the
 //   displacement counter by 1.
 template <std::input_or_output_iterator I>
 class stride_counting_iterator {
 public:
     using value_type = typename iter_value_or_void<I>::type;
     using difference_type = std::iter_difference_t<I>;
     using iterator_concept = iterator_concept_t<I>;
 
     stride_counting_iterator() = default;
 
     constexpr explicit stride_counting_iterator(I current) : base_(std::move(current)) {}
 
-  constexpr I const& base() const& { return base_; }
+    constexpr const I& base() const& { return base_; }
 
     constexpr I base() && { return std::move(base_); }
 
     constexpr difference_type stride_count() const { return stride_count_; }
 
     constexpr difference_type stride_displacement() const { return stride_displacement_; }
 
     constexpr decltype(auto) operator*() const { return *base_; }
 
     constexpr decltype(auto) operator[](difference_type const n) const { return base_[n]; }
 
-  constexpr stride_counting_iterator& operator++()
-  {
+    constexpr stride_counting_iterator& operator++() {
         ++base_;
         ++stride_count_;
         ++stride_displacement_;
         return *this;
     }
 
     constexpr void operator++(int) { ++*this; }
 
     constexpr stride_counting_iterator operator++(int)
         requires std::forward_iterator<I>
     {
         auto temp = *this;
         ++*this;
         return temp;
     }
 
     constexpr stride_counting_iterator& operator--()
         requires std::bidirectional_iterator<I>
     {
         --base_;
         ++stride_count_;
         --stride_displacement_;
         return *this;
     }
 
     constexpr stride_counting_iterator operator--(int)
         requires std::bidirectional_iterator<I>
     {
         auto temp = *this;
         --*this;
         return temp;
     }
 
     constexpr stride_counting_iterator& operator+=(difference_type const n)
         requires std::random_access_iterator<I>
     {
         base_ += n;
         ++stride_count_;
         ++stride_displacement_;
         return *this;
     }
 
     constexpr stride_counting_iterator& operator-=(difference_type const n)
         requires std::random_access_iterator<I>
     {
         base_ -= n;
         ++stride_count_;
         --stride_displacement_;
         return *this;
     }
 
     friend constexpr stride_counting_iterator operator+(stride_counting_iterator i, difference_type const n)
         requires std::random_access_iterator<I>
     {
         return i += n;
     }
 
     friend constexpr stride_counting_iterator operator+(difference_type const n, stride_counting_iterator i)
         requires std::random_access_iterator<I>
     {
         return i += n;
     }
 
     friend constexpr stride_counting_iterator operator-(stride_counting_iterator i, difference_type const n)
         requires std::random_access_iterator<I>
     {
         return i -= n;
     }
 
     friend constexpr difference_type operator-(stride_counting_iterator const& x, stride_counting_iterator const& y)
         requires std::sized_sentinel_for<I, I>
     {
         return x.base() - y.base();
     }
 
     constexpr bool operator==(stride_counting_iterator const& other) const
         requires std::sentinel_for<I, I>
     {
         return base_ == other.base_;
     }
 
     template <std::sentinel_for<I> S>
     constexpr bool operator==(S const last) const
     {
         return base_ == last;
     }
 
     friend constexpr bool operator<(stride_counting_iterator const& x, stride_counting_iterator const& y)
         requires std::random_access_iterator<I>
     {
         return x.base_ < y.base_;
     }
 
     friend constexpr bool operator>(stride_counting_iterator const& x, stride_counting_iterator const& y)
         requires std::random_access_iterator<I>
     {
         return y < x;
     }
 
     friend constexpr bool operator<=(stride_counting_iterator const& x, stride_counting_iterator const& y)
         requires std::random_access_iterator<I>
     {
         return !(y < x);
     }
 
     friend constexpr bool operator>=(stride_counting_iterator const& x, stride_counting_iterator const& y)
         requires std::random_access_iterator<I>
     {
         return !(x < y);
     }
 
+    template <class T>
+    void operator,(T const &) DELETE_FUNCTION;
+
 private:
     I base_;
     difference_type stride_count_ = 0;
     difference_type stride_displacement_ = 0;
 };
 
 template<class T, class U>
 concept sentinel_for_base = requires(U const& u) {
     u.base();
     requires std::input_or_output_iterator<std::remove_cvref_t<decltype(u.base())>>;
     requires std::equality_comparable_with<T, decltype(u.base())>;
 };
 
 template <std::input_or_output_iterator I>
 class sentinel_wrapper {
 public:
     sentinel_wrapper() = default;
     constexpr explicit sentinel_wrapper(I base) : base_(std::move(base)) {}
 
-  constexpr bool operator==(I const& other) const requires std::equality_comparable<I> {
+    constexpr bool operator==(const I& other) const requires std::equality_comparable<I> {
         return base_ == other;
     }
 
     constexpr const I& base() const& { return base_; }
     constexpr I base() && { return std::move(base_); }
 
     template<std::input_or_output_iterator I2>
         requires sentinel_for_base<I, I2>
-  constexpr bool operator==(I2 const& other) const {
+    constexpr bool operator==(const I2& other) const {
         return base_ == other.base();
     }
 
 private:
     I base_ = I();
 };
 
 template <class It>
 class three_way_contiguous_iterator
 {
     static_assert(std::is_pointer_v<It>, "Things probably break in this case");
 
     It it_;
 
     template <class U> friend class three_way_contiguous_iterator;
 public:
     typedef          std::contiguous_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;
     typedef typename std::remove_pointer<It>::type             element_type;
 
-    TEST_CONSTEXPR_CXX14 It base() const {return it_;}
+    constexpr It base() const {return it_;}
 
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator() : it_() {}
-    explicit TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator(It it) : it_(it) {}
+    constexpr three_way_contiguous_iterator() : it_() {}
+    constexpr explicit three_way_contiguous_iterator(It it) : it_(it) {}
     template <class U>
-        TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator(const three_way_contiguous_iterator<U>& u) : it_(u.it_) {}
+    constexpr three_way_contiguous_iterator(const three_way_contiguous_iterator<U>& u) : it_(u.it_) {}
 
-    TEST_CONSTEXPR_CXX14 reference operator*() const {return *it_;}
-    TEST_CONSTEXPR_CXX14 pointer operator->() const {return it_;}
+    constexpr reference operator*() const {return *it_;}
+    constexpr pointer operator->() const {return it_;}
 
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator& operator++() {++it_; return *this;}
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator operator++(int)
-        {three_way_contiguous_iterator tmp(*this); ++(*this); return tmp;}
-
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator& operator--() {--it_; return *this;}
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator operator--(int)
-        {three_way_contiguous_iterator tmp(*this); --(*this); return tmp;}
-
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator& operator+=(difference_type n) {it_ += n; return *this;}
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator operator+(difference_type n) const
-        {three_way_contiguous_iterator tmp(*this); tmp += n; return tmp;}
-    friend TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator operator+(difference_type n, three_way_contiguous_iterator x)
-        {x += n; return x;}
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator& operator-=(difference_type n) {return *this += -n;}
-    TEST_CONSTEXPR_CXX14 three_way_contiguous_iterator operator-(difference_type n) const
-        {three_way_contiguous_iterator tmp(*this); tmp -= n; return tmp;}
+    constexpr three_way_contiguous_iterator& operator++() {++it_; return *this;}
+    constexpr three_way_contiguous_iterator operator++(int) {auto tmp = *this; ++(*this); return tmp;}
 
-    TEST_CONSTEXPR_CXX14 reference operator[](difference_type n) const {return it_[n];}
+    constexpr three_way_contiguous_iterator& operator--() {--it_; return *this;}
+    constexpr three_way_contiguous_iterator operator--(int) {auto tmp = *this; --(*this); return tmp;}
+
+    constexpr three_way_contiguous_iterator& operator+=(difference_type n) {it_ += n; return *this;}
+    constexpr auto operator+(difference_type n) const {auto tmp = *this; tmp += n; return tmp;}
+    friend constexpr auto operator+(difference_type n, three_way_contiguous_iterator x) {x += n; return x;}
+    constexpr three_way_contiguous_iterator& operator-=(difference_type n) {return *this += -n;}
+    constexpr auto operator-(difference_type n) const {auto tmp = *this; tmp -= n; return tmp;}
+
+    constexpr reference operator[](difference_type n) const {return it_[n];}
 
     template <class T>
     void operator,(T const &) DELETE_FUNCTION;
 
-    friend TEST_CONSTEXPR_CXX14
+    friend constexpr
     difference_type operator-(const three_way_contiguous_iterator& x, const three_way_contiguous_iterator& y) {
         return x.base() - y.base();
     }
 
     friend auto operator<=>(const three_way_contiguous_iterator&, const three_way_contiguous_iterator&) = default;
 };
 
-// clang-format on
-
 #endif // TEST_STD_VER > 17 && defined(__cpp_lib_concepts)
 
 #undef DELETE_FUNCTION
 
-#endif // ITERATORS_H
+#endif // SUPPORT_TEST_ITERATORS_H