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

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

Quuxplusone accepted this revision.
Quuxplusone added a comment.

LGTM; just some naming nits.



================
Comment at: libcxx/include/functional:3002
+{
+    _VSTD::tuple<_Bound...> __bound;
+
----------------
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.



================
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>
----------------
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?


================
Comment at: libcxx/test/support/callable_types.h:186
+
+#endif // TEST_CALLABLE_TYPES_H
----------------
Nit: no newline at end of file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60368