[libcxx-commits] [PATCH] D116974: [libc++] Add ranges::in_fun_result

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp:22-28
+template <class T>
+struct ConstructibleFrom {
+  constexpr ConstructibleFrom(T c) : content{c} {}
+  T content;
+};
+
+struct Empty {};
----------------
Optional: maybe move definitions of `ConstructibleFrom` and `Empty` down so that they're right above `test()` definition and thus closer to the point of usage?


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp:60
+constexpr bool test() {
+  {
+    std::ranges::in_fun_result<int, double> res{10L, 0.};
----------------
Optional: consider adding a brief comment to each test case, e.g. `Conversion via copy construction` and `Conversion via casting (also tests that move-only types can be converted)`.


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp:69
+  {
+    std::ranges::in_fun_result<MoveOnly, Empty> res;
+    assert(res.in.get() == 1);
----------------
Perhaps also test with a copy-only type? (that could be overkill)


================
Comment at: libcxx/test/std/algorithms/algorithms.results/in_fun_result.pass.cpp:71
+    assert(res.in.get() == 1);
+    [[maybe_unused]] auto res2 = static_cast<std::ranges::in_fun_result<MoveOnly, Empty>>(std::move(res));
+    assert(res.in.get() == 0);
----------------
Ultranit: please include `<utility>`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116974