[libcxx-commits] [PATCH] D124346: [libc++] Complete the implementation of N4190

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Apr 25 09:21:30 PDT 2022

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.

Comment at: libcxx/docs/Status/Cxx17Papers.csv:1
 "Paper #","Group","Paper Name","Meeting","Status","First released version"
 "`N3911 <https://wg21.link/n3911>`__","LWG","TransformationTrait Alias ``void_t``\ .","Urbana","|Complete|","3.6"
We need to add a release note, since this will cause some code to start failing to compile (because of deprecation warnings and the removed classes).

Comment at: libcxx/include/__config:1235
I don't understand why we're making an ABI change in the stable ABI?

Comment at: libcxx/include/__functional/binary_function.h:24
 template <class _Arg1, class _Arg2, class _Result>
-struct _LIBCPP_TEMPLATE_VIS binary_function
Can you please add a test to check that this is deprecated in C++11? (same for others).

Comment at: libcxx/include/__functional/function.h:100
This comment is inconsistent with the `#if`'s condition. However, regardless, I think we should remove the `#if` entirely, since it is always valid.

Comment at: libcxx/include/ext/__hash:25
 template <> struct _LIBCPP_TEMPLATE_VIS hash<const char*>
  : public std::unary_function<const char*, size_t>
Technically, I think we still need to be providing the `argument_type` and `result_type` typedefs in C++17, but they should be deprecated. In practice, I don't know whether we care, it may be acceptable to just remove them. But whatever we do, we should be consistent between different specializations of `std::hash` (the current patch isn't, since e.g. `std::hash<std::bitset<...>>` provides those typedefs.

Furthermore, if we do provide them in C++17, they should be marked as deprecated.

In C++20, they should never be provided.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list