[libcxx-commits] [PATCH] D98151: [libcxx] adds std::identity to <functional>

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/functional:219
 
+struct identity;
+
----------------
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. :)


================
Comment at: libcxx/test/std/utilities/function.objects/func.identity/identity.pass.cpp:43
+  return result;
+}
+
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98151