[libc++][ranges][NFC] Test the specializations of `tuple_{size,element}` for ranges.

[var-const]

Summary

Also update the synopsis in <ranges> to mention the specializations.

[• Quuxplusone]

I believe this is redundant with D108054, which was committed quite a while ago now.
Anyway, specializations and customizations (of swap, hash, get, tuple_size, enable_borrowed_range, is_error_code_enum,...) belong next to the class they appertain to, not in a separate header. So if there's actually something to do here, please move the code into the subrange.h header (at which point it becomes at best a diff against what's there now).

[var-const]

• remove the specializations to address the feedback;
• add a link to the patch to the Ranges status page;
• rebase on main.

[var-const]

In D118686#3287611, @Quuxplusone wrote:

I believe this is redundant with D108054, which was committed quite a while ago now.
Anyway, specializations and customizations (of swap, hash, get, tuple_size, enable_borrowed_range, is_error_code_enum,...) belong next to the class they appertain to, not in a separate header. So if there's actually something to do here, please move the code into the subrange.h header (at which point it becomes at best a diff against what's there now).

Oh, thanks a lot, I missed that. The new code is indeed unnecessary.

I think updating the synopsis is still necessary, though. What do you think about the tests? They test the specializations more directly and have the advantage of being compile-time tests, but on the other hand, don't really provide more coverage than the tests from the patch you linked.

[• Quuxplusone]

Requesting changes, in the sense that I think this PR should simply be abandoned.
(If the tests are cleaned up a bit, then sure, they don't seem harmful. But I won't be a positive approver on this; I'll let two other people go on record as claiming it's useful.)

libcxx/include/ranges

257

Please remove the comment.

libcxx/test/std/ranges/tuple_specializations.compile.pass.cpp

21–22

This comment is unhelpful, because it is unspecified and unobservable whether <ranges> includes <utility>. (I would bet that it does.)
Anyway, if you really really want to make sure, just include nothing but <ranges>, and s/is_same_v/same_as/ below (because same_as is guaranteed to exist in <concepts> which is guaranteed to be included by <ranges>).

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.ranges.compile.pass.cpp

11 ↗

