[libcxx-commits] [PATCH] D102992: [libcxx][type_traits] remove `std::is_literal_type` and `std::result_of` for C++20

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/std/utilities/function.objects/func.invoke/invoke.pass.cpp:101-104
     // Check that result_of_t matches Expect.
     typedef typename std::result_of<ClassFunc&&(Functor&&, NonCopyable&&)>::type
       ResultOfReturnType;
     static_assert((std::is_same<ResultOfReturnType, Expect>::value), "");
----------------
Quuxplusone wrote:
> I'm ambivalent as to whether you should keep `_LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS` at the top of the file, or simply put lines 101-104, 124-127, and 145-148 under `#if TEST_STD_VER <= 17` instead. I'm not requesting a change here, but maybe @ldionne will have a stronger opinion one way or the other.
Let's keep it as-is. As a matter of policy, I also want to suggest that we don't provide those escape hatches anymore, so they'll be easier to remove if the tests are written as they are right now.


================
Comment at: libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/result_of.deprecated.fail.cpp:9
 
-// UNSUPPORTED: c++03, c++11, c++14
+// REQUIRES: c++17
 
----------------
Quuxplusone wrote:
> I think this should be
> ```
> // UNSUPPORTED: c++03, c++11, c++14
> // ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS
> ```
> but defer to @ldionne since he just went through this with the `std::iterator` tests. (You could grep and check for what he did there. Maybe you already did.)
> 
> Same comment on meta.unary.prop/is_literal_type.deprecated.fail.cpp below.
Agreed with Arthur - that way, we'll be running that test in all standard modes >= C++17, which is what we want (since we're enabling the removed type traits anyway). If we didn't have the `_LIBCPP_ENABLE_CXX20_REMOVED_TYPE_TRAITS` escape hatch, then I agree this should be `// REQUIRES: c++17` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102992