[libcxx-commits] [PATCH] D60368: Add bind_front function (P0356R5)

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added a comment.

Thanks!



================
Comment at: libcxx/include/functional:3002
+{
+    _VSTD::tuple<_Bound...> __bound;
+
----------------
Quuxplusone wrote:
> Nit: Could you call this `tuple<_Bound...> __bounds` or `tuple<_Bound...> __tup` instead?
> I was really puzzled by line 3046's
> 
>     __bound(_VSTD::forward<_BoundArgs>(__bound)...) { }
> 
> for a minute.
> Also nit, qualifying `_VSTD::` shouldn't be necessary on `tuple` because it's a type, not a function.
> 
Thanks. Done and done. I updated `__bound` -> `__bound_`.


================
Comment at: libcxx/test/std/utilities/function.objects/func.bind_front/bind_front.pass.cpp:232
+template <class... Args>
+struct is_bind_front_invokable {
+  template <class... LocalArgs>
----------------
Quuxplusone wrote:
> Utter nit: `invocable`, not `invokable` ;)
> but also less nitty, isn't this just testing more like `is_bind_frontable`? I mean you're testing that `bind_front(LocalArgs...)` is makable, but you're not testing that the resulting bind object is actually invocable at all, right?
Fair enough. I was thinking about it more like, "is `bind_front` invocable" not "is the result invocable." But I think `is_bind_frontable` is more clear anyway, so I'll update it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60368