[libc++] Avoid triggering warnings for implicit conversion

[ldionne]

Summary

This started as fixing a typo in a ADDITIONAL_COMPILE_FLAGS directive
which turned out to uncover a few places where we warned about signedness
changes.

As a fly-by fix, this updates the various __advance overloads
for style consistency.

[ldionne]

Repurpose the patch -- the main purpose is now fixing std::advance

[• Quuxplusone]

libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp

68–72

I don't see anything in https://eel.is/c++draft/iterator.operations that forbids vendors from doing a conversion to iter_difference_t here. (In fact, both std::next and std::ranges::advance require that conversion, so it shouldn't be surprising if std::advance also does it.)
The Standard says only: "Increments i by n if n is non-negative, and decrements i by -n otherwise." Presumably the intention is to use the iterator's operator+= to do that (although the Standard doesn't say); and the iterator is not guaranteed to provide any satisfactory operator+= except the one taking iter_difference_t. So if you're trying to pass Distance unmodified to operator+=, you're not always going to be able to satisfy the Standard's behavioral requirement here. The only way to guarantee that you're getting a valid, concept-semantic-modeling overload of operator+= is to do the conversion first.

This is https://cplusplus.github.io/LWG/issue3439 ; see also https://cplusplus.github.io/LWG/issue3213 which mandates convertible-to-integral for Size.

TLDR: I think this patch is misguided. If anything, we should be testing for

template<class Distance>
struct DistanceChecker {
    ~~~
    TEST_CONSTEXPR_CXX17 DistanceChecker& operator+=(difference_type) { ~~~ }
    TEST_CONSTEXPR_CXX17 DistanceChecker& operator+=(int) = delete;
}
std::advance(it, 1);  // should advance by difference_type(1), not int(1)

However, in practice I would recommend not testing for that, either; that kind of iterator type seems to violate common sense re the proper design of overload sets.

[ldionne]

libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp

68–72

Hmm, okay, I guess I am swayed. I'll repurpose this patch again to only remove the -Wsign-conversion warnings.

[ldionne]

Repurpose.

[• Quuxplusone]

LGTM at this point if buildkite is happy (which it seems it's not, yet).
If you think the advance stuff needs more thought/discussion, then feel free to ship the hash and tuple pieces, which seem utterly non-controversial, just to get them out of the way.

libcxx/include/__iterator/advance.h

60–61 ↗

(On Diff #360413)

_IntegralDistance is used only once. If you want, you could roll these back up into a single parameter

class = _EnableIf<is_integral<decltype(_VSTD::__convert_to_integral(declval<_Distance>()))>::value>

[ldionne]

libcxx/include/__iterator/advance.h

60–61 ↗

(On Diff #360413)

I think I'll leave it as-is, I find it easier to read.

[ldionne]

Fix CI (hopefully).

[• Quuxplusone]

@ldionne: Serendipitously, @jwakely just alerted me to LWG's Tentatively Ready on @tcanens' paper P2393R0 "Cleaning up integer-class types", which specifically mandates (the effect of) an explicit cast from decltype(n) to iter_difference_t<I> every time the library clauses talk about i + n or i - n. See the added section marked '?', in green.
The state-of-trunk after D106372 seems pretty darn close to what's about to be mandated — and in particular, D106372 didn't make things worse. :) Good!

libcxx/test/std/iterators/iterator.primitives/iterator.operations/advance.pass.cpp

 //
 // template <BidirectionalIterator Iter, class Distance>
 //   constexpr void advance(Iter& i, Distance n);
 //
 // template <RandomAccessIterator Iter, class Distance>
 //   constexpr void advance(Iter& i, Distance n);
 
 // Make sure we catch forced conversions to the difference_type if they happen.
-// ADDITIONAL_COMPILER_FLAGS: -Wsign-conversion
+// ADDITIONAL_COMPILE_FLAGS: -Wsign-conversion
 
 #include <iterator>
 #include <cassert>
 #include <cstddef>
 #include <type_traits>
 
 #include "test_macros.h"
 #include "test_iterators.h"
TEST_CONSTEXPR_CXX17 bool tests()
 
     return true;
 }
 
 int main(int, char**)
 {
     tests();
 #if TEST_STD_VER >= 17
     static_assert(tests(), "");
 #endif
     return 0;
 }