[libcxx-commits] [PATCH] D124346: [libc++] Complete the implementation of N4190
Nikolas Klauser via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sat Apr 30 07:23:05 PDT 2022
philnik marked 5 inline comments as done.
philnik added inline comments.
Comment at: libcxx/include/__config:1235
+#if !defined(_LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION) && _LIBCPP_STD_VER > 14
+# define _LIBCPP_ABI_NO_BINDER_BASES
> I don't understand why we're making an ABI change in the stable ABI?
AFAICT this can only be ABI affecting if you have a class that is derived from both `unary_function` and another class that derives from it. Since there is no more `unary_function` to derive from, there is no way to have an ABI break. Same for `binary_function`.
Comment at: libcxx/include/__functional/function.h:100
+#endif // defined(_LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION)
> This comment is inconsistent with the `#if`'s condition. However, regardless, I think we should remove the `#if` entirely, since it is always valid.
How is the condition 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.
I'm going with removing them, since they are removed in C++20 anyways.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits