[libc++][ranges] Add ranges::out_value_result

[var-const]

libcxx/include/__algorithm/out_value_result.h

24

I think this is no longer necessary? Should it check for C++20 and later instead?

libcxx/test/std/algorithms/algorithms.results/out_value_result.pass.cpp

11

I think this is no longer necessary as well.

12

Hmm, but the header file isn't guarded on incomplete ranges?

27

Nit: a few of these lines are longer than 120 columns.

Optional: maybe use a type alias to cut down the boilerplate? I find it a little hard to separate the useful information in each check.

template <class T, class U>
using Res = std::ranges::out_value_result<T, U>; // Any short name would do

36

Question: would it make sense to also check rvalue references for good measure?

41

Nit: a similar check above uses a value, not a reference, for the second template argument. Can this be consistent? (assuming either can be used in this case)

64

A few test cases I'd like to add:

• check that out_value_result is trivial and standard layout when it is true of the arguments;
• check default construction;
• check that out_value_result is copyable and movable if the arguments are (I think a static_assert is enough);
• check that the rvalue overload of the conversion operator actually moves the arguments (by using a helper type that counts the number of copies and moves);
• check that the [[no_unique_address]] optimization is used;
• maybe check aggregate initialization (might be overkill).

65

Can you please add a comment to each test case that briefly explains what it does?

[var-const]

libcxx/test/std/algorithms/algorithms.results/out_value_result.pass.cpp

64

check that the [[no_unique_address]] optimization is used;

Ah, I see that it's addressed in D121435.

[var-const]

libcxx/test/std/algorithms/algorithms.results/out_value_result.pass.cpp

64

I spoke too soon -- while D121435 adds a few such tests, it doesn't address out_value_result, so the original comment still stands.

[philnik]

• Update to reflect changes from D121435

[philnik]

I haven't read all you comments @var-const. I updated the tests reflect the changes from D121435, that should address all your comments. If it doesn't, then we might be missing test coverage in all tests. I also updated the naming style here, which I will do for the other result types in a different PR.

[var-const]

I haven't read all you comments @var-const. I updated the tests reflect the changes from D121435, that should address all your comments.

I think a few of those comments are still relevant (unless I'm missing something).

libcxx/test/std/algorithms/algorithms.results/out_value_result.pass.cpp

27

This is still relevant.

64

Still relevant.

65

Still relevant.

[philnik]

• Address comments

[huixie90]

LGTM with comments addressed and green CI

libcxx/test/std/algorithms/algorithms.results/out_value_result.pass.cpp

65

I would also add a runtime test to check out the && overload works (for move only types)

[huixie90]

LGTM with green CI and comments addressed

[var-const]

libcxx/include/__algorithm/out_value_result.h

23

Please don't forget to remove this after rebasing (due to https://reviews.llvm.org/D132151).

libcxx/test/std/algorithms/algorithms.results/in_out_result.pass.cpp

94 ↗

(On Diff #420511)

return 0 should stay, unless there was a recent change regarding this that I'm not aware of.

libcxx/test/std/algorithms/algorithms.results/out_value_result.pass.cpp

64

A few test cases I'd like to add:

• check that out_value_result is trivial and standard layout when it is true of the arguments;
• check default construction;
• check that out_value_result is copyable and movable if the arguments are (I think a static_assert is enough);
• check that the rvalue overload of the conversion operator actually moves the arguments (by using a helper type that counts the number of copies and moves);
• check that the [[no_unique_address]] optimization is used;
• maybe check aggregate initialization (might be overkill).

I think a few of these are still not addressed -- in particular, trivial check, default construction and that the arguments are actually moved.

libcxx/include/CMakeLists.txt

set(files
   __algorithm/minmax.h
   __algorithm/minmax_element.h
   __algorithm/mismatch.h
   __algorithm/move.h
   __algorithm/move_backward.h
   __algorithm/next_permutation.h
   __algorithm/none_of.h
   __algorithm/nth_element.h
+  __algorithm/out_value_result.h
   __algorithm/partial_sort.h
   __algorithm/partial_sort_copy.h
   __algorithm/partition.h
   __algorithm/partition_copy.h
   __algorithm/partition_point.h
   __algorithm/pop_heap.h
   __algorithm/prev_permutation.h
   __algorithm/push_heap.h

libcxx/include/__algorithm/out_value_result.h

+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _LIBCPP___ALGORITHM_OUT_VALUE_RESULT_H
+#define _LIBCPP___ALGORITHM_OUT_VALUE_RESULT_H
+
+#include <__concepts/convertible_to.h>
+#include <__config>
+#include <__utility/move.h>
+
+#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
+#  pragma GCC system_header
+#endif
+
+_LIBCPP_BEGIN_NAMESPACE_STD
+
+#if _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+
+namespace ranges {
+
+template <class _OutIter1, class _Type1>
+struct out_value_result {
+  _LIBCPP_NO_UNIQUE_ADDRESS _OutIter1 out;
+  _LIBCPP_NO_UNIQUE_ADDRESS _Type1 value;
+
+  template <class _OutIter2, class _Type2>
+    requires convertible_to<const _OutIter1&, _OutIter2> && convertible_to<const _Type1&, _Type2>
+   _LIBCPP_HIDE_FROM_ABI constexpr
+   operator out_value_result<_OutIter2, _Type2>() const & {
+    return {out, value};
+  }
+
+  template <class _OutIter2, class _Type2>
+    requires convertible_to<_OutIter1, _OutIter2> && convertible_to<_Type1, _Type2>
+  _LIBCPP_HIDE_FROM_ABI constexpr
+  operator out_value_result<_OutIter2, _Type2>() && {
+    return {std::move(out), std::move(value)};
+  }
+};
+
+} // namespace ranges
+
+#endif // _LIBCPP_STD_VER > 17 && !defined(_LIBCPP_HAS_NO_INCOMPLETE_RANGES)
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___ALGORITHM_OUT_VALUE_RESULT_H

libcxx/include/algorithm

template <class I, class O1, class O2>
     struct in_out_out_result; // since C++20
 
   template <class I1, class I2>
     struct min_max_result;    // since C++20
 
   template <class I>
     struct in_found_result;   // since C++20
 
+  template <class O, class T>
+    struct out_value_result;  // since C++20
+
   template<forward_iterator I, sentinel_for<I> S, class Proj = identity,
     indirect_strict_weak_order<projected<I, Proj>> Comp = ranges::less>             // since C++20
   constexpr I min_element(I first, S last, Comp comp = {}, Proj proj = {});
 
   template<forward_range R, class Proj = identity,
     indirect_strict_weak_order<projected<iterator_t<R>, Proj>> Comp = ranges::less> // since C++20
   constexpr borrowed_iterator_t<R> min_element(R&& r, Comp comp = {}, Proj proj = {});
 

 #include <__algorithm/minmax.h>
 #include <__algorithm/minmax_element.h>
 #include <__algorithm/mismatch.h>
 #include <__algorithm/move.h>
 #include <__algorithm/move_backward.h>
 #include <__algorithm/next_permutation.h>
 #include <__algorithm/none_of.h>
 #include <__algorithm/nth_element.h>
+#include <__algorithm/out_value_result.h>
 #include <__algorithm/partial_sort.h>
 #include <__algorithm/partial_sort_copy.h>
 #include <__algorithm/partition.h>
 #include <__algorithm/partition_copy.h>
 #include <__algorithm/partition_point.h>
 #include <__algorithm/pop_heap.h>
 #include <__algorithm/prev_permutation.h>
 #include <__algorithm/push_heap.h>

libcxx/include/module.modulemap.in

module __algorithm {
       module minmax                          { private header "__algorithm/minmax.h" }
       module minmax_element                  { private header "__algorithm/minmax_element.h" }
       module mismatch                        { private header "__algorithm/mismatch.h" }
       module move                            { private header "__algorithm/move.h" }
       module move_backward                   { private header "__algorithm/move_backward.h" }
       module next_permutation                { private header "__algorithm/next_permutation.h" }
       module none_of                         { private header "__algorithm/none_of.h" }
       module nth_element                     { private header "__algorithm/nth_element.h" }
+      module out_value_result                { private header "__algorithm/out_value_result.h" }
       module partial_sort                    { private header "__algorithm/partial_sort.h" }
       module partial_sort_copy               { private header "__algorithm/partial_sort_copy.h" }
       module partition                       { private header "__algorithm/partition.h" }
       module partition_copy                  { private header "__algorithm/partition_copy.h" }
       module partition_point                 { private header "__algorithm/partition_point.h" }
       module pop_heap                        { private header "__algorithm/pop_heap.h" }
       module prev_permutation                { private header "__algorithm/prev_permutation.h" }
       module push_heap                       { private header "__algorithm/push_heap.h" }

libcxx/test/std/algorithms/algorithms.results/no_unique_address.compile.pass.cpp

 static_assert(sizeof(std::ranges::in_out_out_result<int, Empty, int>) == 2 * sizeof(int));
 static_assert(sizeof(std::ranges::in_out_out_result<int, int, Empty>) == 2 * sizeof(int));
 static_assert(sizeof(std::ranges::in_out_out_result<char, Empty, Empty>) == 2);
 static_assert(sizeof(std::ranges::in_out_out_result<Empty, char, Empty>) == 2);
 static_assert(sizeof(std::ranges::in_out_out_result<Empty, Empty, char>) == 2);
 static_assert(sizeof(std::ranges::in_out_out_result<int, Empty, Empty2>) == sizeof(int));
 static_assert(sizeof(std::ranges::in_out_out_result<Empty, Empty, Empty>) == 3);
 
+static_assert(sizeof(std::ranges::out_value_result<Empty, int>) == sizeof(int));
+static_assert(sizeof(std::ranges::out_value_result<int, Empty>) == sizeof(int));
+static_assert(sizeof(std::ranges::out_value_result<Empty, Empty>) == 2);
+
 // In min_max_result both elements have the same type, so they can't have the same address.
 // So the only way to test that [[no_unique_address]] is used is to have it in another struct
 struct MinMaxNoUniqueAddress {
   int a;
   TEST_NO_UNIQUE_ADDRESS std::ranges::min_max_result<Empty> b;
   int c;
 };
 
 static_assert(sizeof(std::ranges::min_max_result<Empty>) == 2);
 static_assert(sizeof(MinMaxNoUniqueAddress) == 8);
 
 static_assert(sizeof(std::ranges::in_found_result<Empty>) == sizeof(bool));

libcxx/test/std/algorithms/algorithms.results/out_value_result.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-has-no-incomplete-ranges
+
+// template <class O, class T>
+// struct out_value_result;
+
+#include <algorithm>
+#include <cassert>
+#include <type_traits>
+
+#include "MoveOnly.h"
+
+using std::ranges::out_value_result;
+
+struct A {
+  explicit A(int);
+};
+// no implicit conversion
+static_assert(!std::is_constructible_v<out_value_result<A, A>, out_value_result<int, int>>);
+
+struct B {
+  B(int);
+};
+// implicit conversion
+static_assert(std::is_constructible_v<out_value_result<B, B>, out_value_result<int, int>>);
+static_assert(std::is_constructible_v<out_value_result<B, B>, out_value_result<int, int>&>);
+static_assert(std::is_constructible_v<out_value_result<B, B>, const out_value_result<int, int>>);
+static_assert(std::is_constructible_v<out_value_result<B, B>, const out_value_result<int, int>&>);
+
+struct C {
+  C(int&);
+};
+static_assert(!std::is_constructible_v<out_value_result<C, C>, out_value_result<int, int>&>);
+
+// has to be convertible via const&
+static_assert(std::is_convertible_v<out_value_result<int, int>&, out_value_result<long, long>>);
+static_assert(std::is_convertible_v<const out_value_result<int, int>&, out_value_result<long, long>>);
+static_assert(std::is_convertible_v<out_value_result<int, int>&&, out_value_result<long, long>>);
+static_assert(std::is_convertible_v<const out_value_result<int, int>&&, out_value_result<long, long>>);
+
+// should be move constructible
+static_assert(std::is_move_constructible_v<out_value_result<MoveOnly, int>>);
+static_assert(std::is_move_constructible_v<out_value_result<int, MoveOnly>>);
+
+struct NotConvertible {};
+// conversions should not work if there is no conversion
+static_assert(!std::is_convertible_v<out_value_result<NotConvertible, int>, out_value_result<int, int>>);
+static_assert(!std::is_convertible_v<out_value_result<int, NotConvertible>, out_value_result<int, int>>);
+
+template <class T>
+struct ConstructibleFrom {
+  constexpr ConstructibleFrom(T c) : content{c} {}
+  T content;
+};
+
+constexpr bool test() {
+  { // check that the conversion operator works
+    out_value_result<int, double> res{10, 0.};
+    assert(res.out == 10);
+    assert(res.value == 0.);
+    out_value_result<ConstructibleFrom<int>, ConstructibleFrom<double>> res2 = res;
+    assert(res2.out.content == 10);
+    assert(res2.value.content == 0.);
+  }
+  { // check that out_value_result isn't overconstrained w.r.t. move/copy constuctors
+    out_value_result<MoveOnly, int> res{MoveOnly{}, 10};
+    assert(res.out.get() == 1);
+    assert(res.value == 10);
+    auto res2 = std::move(res);
+    assert(res.out.get() == 0);
+    assert(res.value == 10);
+    assert(res2.out.get() == 1);
+    assert(res2.value == 10);
+  }
+  { // check that structured bindings work
+    auto [out, value] = out_value_result<int, int>{1, 2};
+    assert(out == 1);
+    assert(value == 2);
+  }
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}