[libcxx-commits] [PATCH] D130631: [libc++] Implement P0288R9 (move_only_function)

[Zhihao Yuan via Phabricator via libcxx-commits]

lichray added inline comments.


================
Comment at: libcxx/include/__functional/move_only_function.h:26
+#  define _LIBCPP_MOVE_ONLY_FUNCTION_REF &
+#  include <__functional/move_only_function_impl.h>
+
----------------
philnik wrote:
> lichray wrote:
> > We should aim for a macro-free implementation to benefit users.
> If you have an idea on how to do that I'd be happy to try it, but I don't think writing the implementation 12 times is worth removing the macros.
No macro and maintainable. Here is an example: https://github.com/zhihaoy/nontype_functional/blob/main/include/std23/move_only_function.h
With C++ explicit object member function, you can also turn the six `operator()` into one.


================
Comment at: libcxx/include/__functional/move_only_function_impl.h:70
+  using _DestroyFn = void(void*);
+  using _CallFn    = void;
+
----------------
philnik wrote:
> lichray wrote:
> > In his CppCon 2017 talk, Louis @ldionne showed that embedding a vtable in the object storage is not an optimization compared to using a vptr even if we ignore the fact that embedded vtable uses more space. Also, mo-func of 6 x pointers in size may be too large. It's even larger than Swift's existential container (3-pointers + metadata + vptr).
> Do you have any reasoning how large it should be? It's simple to change, since I don't know what the size should be.
According to STL the person, it is reasonable to "store a `std::string` locally." To libc++, that's 24 bytes; 32 bytes for the whole mv-func object with a vptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130631