[libcxx] adds std::identity to <functional>

[cjdb]

Summary

Implements parts of:

• P0898R3 Standard Library Concepts

[cjdb]

libcxx/include/functional

3073

Will add this line back in on commit or next arc diff.

[curdeius]

libcxx/include/functional

3222

is_transparent member type seems to be missing.

[cjdb]

applies @curdeius' feedback

[• Quuxplusone]

libcxx/include/functional

221

Please move this up to directly after bit_not (and before unary_negate).
The synopses of the bitwise operators were all cut-and-paste-typoed; I just pushed a fix (changing comments only) so now there is a bit_not to put this after. :)

libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp

44

Please, as @ldionne would say, "add some simple tests." :)
I think given the simplicity of std::identity, the entire test suite for it should look like this:

#include "support/moveonly.h"

constexpr bool test() {
    std::identity id;
    int i = 42;
    assert(id(i) == 42);
    assert(id(std::move(i)) == 42);

    MoveOnly m1 = 2;
    MoveOnly m2 = id(std::move(m1));
    assert(m2.value == 2);

    assert(&id(i) == &i);
    assert(&id(std::move(i)) == &i);
    assert(&id(std::move(m1) == &m1);
    static_assert(&id(id) == &id);

    const std::identity idc;
    assert(idc(1) == 1);
    assert(std::move(id)(1) == 1);
    assert(std::move(idc)(1) == 1);

    id = idc;  // test assignability

    static_assert(std::is_same_v<decltype(id(i)), int&>);
    static_assert(std::is_same_v<decltype(id(std::declval<int&&>())), int&&>);
    static_assert(std::is_same_v<decltype(id(std::declval<const int&>)), const int&>);
    static_assert(std::is_same_v<decltype(id(std::declval<const int&&>)), const int&&>);
    static_assert(std::is_same_v<decltype(id(std::declval<volatile int&>)), volatile int&>);
    static_assert(std::is_same_v<decltype(id(std::declval<volatile int&&>)), volatile int&&>);

    return true;
}

int main(int, char**) {
    test();
    static_assert(test());

    return 0;
}

I don't think the test suite should depend on C++20 nor concepts. I don't think we need any more lines of test code than what I've written here.

[• Quuxplusone]

libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp

44

...well, and also a test that std::identity::is_transparent is a typename.

static_assert(std::is_same_v<std::void_t<std::identity::is_transparent>, void>);

should do the trick.

[Mordante]

libcxx/include/functional

221

Please add comment that this has been added in C++20.

3224

Can you add a test that this function is noexcept?

libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp

44

@Quuxplusone The feature is C++20 so we can't drop that, but I agree with the concepts.
Since it's unspecified what is_transparent is I think it would be better not to test it's value. Doesn't the test at line 23 already test what you want?

@cjdb I'm also in favour of a simpler test for this feature.

[• Quuxplusone]

libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp

44

Since it's unspecified what is_transparent is I think it would be better not to test its value.

I agree; note the void_t in my suggestion. I believe this accomplishes the "it must be a type but I don't care what type it is" goal.

And this plays into my "not depend on C++20" request. The test on line 23 does test what I want, but it uses C++20 core-language Concepts features to do it. I prefer my suggestion because it doesn't depend on the compiler to implement C++20 Concepts; in that sense I'm requesting a "C++17 test for a C++20 library feature." I know we can't remove UNSUPPORTED: c++17; I just think it's cleaner to use the more "basic" language features anywhere we can. For the same reason, my proposed test cases use e.g. std::is_same_v<decltype(id(i)), int&> and not std::same_as<decltype(id(i)), int&>.

[Mordante]

libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp

44

My bad I missed the void_t.

Thanks for the additional information. I agree with your point.

[EricWF]

libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp

24

Is the type required to be void? If so, add a test.

112

Do we the defaulted comparison operator here? I would rather not depend on yet another new language feature that we don't need too.

[EricWF]

LGTM after addressing the current inline comments.
Please make sure the other reviewers don't have any unresolved comments before submitting.

[• Quuxplusone]

@cjdb: I am willing to commandeer this revision for the purpose of replacing the test file with my "simpler test" above (and then I'd be responsible for watching buildkite and landing it). Let me know if you want me to do that.
Alternatively, you don't have to do exactly what I pasted above, but IMNSHO the tests really need to be shorter and simpler somehow.

[cjdb]

After checking that no coverage is lost, I've adopted @Quuxplusone's suggested test with minor changes. There are still two concept tests, since I think both of those are simpler than the alternative.

[• Quuxplusone]

LGTM modulo the two unaddressed comments I see: "move the mention in the synopsis" and "add a test for noexceptness" (which I've taken the liberty of drafting).

libcxx/include/functional

3224

+1

struct X { X(const X&); X(X&&); };
void test_noexcept() {
    X x;
    static_assert(noexcept(id(x)));
    static_assert(noexcept(id(X()));
};

[cjdb]

applies final bit of feedback

[cjdb]

libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp

112

I think per @Quuxplusone's changes, this comment is outdated.

libcxx/include/functional

 };
 
 template <class T=void> // C++14
 struct bit_not : unary_function<T, T>
 {
     T operator()(const T& x) const;
 };
 
+struct identity; // C++20
+
 template <class Predicate>
 class unary_negate // deprecated in C++17
     : public unary_function<typename Predicate::argument_type, bool>
 {
 public:
     explicit unary_negate(const Predicate& pred);
     bool operator()(const typename Predicate::argument_type& x) const;
 };
public:
     explicit binary_negate(const Predicate& pred);
     bool operator()(const typename Predicate::first_argument_type& x,
                     const typename Predicate::second_argument_type& y) const;
 };
 
 template <class Predicate> // deprecated in C++17
 binary_negate<Predicate> not2(const Predicate& pred);
 
 template <class F>
 constexpr unspecified not_fn(F&& f); // C++17, constexpr in C++20
 
 template<class T> struct is_bind_expression;
 template<class T> struct is_placeholder;
 
     // See C++14 20.9.9, Function object binders
 template <class T> inline constexpr bool is_bind_expression_v
   = is_bind_expression<T>::value; // C++17

 _LIBCPP_CONSTEXPR_AFTER_CXX17 auto not_fn(_Fn&& __f)
 {
     return __perfect_forward<__not_fn_op, _Fn>(_VSTD::forward<_Fn>(__f));
 }
 
 #endif // _LIBCPP_STD_VER > 14
 
 #if _LIBCPP_STD_VER > 17
 
 struct __bind_front_op
 {
     template<class... _Args>
     constexpr static auto __call(_Args&&... __args)
     noexcept(noexcept(_VSTD::invoke(_VSTD::forward<_Args>(__args)...)))
     -> decltype(      _VSTD::invoke(_VSTD::forward<_Args>(__args)...))
     { return          _VSTD::invoke(_VSTD::forward<_Args>(__args)...); }
 };

 #if _LIBCPP_STD_VER > 17
 template <class _Tp>
 using unwrap_reference_t = typename unwrap_reference<_Tp>::type;
 
 template <class _Tp>
 using unwrap_ref_decay_t = typename unwrap_ref_decay<_Tp>::type;
 #endif // > C++17
 
+#if _LIBCPP_STD_VER > 17
+// [func.identity]
+struct identity {
+    template<class _Tp>
+    constexpr _Tp&& operator()(_Tp&& __t) const noexcept
+    {
+        return _VSTD::forward<_Tp>(__t);
+    }
+
+    using is_transparent = void;
+};
+#endif // _LIBCPP_STD_VER > 17
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif  // _LIBCPP_FUNCTIONAL

libcxx/test/std/utilities/function.objects/func.identity/identity.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
+
+// struct identity;
+
+#include <functional>
+
+#include <cassert>
+#include <concepts>
+
+#include "MoveOnly.h"
+
+static_assert(std::semiregular<std::identity>);
+static_assert(requires { typename std::identity::is_transparent; });
+
+constexpr bool test() {
+  std::identity id;
+  int i = 42;
+  assert(id(i) == 42);
+  assert(id(std::move(i)) == 42);
+
+  MoveOnly m1 = 2;
+  MoveOnly m2 = id(std::move(m1));
+  assert(m2.get() == 2);
+
+  assert(&id(i) == &i);
+  static_assert(&id(id) == &id);
+
+  const std::identity idc;
+  assert(idc(1) == 1);
+  assert(std::move(id)(1) == 1);
+  assert(std::move(idc)(1) == 1);
+
+  id = idc; // run-time checks assignment
+  static_assert(std::is_same_v<decltype(id(i)), int&>);
+  static_assert(std::is_same_v<decltype(id(std::declval<int&&>())), int&&>);
+  static_assert(
+      std::is_same_v<decltype(id(std::declval<int const&>())), int const&>);
+  static_assert(
+      std::is_same_v<decltype(id(std::declval<int const&&>())), int const&&>);
+  static_assert(std::is_same_v<decltype(id(std::declval<int volatile&>())),
+                               int volatile&>);
+  static_assert(std::is_same_v<decltype(id(std::declval<int volatile&&>())),
+                               int volatile&&>);
+  static_assert(
+      std::is_same_v<decltype(id(std::declval<int const volatile&>())),
+                     int const volatile&>);
+  static_assert(
+      std::is_same_v<decltype(id(std::declval<int const volatile&&>())),
+                     int const volatile&&>);
+
+  struct S {
+    constexpr S() = default;
+    constexpr S(S&&) noexcept(false) {}
+    constexpr S(S const&) noexcept(false) {}
+  };
+  S x;
+  static_assert(noexcept(id(x)));
+  static_assert(noexcept(id(S())));
+
+  return true;
+}
+
+int main(int, char**) {
+  test();
+  static_assert(test());
+
+  return 0;
+}