[libcxx-commits] [PATCH] D115422: [libc++][ranges] Add subsumption tests to `[special.mem.concepts]`.

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const marked 2 inline comments as done.
var-const added inline comments.


================
Comment at: libcxx/test/libcxx/algorithms/specialized.algorithms/special.mem.concepts/nothrow_forward_range.compile.pass.cpp:38-42
+constexpr bool forward_subsumes_input(std::ranges::__nothrow_forward_range auto)
+  requires true {
+  return true;
+}
+constexpr bool forward_subsumes_input(std::ranges::__nothrow_input_range auto);
----------------
Quuxplusone wrote:
> Quuxplusone wrote:
> > Here, `requires true` is unnecessary: the former should //properly subsume// the latter, in the sense of "proper subset".
> > 
> > Also, I think it is actually //more// important (so I'd like to see) that you test "nothrow-sentinel-for and sentinel_for subsume each other." In that case, since the subsumption is not proper, the `requires true` will be needed in order to make the one function more-constrained than the other.
> Oh, and also, please always pass [iterator types by value, but] range types by forwarding reference:
> ```
> constexpr bool forward_subsumes_input(std::ranges::__nothrow_input_range auto&&);
> ```
> Then you can use `"foo"` as your forwarding range here, too, instead of dragging in a `std::array` dependency.
Done, thanks for explaining!


================
Comment at: libcxx/test/libcxx/algorithms/specialized.algorithms/special.mem.concepts/nothrow_forward_range.compile.pass.cpp:38-42
+constexpr bool forward_subsumes_input(std::ranges::__nothrow_forward_range auto)
+  requires true {
+  return true;
+}
+constexpr bool forward_subsumes_input(std::ranges::__nothrow_input_range auto);
----------------
var-const wrote:
> Quuxplusone wrote:
> > Quuxplusone wrote:
> > > Here, `requires true` is unnecessary: the former should //properly subsume// the latter, in the sense of "proper subset".
> > > 
> > > Also, I think it is actually //more// important (so I'd like to see) that you test "nothrow-sentinel-for and sentinel_for subsume each other." In that case, since the subsumption is not proper, the `requires true` will be needed in order to make the one function more-constrained than the other.
> > Oh, and also, please always pass [iterator types by value, but] range types by forwarding reference:
> > ```
> > constexpr bool forward_subsumes_input(std::ranges::__nothrow_input_range auto&&);
> > ```
> > Then you can use `"foo"` as your forwarding range here, too, instead of dragging in a `std::array` dependency.
> Done, thanks for explaining!
Thanks! Added the tests for sentinels, PTAL.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115422