[libcxx-commits] [PATCH] D107785: [libc++] Add the __bind_back and __compose helpers

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

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/__functional/bind_back.h:27
+
+#if _LIBCPP_STD_VER > 17
+
----------------
ldionne wrote:
> Quuxplusone wrote:
> > Could this be `> 11`?
> > Could it be `> 11` if you used `_VSTD::__invoke` instead of `_VSTD::invoke`?
> > (Besides just feeling nicer, keeping our dependencies minimal might one day help us clean up old code faster. As soon as we drop support for C++11, the whole `#if` goes away. Whereas if you make it depend on C++20, then we have to keep the `#if` in place for another 9 years until we drop support for C++17.)
> I would agree because of the "dropping `#if`s faster" story, but I think the benefit of using standard facilities (i.e. `invoke` instead of `__invoke`) outweighs that. WDYT?
I see cost-but-no-benefit to using `_VSTD::invoke` over `_VSTD::__invoke`. We have `__invoke` precisely //so that// we can use it in places where `invoke` wouldn't have been available until C++17. Also `invoke` is heavier-weight than `__invoke`; we'd rather use `__invoke` than `invoke` throughout the library.


================
Comment at: libcxx/test/libcxx/utilities/function.objects/func.bind.partial/bind_back.pass.cpp:9
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17
+
----------------
ldionne wrote:
> Quuxplusone wrote:
> > If you do enable `__bind_back` back to C++14, then this line will need to change and you'll need to use more `::value` and less `_v` in this test.
> There isn't a huge benefit in doing this, since if Barry's paper gets in, `bind_back` would be enabled in C++20 (or 23) only. I'd basically rename it from `__bind_back` to `bind_back` and expose it publicly.
I doubt `bind_back` would be "DR'ed" back to C++20 at this point — but that doesn't stop libc++ from supporting it in C++20 anyway. Leaving `__bind_back` untested in '14 and '17 SGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107785