[libcxx-commits] [PATCH] D118686: [libc++][ranges][NFC] Test the specializations of `tuple_{size, element}` for ranges.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const marked an inline comment as done.
var-const added a comment.

In D118686#3288772 <https://reviews.llvm.org/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?



================
Comment at: libcxx/include/ranges:257
 #include <__ranges/size.h>
-#include <__ranges/subrange.h>
+#include <__ranges/subrange.h> // This also makes the specializations of `tuple_{size,element}` available.
 #include <__ranges/take_view.h>
----------------
Quuxplusone wrote:
> Please remove the comment.
Why?


================
Comment at: libcxx/test/std/ranges/tuple_specializations.compile.pass.cpp:20-21
+#include <type_traits>
+// 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..
+
----------------
Quuxplusone wrote:
> 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>`).
> 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.


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.ranges.compile.pass.cpp:11
+
+// Tested in ranges/tuple_specializations.compile.pass.cpp
----------------
Quuxplusone wrote:
> I would like us to move away from no-op tests, if possible.
Sure. I saw this done in other tests and mimicked that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118686/new/

https://reviews.llvm.org/D118686