[libc++] Add ranges::in_fun_result

[philnik]

Summary

Add ranges::in_fun_result

[• Quuxplusone]

libcxx/include/CMakeLists.txt

36

Here and modulemap, please alphabetize.
Good news: I'll render this nit unnecessary Real Soon Now via however D116958 ends up.

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

21–27

Could this plausibly use the MoveOnly type from test/support/moveonly.h instead, just to reduce the line count a bit?
(Ditto consistently in the in_in_result PR, if it also applies there; and in @var-const's PR that includes in_out_result. I hadn't noticed this nit before.)

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

1 ↗

(On Diff #398743)

Please fix the spelling of this file's filename, at some point (here or elsewhere or just as an NFC commit).

[philnik]

• Use MoveOnyl.h
• alphabetize CMakeLists.txt and module.modulemap

[philnik]

libcxx/include/CMakeLists.txt

36

I've already looked at D116958. Thanks for that!

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

1 ↗

(On Diff #398743)

This file was introduced in D116278. I'll fix it there.

[philnik]

• Use MoveOnly.h
• alphabetize CMakeLists.txt and module.modulemap

[philnik]

• Rebased
• Add constexpr test

[philnik]

• Add generated file

[CaseyCarter]

libcxx/include/__algorithm/in_fun_result.h

37

Shouldn't these be _VSTD-qualified?

libcxx/include/algorithm

720

This should be before in_in_result.h on line 702.

[philnik]

• Address comments

[philnik]

ping

[• Quuxplusone]

libcxx/include/algorithm

29

in_out_result isn't listed here; and in_in_out_result either isn't listed, or merge-conflicts.

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

32

Same as on the in_in_out review, I think A would benefit from a comment about // no implicit conversion, and B could just be B(int);
In fact, all my comments from over there also apply here. Please add the exhaustive sets of MoveOnly tests, and so on and so forth. When you're done, diff <(sed s/in_fun/foo/g in_fun_result.pass.cpp) <(sed s/in_in/foo/g in_in_result.pass.cpp) should be basically an empty diff, and ~~~ <(sed s/in_in_out/foo/g in_in_out_result.pass.cpp) not much different.

[var-const]

libcxx/include/__algorithm/in_fun_result.h

17

Does this file need the #pragma GCC system_header annotation?

23

Note: in in_out_result, I used more descriptive/verbose names like _InputIterator. Ideally these should be consistent. I think that having long names is more common in __algorithm/ (quick grepping shows ~360 hits for _InputIterator and 0 hits for _Ip in my current working copy).

30

_LIBCPP_HIDE_FROM_ABI?

libcxx/include/algorithm

29

in_out_result is at the bottom of synopsis (around line 655). I don't have a very strong opinion on where in the synopsis these should be listed, but they should all be together; I'd slightly prefer having them in the end because I feel that these are helper types that don't really need to be in the spotlight.

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

81

Please add return 0;.

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

25

Can you please move the corresponding tests in in_out_result.compile.pass.cpp to this file for consistency? (not necessarily in this patch)

32

Would it be worthwhile to also test for the case of Empty and Empty2 where both classes are empty but they are distinct types?

[var-const]

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

23–29

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?

61

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).

70

Perhaps also test with a copy-only type? (that could be overkill)

72

Ultranit: please include <utility>.

[philnik]

• Use MoveOnyl.h
• Add constexpr test
• Add generated file
• Address comments
• Address comments

[philnik]

• Apply changes from in_in_out_result

[philnik]

• Rebased
• use std

[var-const]

LGTM modulo the remaining unresolved comments.

[philnik]

• Fix CI

[• Quuxplusone]

libcxx/include/algorithm

29

@philnik thanks for moving in_out_result up! :)

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

73

s/10L/10/, right?

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

25

This was marked "Done", but not done. Since you moved the synopsis comment for in_out_result, I think it would be nice to also move the mentioned tests in this same commit. But if it happens in a followup, also cool.

[philnik]

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

25

I'll do it in the results cleanup PR. The synopsis are just to easy to miss for my taste, so I moved them here.

libcxx/include/CMakeLists.txt