(On Diff #405036)

I would like us to move away from no-op tests, if possible.

[var-const]

In D118686#3288772, @Quuxplusone wrote:

Requesting changes, in the sense that I think this PR should simply be abandoned.
(If the tests are cleaned up a bit, then sure, they don't seem harmful. But I won't be a positive approver on this; I'll let two other people go on record as claiming it's useful.)

Changing the synopsis is still necessary though, right? Also, how would you like to clean up the tests?

libcxx/include/ranges

257

Why?

libcxx/test/std/ranges/tuple_specializations.compile.pass.cpp

21–22

it is unspecified and unobservable whether <ranges> includes <utility>. (I would bet that it does.)

I still don't think it's a good idea to include it on purpose.

just include nothing but <ranges>, and s/is_same_v/same_as/ below

Done, thanks.

libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.ranges.compile.pass.cpp

11 ↗

(On Diff #405036)

Sure. I saw this done in other tests and mimicked that.

[var-const]

In D118686#3288772, @Quuxplusone wrote:

they don't seem harmful.

They test a requirement from the Standard that we otherwise test more-or-less accidentally, so it seems like they are actually useful.

[var-const]

Address feedback.

[ldionne]

The synopsis updates LGTM. Adding a test for the mandatory inclusions do make sense to me as well, however shouldn't we be doing that via libcxx/utils/generate_header_inclusion_tests.py instead?

[• Quuxplusone]

libcxx/include/ranges

257

Because (1) we don't add comments for any other headers, and (2) the fact that __ranges/subrange.h includes the API of std::ranges::subrange is obvious. We don't need to say "This makes subrange available, and all its methods, and its get overload, and its tuple_size and tuple_element, and its enable_borrowed_range, and its enable_view, and its..." We don't need to say any of that, because it's obvious from the filename.

[var-const]

Fix a comment in <ranges> regarding includes and rebase on main.

[var-const]

libcxx/include/ranges

257

Ah, thanks for calling it out, the comment is indeed wrong. It should refer to the "general" templates, not the specializations. PTAL.

[var-const]

In D118686#3288925, @ldionne wrote:

shouldn't we be doing that via libcxx/utils/generate_header_inclusion_tests.py instead?

The Standard says in [tuple.helper](https://eel.is/c++draft/tuple.helper#6):

In addition to being available via inclusion of the `<tuple>` header, the template is available when any of the headers `<array>`, `<ranges>`, or `<utility>` are included.

(the templates in question are tuple_size and tuple_element)

I don't think generate_header_inclusion_tests.py can do that because it checks for inclusion of headers, not for the availability of particular classes and functions.

The existing tests for the similar requirement in <array> and <utility> do it by instantiating every distinct specialization of array and pair, respectively (test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp and test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.utility.pass.cpp). Now that I think about it, it seems like I should move the new tests under test/std/utilities/tuple/tuple.tuple/tuple.helper, what do you think? Also, would it make sense to instantiate tuple_size and tuple_element for something other than the range specializations (because if I'm reading the requirement correctly, the templates should become generally available, not just the specializations)?

[ldionne]

In D118686#3289137, @var-const wrote:

I don't think generate_header_inclusion_tests.py can do that because it checks for inclusion of headers, not for the availability of particular classes and functions.

Oh, now that you show me the standard wording, yes I agree with you 100%.

The existing tests for the similar requirement in <array> and <utility> do it by instantiating every distinct specialization of array and pair, respectively (test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp and test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.utility.pass.cpp). Now that I think about it, it seems like I should move the new tests under test/std/utilities/tuple/tuple.tuple/tuple.helper, what do you think?

Yes, that sounds like the correct thing to do.

Also, would it make sense to instantiate tuple_size and tuple_element for something other than the range specializations (because if I'm reading the requirement correctly, the templates should become generally available, not just the specializations)?

I'm neutral on this. I think we should aim for the simplest test that gives us confidence that we're doing the right thing. We should avoid getting into diminishing returns IMO.

[var-const]

Move the new test under test/std/utilities/tuple/tuple.tuple/tuple.helper so
that it's next to similar tests for <array> and <utility>.

[var-const]

In D118686#3291426, @ldionne wrote:

In D118686#3289137, @var-const wrote:

Yes, that sounds like the correct thing to do.

Done.

I'm neutral on this. I think we should aim for the simplest test that gives us confidence that we're doing the right thing. We should avoid getting into diminishing returns IMO.

Ok, in that case, I'll just test the subrange specializations, similar to the existing tests.

[var-const]

Address review feedback:

• remove a comment in the <ranges> header;
• add a test case for a sized range with an unsized sentinel.

[ldionne]

LGTM.

I understand @Quuxplusone 's point about this being somewhat redundant, however adding these tests is consistent with what we're doing for the <array> and <utility> tests in the same directory. If we don't find that this adds enough value, we should be removing the tests for <array> and <utility> instead.

However, from my point of view, these three tests do add some value (especially since we like to move stuff around in the headers, and I can definitely imagine we'd break users relying on this guarantee).

[var-const]

libcxx/include/ranges

257

(Note: removed the comment per our offline discussion)

[var-const]

@Quuxplusone Per our offline discussion, my understanding was that you're okay with the patch modulo two comments which I have since addressed. I'll merge based on this understanding -- I can address any additional feedback in a follow-up if necessary.

libcxx/docs/Status/RangesPaper.csv

 Section,Description,Dependencies,Assignee,Complete
-`[tuple.helper] <https://wg21.link/tuple.helper>`_,Update <tuple> includes.,None,Konstantin Varlamov,Not started
+`[tuple.helper] <https://wg21.link/tuple.helper>`_,`Update <tuple> includes. <https://llvm.org/D118686>`_,None,Konstantin Varlamov,✅
 `[range.cmp] <https://wg21.link/range.cmp>`_,"| `ranges::equal_to <https://llvm.org/D100429>`_
 | `ranges::not_equal_to <https://llvm.org/D100429>`_
 | `ranges::less <https://llvm.org/D100429>`_
 | `ranges::greater <https://llvm.org/D100429>`_
 | `ranges::less_equal <https://llvm.org/D100429>`_
 | `ranges::greater_equal <https://llvm.org/D100429>`_",None,Zoe Carver,✅
 `[readable.traits] <https://wg21.link/readable.traits>`_,"| `indirectly_readable_traits <https://llvm.org/D99461>`_
 | `iter_value_t <https://llvm.org/D99863>`_",None,Christopher Di Bella,✅

libcxx/include/ranges

template<class W, class Bound>
     inline constexpr bool enable_borrowed_range<iota_view<W, Bound>> = true;
 
   // [range.join], join view
   template<input_range V>
     requires view<V> && input_range<range_reference_t<V>>
   class join_view;
 }
 
+namespace std {
+  namespace views = ranges::views;
+
+  template<class T> struct tuple_size;
+  template<size_t I, class T> struct tuple_element;
+
+  template<class I, class S, ranges::subrange_kind K>
+  struct tuple_size<ranges::subrange<I, S, K>>
+    : integral_constant<size_t, 2> {};
+
+  template<class I, class S, ranges::subrange_kind K>
+  struct tuple_element<0, ranges::subrange<I, S, K>> {
+    using type = I;
+  };
+
+  template<class I, class S, ranges::subrange_kind K>
+  struct tuple_element<1, ranges::subrange<I, S, K>> {
+    using type = S;
+  };
+
+  template<class I, class S, ranges::subrange_kind K>
+  struct tuple_element<0, const ranges::subrange<I, S, K>> {
+    using type = I;
+  };
+
+  template<class I, class S, ranges::subrange_kind K>
+  struct tuple_element<1, const ranges::subrange<I, S, K>> {
+    using type = S;
+  };
+}
 */
 
 // Make sure all feature-test macros are available.
 #include <version>
 // Enable the contents of the header only when libc++ was built with LIBCXX_ENABLE_INCOMPLETE_FEATURES.
 #if !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
 
 #include <__config>

 #include <__ranges/enable_borrowed_range.h>
 #include <__ranges/enable_view.h>
 #include <__ranges/iota_view.h>
 #include <__ranges/join_view.h>
 #include <__ranges/ref_view.h>
 #include <__ranges/reverse_view.h>
 #include <__ranges/single_view.h>
 #include <__ranges/size.h>
-#include <__ranges/subrange.h>
+#include <__ranges/subrange.h> // Also makes `tuple_size` and `tuple_element` available as required by the standard.
 #include <__ranges/take_view.h>
 #include <__ranges/transform_view.h>
 #include <__ranges/view_interface.h>
 #include <compare>          // Required by the standard.
 #include <initializer_list> // Required by the standard.
 #include <iterator>         // Required by the standard.
 #include <type_traits>
 

libcxx/test/std/ranges/tuple_specializations.compile.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+// 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
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+
+// <ranges>
+
+//  template<class T> struct tuple_size;
+//  template<size_t I, class T> struct tuple_element;
+
+#include <ranges>
+// Note: make sure to not include `<utility>` (or any other header including `<utility>`) because it also makes some
+// tuple specializations available, thus obscuring whether the `<ranges>` includes work correctly.
+
+using Iterator = int*;
+
+class SizedSentinel {
+public:
+    constexpr bool operator==(int*) const;
+    friend constexpr ptrdiff_t operator-(const SizedSentinel&, int*);
+    friend constexpr ptrdiff_t operator-(int*, const SizedSentinel&);
+};
+
+static_assert(std::sized_sentinel_for<SizedSentinel, Iterator>);
+using SizedRange = std::ranges::subrange<Iterator, SizedSentinel>;
+
+using UnsizedSentinel = std::unreachable_sentinel_t;
+static_assert(!std::sized_sentinel_for<UnsizedSentinel, Iterator>);
+using UnsizedRange = std::ranges::subrange<Iterator, UnsizedSentinel>;
+
+static_assert(std::tuple_size<SizedRange>::value == 2);
+static_assert(std::tuple_size<UnsizedRange>::value == 2);
+
+template <int I, class Range, class Expected>
+constexpr bool test_tuple_element() {
+  static_assert(std::same_as<typename std::tuple_element<I, Range>::type, Expected>);
+  static_assert(std::same_as<typename std::tuple_element<I, const Range>::type, Expected>);
+  // Note: the Standard does not mandate a specialization of `tuple_element` for volatile, so trying a `volatile Range`
+  // would fail to compile.
+
+  return true;
+}
+
+int main(int, char**) {
+  static_assert(test_tuple_element<0, SizedRange, Iterator>());
+  static_assert(test_tuple_element<1, SizedRange, SizedSentinel>());
+  static_assert(test_tuple_element<0, UnsizedRange, Iterator>());
+  static_assert(test_tuple_element<1, UnsizedRange, UnsizedSentinel>());
+
+  return 0;
+}