[libc++][test] Adjust move_iterator tests to allow C++20

[CaseyCarter]

Summary

These tests fail due to a couple of changes to move_iterator for C++20:

1. move_iterator<I>::operator++(int) returns void in C++20 if I doesn't model forward_iterator.
2. move_iterator<I>::reference is calculated in C++20, so I must actually have an operator*() const.

[CaseyCarter]

I'm not going to reformat *only* my additions per the clang-format instructions - that would be silly - and I suspect that folks would clang-format all of the tests if they actually wanted them formatted, so I won't format the entire file, either.

[dexonsmith]

In D79343#2023658, @CaseyCarter wrote:

I'm not going to reformat *only* my additions per the clang-format instructions - that would be silly - and I suspect that folks would clang-format all of the tests if they actually wanted them formatted, so I won't format the entire file, either.

I'm not sure how the libcxx/ maintainers feel, but clang-formatting just the changes is the operating procedure in llvm/ and clang/. Here's a script that makes it easy:
https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting
I believe there's also a git-clang-format script checked in that you can leverage by adding it to your PATH.

The rules change regularly in subtle ways, usually because of fixes/regressions in clang-format, sometimes due to changes in policy. We don't do a mass reformat of the entire codebase on every commit though, instead we just update the lines that are being changed, edging things "closer". Maybe it would be better to do a mass reformat on policy changes but we haven't done that historically.

If a file is drastically out-of-whack, it's an option to reformat it in a separate format-only commit before making a change (never the same commit as a change because that makes it hard to see what changed).

Not trying to impose rules on libc++, just providing some context; I suspect these new linters are broadly following the procedure from LLVM and Clang.

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iterator/types.pass.cpp

33

You seem to be changing from 2-space indent (the usual practice for LLVM) to 4-space indent here. I don't know if that's intentional, but if it's the right thing to do please do so in a separate commit to avoid adding noise to this patch.

[CaseyCarter]

clang-format new/changed sections of code per policy.

[CaseyCarter]

Format issues addressed.

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iterator/types.pass.cpp

33

Intentional - 4-space is the predominant indent in this file - but misguided. Thank you for the rundown of general formatting policy.

[ldionne]

Regarding the formatting changes, I personally think we should clang-format all of libc++, libc++abi and libunwind in one go. Then we'd be done with these small issues that add up. And we can even add the revision to .git-blame-ignore-revs.

[CaseyCarter]

In D79343#2025211, @ldionne wrote:

Regarding the formatting changes, I personally think we should clang-format all of libc++, libc++abi and libunwind in one go. Then we'd be done with these small issues that add up. And we can even add the revision to .git-blame-ignore-revs.

I'm the furthest thing from a core libc++ maintainer, but FWIW I agree. I set up a .git-blame-ignore-revs for the MSVC internal git repo a week or two ago with our "clang-format the things" and "change all the line endings" commits and it has been a great experience.

[dexonsmith]

In D79343#2025211, @ldionne wrote:

Regarding the formatting changes, I personally think we should clang-format all of libc++, libc++abi and libunwind in one go. Then we'd be done with these small issues that add up. And we can even add the revision to .git-blame-ignore-revs.

(FWIW, that's what I think we should do with llvm/ and clang/ too...)

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.op.incr/post.pass.cpp

 // move_iterator
 
 // move_iterator operator++(int);
 //
 //  constexpr in C++17
 
 #include <iterator>
 #include <cassert>
+#include <utility>
 
 #include "test_macros.h"
 #include "test_iterators.h"
 
+#if TEST_STD_VER > 17
+template <class It>
+void
+test_single_pass(It i, It x)
+{
+    std::move_iterator<It> r(std::move(i));
+    r++;
+    assert(std::move(r).base() == x);
+}
+#endif
+
 template <class It>
 void
 test(It i, It x)
 {
     std::move_iterator<It> r(i);
     std::move_iterator<It> rr = r++;
     assert(r.base() == x);
     assert(rr.base() == i);
 }
 
 int main(int, char**)
 {
     char s[] = "123";
+#if TEST_STD_VER > 17
+    test_single_pass(input_iterator<char*>(s), input_iterator<char*>(s+1));
+#else
     test(input_iterator<char*>(s), input_iterator<char*>(s+1));
+#endif
     test(forward_iterator<char*>(s), forward_iterator<char*>(s+1));
     test(bidirectional_iterator<char*>(s), bidirectional_iterator<char*>(s+1));
     test(random_access_iterator<char*>(s), random_access_iterator<char*>(s+1));
     test(s, s+1);
 
 #if TEST_STD_VER > 14
     {
     constexpr const char *p = "123456789";
     typedef std::move_iterator<const char *> MI;
     constexpr MI it1 = std::make_move_iterator(p);
     constexpr MI it2 = std::make_move_iterator(p+1);
     static_assert(it1 != it2, "");
     constexpr MI it3 = std::make_move_iterator(p) ++;
     static_assert(it1 == it3, "");
     static_assert(it2 != it3, "");
     }
 #endif
 
     return 0;
 }

libcxx/test/std/iterators/predef.iterators/move.iterators/move.iterator/types.pass.cpp

 #include <iterator>
 #include <type_traits>
 
 #include "test_macros.h"
 #include "test_iterators.h"
 
 template <class ValueType, class Reference>
 struct DummyIt {
     typedef std::forward_iterator_tag iterator_category;
     typedef ValueType value_type;
     typedef std::ptrdiff_t difference_type;
     typedef ValueType* pointer;
     typedef Reference reference;
+
+    Reference operator*() const;
 };
 
 template <class It>
 void
 test()
 {
     typedef std::move_iterator<It> R;
     typedef std::iterator_traits<It> T;
#if TEST_STD_VER >= 11
     }
     {
         typedef DummyIt<int, int&&> T;
         typedef std::move_iterator<T> It;
         static_assert(std::is_same<It::reference, int&&>::value, "");
     }
 #endif
 
     return 0;
 }