[libcxx-commits] [PATCH] D103753: [libc++] [P0619] Add _LIBCPP_ABI_NO_BINDER_BASES and remove binder typedefs in C++20.
Arthur O'Dwyer via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Sun Jun 6 08:09:09 PDT 2021
Quuxplusone added inline comments.
================
Comment at: libcxx/include/__functional_base:58
typedef bool __result_type; // used by valarray
+#if _LIBCPP_STD_VER <= 17 || defined(_LIBCPP_ENABLE_CXX20_REMOVED_BINDER_TYPEDEFS)
+ typedef bool result_type;
----------------
Mordante wrote:
> I'm not entirely happy with this construction. IIRC @ldionne considered not adding these `_LIBCPP_ENABLE_CXX20_REMOVED_BINDER_TYPEDEFS` flags anymore. If we don't add this enable macro we can simplify the code by only omitting the base class only in C++20 with the unstable API.
Well, I'd definitely like to get the better ABI layout even in `-std=c++14` mode; there's nothing C++20-specific about removing the base classes. It's just that in C++17-and-earlier we still need to provide the binder typedefs. This is the same general shape as what we did for `reverse_iterator`: remove the `std::iterator` base class whenever possible, but then continue to provide the iterator typedefs whenever necessary.
As I said somewhere else recently, I'm strongly in favor of keeping the `_LIBCPP_ABI` and `_LIBCPP_ENABLE_CXX??_REMOVED` macros, for internal documentation: it's easy to grep for and see all the places affected by a given section of p0619, and it's easy to see which sections we've actually implemented and which we still need to do.
I do think the `_LIBCPP_SUPPRESS_DEPRECATED_PUSH`/`_LIBCPP_SUPPRESS_DEPRECATED_POP` construction is confusing and noisy (and error-prone, as I found out by initially misplacing the push outside the `#if` on `bit_not` ;)) — but I'm trusting @ldionne that it's required.
================
Comment at: libcxx/test/std/utilities/function.objects/negators/binary_negate.pass.cpp:27
const F f = F(std::logical_and<int>());
+#if TEST_STD_VER <= 17
static_assert((std::is_same<int, F::first_argument_type>::value), "" );
----------------
Mordante wrote:
> Looking at the similar tests below, this `#if` should be removed due to using `-D_LIBCPP_ENABLE_CXX20_REMOVED_BINDER_TYPEDEFS`.
Whoops. I think I glanced at `logical_and` too quickly here; this test is actually testing the typedefs on `binary_negate`, not `logical_and`. So you're right, will remove this `#if` (and looks like I already managed to remove the ones below).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103753/new/
https://reviews.llvm.org/D103753
More information about the libcxx-commits
mailing list