set(files
   __algorithm/find_first_of.h
   __algorithm/find_if.h
   __algorithm/find_if_not.h
   __algorithm/for_each.h
   __algorithm/for_each_n.h
   __algorithm/generate.h
   __algorithm/generate_n.h
   __algorithm/half_positive.h
+  __algorithm/in_fun_result.h
   __algorithm/in_in_out_result.h
   __algorithm/in_in_result.h
   __algorithm/in_out_out_result.h
   __algorithm/in_out_result.h
   __algorithm/includes.h
   __algorithm/inplace_merge.h
   __algorithm/is_heap.h
   __algorithm/is_heap_until.h
   __algorithm/is_partitioned.h
   __algorithm/is_permutation.h
   __algorithm/is_sorted.h
   __algorithm/is_sorted_until.h
   __algorithm/iter_swap.h
   __algorithm/lexicographical_compare.h
   __algorithm/lower_bound.h

libcxx/include/__algorithm/in_fun_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_IN_FUN_RESULT_H
+#define _LIBCPP___ALGORITHM_IN_FUN_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
+
+#ifndef _LIBCPP_HAS_NO_CONCEPTS
+
+namespace ranges {
+template <class _Ip, class _Fp>
+struct in_fun_result {
+  [[no_unique_address]] _Ip in;
+  [[no_unique_address]] _Fp fun;
+
+  template <class _I2, class _F2>
+    requires convertible_to<const _Ip&, _I2> && convertible_to<const _Fp&, _F2>
+  _LIBCPP_HIDE_FROM_ABI constexpr operator in_fun_result<_I2, _F2>() const & {
+    return {in, fun};
+  }
+
+  template <class _I2, class _F2>
+    requires convertible_to<_Ip, _I2> && convertible_to<_Fp, _F2>
+  _LIBCPP_HIDE_FROM_ABI constexpr operator in_fun_result<_I2, _F2>() && {
+    return {std::move(in), std::move(fun)};
+  }
+};
+} // namespace ranges
+
+#endif // _LIBCPP_HAS_NO_RANGES
+
+_LIBCPP_END_NAMESPACE_STD
+
+#endif // _LIBCPP___ALGORITHM_IN_FUN_RESULT_H

libcxx/include/algorithm

/*
     algorithm synopsis
 
 #include <initializer_list>
 
 namespace std
 {
 
 namespace ranges {
+  template <class I, class F>
+    struct in_fun_result; // since C++20
+
   template <class I1, class I2>
     struct in_in_result;     // since C++20
 
   template <class I1, class I2, class O>
     struct in_in_out_result; // since C++20
 
   template <class I, class O1, class O2>
     struct in_out_out_result; // since C++20
+
+  template<class I, class O>
+    struct in_out_result; // since C++20
 }
 
 template <class InputIterator, class Predicate>
     constexpr bool     // constexpr in C++20
     all_of(InputIterator first, InputIterator last, Predicate pred);
 
 template <class InputIterator, class Predicate>
     constexpr bool     // constexpr in C++20

 
 template <class BidirectionalIterator>
     constexpr bool     // constexpr in C++20
     prev_permutation(BidirectionalIterator first, BidirectionalIterator last);
 
 template <class BidirectionalIterator, class Compare>
     constexpr bool     // constexpr in C++20
     prev_permutation(BidirectionalIterator first, BidirectionalIterator last, Compare comp);
-
-namespace ranges {
-// [algorithms.results], algorithm result types
-template<class InputIterator, class OutputIterator>
-    struct in_out_result;
-}
-
 }  // std
 
 */
 
 #include <__bits>
 #include <__config>
 #include <__debug>
 #include <cstddef>

 #include <__algorithm/find_first_of.h>
 #include <__algorithm/find_if.h>
 #include <__algorithm/find_if_not.h>
 #include <__algorithm/for_each.h>
 #include <__algorithm/for_each_n.h>
 #include <__algorithm/generate.h>
 #include <__algorithm/generate_n.h>
 #include <__algorithm/half_positive.h>
+#include <__algorithm/in_fun_result.h>
 #include <__algorithm/in_in_out_result.h>
 #include <__algorithm/in_in_result.h>
 #include <__algorithm/in_out_out_result.h>
 #include <__algorithm/in_out_result.h>
 #include <__algorithm/includes.h>
 #include <__algorithm/inplace_merge.h>
 #include <__algorithm/is_heap.h>
 #include <__algorithm/is_heap_until.h>
 #include <__algorithm/is_partitioned.h>
 #include <__algorithm/is_permutation.h>
 #include <__algorithm/is_sorted.h>
 #include <__algorithm/is_sorted_until.h>
 #include <__algorithm/iter_swap.h>
 #include <__algorithm/lexicographical_compare.h>

libcxx/include/module.modulemap

module __algorithm {
       module find_first_of            { private header "__algorithm/find_first_of.h" }
       module find_if                  { private header "__algorithm/find_if.h" }
       module find_if_not              { private header "__algorithm/find_if_not.h" }
       module for_each                 { private header "__algorithm/for_each.h" }
       module for_each_n               { private header "__algorithm/for_each_n.h" }
       module generate                 { private header "__algorithm/generate.h" }
       module generate_n               { private header "__algorithm/generate_n.h" }
       module half_positive            { private header "__algorithm/half_positive.h" }
+      module in_fun_result            { private header "__algorithm/in_fun_result.h" }
       module in_in_out_result         { private header "__algorithm/in_in_out_result.h" }
       module in_in_result             { private header "__algorithm/in_in_result.h" }
       module in_out_out_result        { private header "__algorithm/in_out_out_result.h" }
       module in_out_result            { private header "__algorithm/in_out_result.h" }
       module includes                 { private header "__algorithm/includes.h" }
       module inplace_merge            { private header "__algorithm/inplace_merge.h" }
       module is_heap                  { private header "__algorithm/is_heap.h" }
       module is_heap_until            { private header "__algorithm/is_heap_until.h" }

libcxx/test/libcxx/diagnostics/detail.headers/algorithm/in_fun_result.module.verify.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
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: modules-build
+
+// WARNING: This test was generated by 'generate_private_header_tests.py'
+// and should not be edited manually.
+
+// expected-error@*:* {{use of private header from outside its module: '__algorithm/in_fun_result.h'}}
+#include <__algorithm/in_fun_result.h>

libcxx/test/std/algorithms/algorithms.results/in_fun_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-no-concepts
+// UNSUPPORTED: libcpp-has-no-incomplete-ranges
+
+// template <class I1, class I2>
+// struct in_fun_result;
+
+#include <algorithm>
+#include <cassert>
+#include <type_traits>
+#include <utility>
+
+#include "MoveOnly.h"
+
+struct A {
+  explicit A(int);
+};
+// no implicit conversion
+static_assert(!std::is_constructible_v<std::ranges::in_fun_result<A, A>, std::ranges::in_fun_result<int, int>>);
+
+struct B {
+  B(int);
+};
+// implicit conversion
+static_assert(std::is_constructible_v<std::ranges::in_fun_result<B, B>, std::ranges::in_fun_result<int, int>>);
+static_assert(std::is_constructible_v<std::ranges::in_fun_result<B, B>, std::ranges::in_fun_result<int, int>&>);
+static_assert(std::is_constructible_v<std::ranges::in_fun_result<B, B>, const std::ranges::in_fun_result<int, int>>);
+static_assert(std::is_constructible_v<std::ranges::in_fun_result<B, B>, const std::ranges::in_fun_result<int, int>&>);
+
+struct C {
+  C(int&);
+};
+// has to be convertible via const&
+static_assert(!std::is_constructible_v<std::ranges::in_fun_result<C, C>, std::ranges::in_fun_result<int, int>&>);
+
+static_assert(std::is_convertible_v<std::ranges::in_fun_result<int, int>&, std::ranges::in_fun_result<long, long>>);
+static_assert(std::is_convertible_v<const std::ranges::in_fun_result<int, int>&, std::ranges::in_fun_result<long, long>>);
+static_assert(std::is_convertible_v<std::ranges::in_fun_result<int, int>&&, std::ranges::in_fun_result<long, long>>);
+static_assert(std::is_convertible_v<const std::ranges::in_fun_result<int, int>&&, std::ranges::in_fun_result<long, long>>);
+
+// should be move constructible
+static_assert(std::is_move_constructible_v<std::ranges::in_fun_result<MoveOnly, int>>);
+static_assert(std::is_move_constructible_v<std::ranges::in_fun_result<int, MoveOnly>>);
+
+// should not copy constructible with move-only type
+static_assert(!std::is_copy_constructible_v<std::ranges::in_fun_result<MoveOnly, int>>);
+static_assert(!std::is_copy_constructible_v<std::ranges::in_fun_result<int, MoveOnly>>);
+
+struct NotConvertible {};
+// conversions should not work if there is no conversion
+static_assert(!std::is_convertible_v<std::ranges::in_fun_result<NotConvertible, int>,
+                                     std::ranges::in_fun_result<int, int>>);
+static_assert(!std::is_convertible_v<std::ranges::in_fun_result<int, NotConvertible>,
+                                     std::ranges::in_fun_result<int, int>>);
+
+template <class T>
+struct ConvertibleFrom {
+  constexpr ConvertibleFrom(T c) : content{c} {}
+  T content;
+};
+
+constexpr bool test() {
+  {
+    std::ranges::in_fun_result<int, double> res{10, 0.};
+    assert(res.in == 10);
+    assert(res.fun == 0.);
+    std::ranges::in_fun_result<ConvertibleFrom<int>, ConvertibleFrom<double>> res2 = res;
+    assert(res2.in.content == 10);
+    assert(res2.fun.content == 0.);
+  }
+  {
+    std::ranges::in_fun_result<MoveOnly, int> res{MoveOnly{}, 2};
+    assert(res.in.get() == 1);
+    assert(res.fun == 2);
+    auto res2 = static_cast<std::ranges::in_fun_result<MoveOnly, int>>(std::move(res));
+    assert(res.in.get() == 0);
+    assert(res2.in.get() == 1);
+    assert(res2.fun == 2);
+  }
+  {
+    auto [in, fun] = std::ranges::in_fun_result<int, int>{1, 2};
+    assert(in == 1);
+    assert(fun == 2);
+  }
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}

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

 //   Each of the class templates specified in this subclause has the template parameters,
 //   data members, and special members specified below, and has no base classes or members
 //   other than those specified.
 
 #include <algorithm>
 
 struct Empty {};
 struct Empty2 {};
 
 static_assert(sizeof(std::ranges::in_in_result<Empty, int>) == sizeof(int));
 static_assert(sizeof(std::ranges::in_in_result<int, Empty>) == sizeof(int));
 static_assert(sizeof(std::ranges::in_in_result<Empty, Empty>) == 2);
 
 static_assert(sizeof(std::ranges::in_in_out_result<Empty, int, int>) == 2 * sizeof(int));
 static_assert(sizeof(std::ranges::in_in_out_result<int, Empty, int>) == 2 * sizeof(int));
 static_assert(sizeof(std::ranges::in_in_out_result<int, int, Empty>) == 2 * sizeof(int));
 static_assert(sizeof(std::ranges::in_in_out_result<char, Empty, Empty>) == 2);
 static_assert(sizeof(std::ranges::in_in_out_result<Empty, char, Empty>) == 2);
 static_assert(sizeof(std::ranges::in_in_out_result<Empty, Empty, char>) == 2);
 static_assert(sizeof(std::ranges::in_in_out_result<int, Empty, Empty2>) == sizeof(int));
 static_assert(sizeof(std::ranges::in_in_out_result<Empty, Empty, Empty>) == 3);
 
 static_assert(sizeof(std::ranges::in_out_out_result<Empty, int, int>) == 2 * sizeof(int));
 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::in_fun_result<Empty, int>) == sizeof(int));
+static_assert(sizeof(std::ranges::in_fun_result<int, Empty>) == sizeof(int));
+static_assert(sizeof(std::ranges::in_fun_result<Empty, Empty>) == 2);